diff --git a/clients/claude-code/README.md b/clients/claude-code/README.md index 535d1d0..2e983c1 100644 --- a/clients/claude-code/README.md +++ b/clients/claude-code/README.md @@ -303,7 +303,7 @@ and history reads cover the browse case. | `get_channel_description` | Read a channel's standing description — the short statement of what it's for, which a project keeps as its charter. `{description}`, or `null` when nobody has set one. Channel-level state, so `read_channel` won't show it. | | `set_channel_description` | Set a channel's description, replacing whatever it says now; pass an empty string to clear it. Idempotent. What you write is read back verbatim — a description the substrate can't store as given (too long, or spanning multiple lines) is refused with an error saying which, never silently trimmed or reflowed. Fails with a clear error when the bound identity can't edit the channel. | | `presence` | Report presence (`online` / `idle` / `offline`) for a single identity. | -| `current_identity` | Return `{state: 'bound', identity: {...}}` once acquire has resolved, or `{state: 'unbound', identity: null}` while a lazy ephemeral session is still lurking. Passive — never triggers acquire. | +| `current_identity` | Return `{state: 'bound', identity: {...}}` once acquire has resolved, `{state: 'credential_rejected', identity: {...}, message}` once the realm has refused the bound key as invalid (writes fail and inbound events stop until the seat restarts), or `{state: 'unbound', identity: null}` while a lazy ephemeral session is still lurking. Passive — never triggers acquire. | | `resolve` | Look up an identity by name. | | `download_file` | Fetch a `/user_uploads/...` attachment to a temp file; returns `{file_path, content_type, size}`. Use the Read tool on the returned path to view images. | | `upload_file` | Upload a local file (absolute path) to the realm; returns `{reference, filename, size}`. The `reference` is a ready-to-embed string — drop it into a `post` body wherever you want the attachment to render, the same way you write `@**Name**` mention markup yourself. | diff --git a/packages/core/ports.ts b/packages/core/ports.ts index 186f6d8..beb7854 100644 --- a/packages/core/ports.ts +++ b/packages/core/ports.ts @@ -512,6 +512,15 @@ export interface IdentityPort { * a programmer error, not a typed failure. */ currentIdentity(): Effect.Effect + /** + * Whether the realm has refused the bound credential as invalid since this + * adapter bound it. Passive: reads what earlier calls already learned and + * never touches the realm or mints. A rejected credential stays rejected + * until `release()` and a fresh `acquire`, because nothing else replaces + * it. Always `false` on a substrate with no credentials to invalidate, and + * `false` before any bind. + */ + credentialRejected(): Effect.Effect /** * Bind the adapter to an identity with the given display name. Per * substrate: diff --git a/packages/mcp/server.test.ts b/packages/mcp/server.test.ts index 9618314..b3fdfeb 100644 --- a/packages/mcp/server.test.ts +++ b/packages/mcp/server.test.ts @@ -209,6 +209,7 @@ const buildFakeAdapter = ( } const identityPort: IdentityPort = { currentIdentity: () => Effect.succeed(identity), + credentialRejected: () => Effect.succeed(false), acquire: (name) => Effect.suspend(() => { events.push(`acquire(${name})`) diff --git a/packages/mcp/tools.test.ts b/packages/mcp/tools.test.ts index 5290552..72d623c 100644 --- a/packages/mcp/tools.test.ts +++ b/packages/mcp/tools.test.ts @@ -348,6 +348,30 @@ test('current_identity returns the bound identity after ensureBound resolves', ( ), )) +test('current_identity reports credential_rejected once the realm has refused the bound key', () => + Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + const base = yield* rigAdapter() + const adapter: MemoryAdapterRig = { + ...base, + identity: { ...base.identity, credentialRejected: () => Effect.succeed(true) }, + } + const deps = yield* buildDeps(adapter) + yield* deps.ensureBound() + const rig = yield* mountAndConnect(adapter, deps) + const expected = yield* adapter.identity.currentIdentity() + const result = yield* Effect.promise(() => + rig.client.callTool({ name: 'current_identity', arguments: {} }), + ) + expect(result.structuredContent).toMatchObject({ + state: 'credential_rejected', + identity: { id: expected.id, name: expected.name, kind: expected.kind }, + }) + }), + ), + )) + test('current_identity before acquire returns the unbound sentinel (passive, never triggers acquire)', () => Effect.runPromise( Effect.scoped( diff --git a/packages/mcp/tools.ts b/packages/mcp/tools.ts index df5fe02..4698cac 100644 --- a/packages/mcp/tools.ts +++ b/packages/mcp/tools.ts @@ -278,6 +278,11 @@ type CurrentIdentityResult = readonly last_post_body: string }> } + | { + readonly state: 'credential_rejected' + readonly identity: SerializedIdentity + readonly message: string + } | { readonly state: 'unbound'; readonly identity: null } /** @@ -641,7 +646,7 @@ const buildToolDefs = (deps: RegisterToolsDeps, cache: InternalCache): ReadonlyA { name: 'current_identity', description: - 'Return the identity this session is bound to. Passive — never triggers acquire. Returns {state: "unbound", identity: null} for ephemeral sessions that have not yet performed an attribution-producing action; {state: "bound", identity: {id, name, kind}, recent_threads?} once acquire has resolved. recent_threads is a best-effort orientation list of {channel, thread, last_post_ts, last_post_body} for the threads this identity most recently posted in; it is omitted when the enrichment lookup fails — the binding self-check itself never fails on it.', + 'Return the identity this session is bound to. Passive — never triggers acquire. Returns {state: "unbound", identity: null} for ephemeral sessions that have not yet performed an attribution-producing action; {state: "bound", identity: {id, name, kind}, recent_threads?} once acquire has resolved; {state: "credential_rejected", identity, message} when the realm has refused the bound credential as invalid, so writes fail and inbound events stop until the seat restarts. recent_threads is a best-effort orientation list of {channel, thread, last_post_ts, last_post_body} for the threads this identity most recently posted in; it is omitted when the enrichment lookup fails — the binding self-check itself never fails on it.', inputSchema: { type: 'object', properties: { cwd: cwdField }, @@ -663,6 +668,14 @@ const buildToolDefs = (deps: RegisterToolsDeps, cache: InternalCache): ReadonlyA return { state: 'unbound', identity: null } } cache.rememberIdentity(current.identity) + if (await run(adapter.identity.credentialRejected())) { + return { + state: 'credential_rejected', + identity: identityShape(current.identity), + message: + "The realm has refused this seat's credential as invalid. Writes will keep failing and inbound events are not being delivered. Restart the seat to bind a fresh credential.", + } + } const bound = { state: 'bound' as const, identity: identityShape(current.identity) } try { const threads = await run(adapter.history.recentThreads(current.identity.id)) diff --git a/packages/memory/adapter.ts b/packages/memory/adapter.ts index 6723c05..911a5db 100644 --- a/packages/memory/adapter.ts +++ b/packages/memory/adapter.ts @@ -581,6 +581,7 @@ const memorySeat = (realm: MemoryRealm, config: MemorySeatConfig): Effect.Effect const identity: IdentityPort = { currentIdentity: () => readBound(), + credentialRejected: () => Effect.succeed(false), acquire: (name) => Ref.get(bound).pipe( Effect.flatMap( diff --git a/packages/testing/contract.ts b/packages/testing/contract.ts index c486638..db1d014 100644 --- a/packages/testing/contract.ts +++ b/packages/testing/contract.ts @@ -127,6 +127,13 @@ export interface ContractEnv { body: MessageBody, opts?: PostOpts, ) => Effect.Effect + /** + * Optional. Invalidate the credential the bound self holds, so the + * substrate refuses its next call as unauthenticated. Substrates with no + * credentials to invalidate (Memory) omit it and the suite skips the + * tests that need a refused credential. + */ + readonly revokeBoundCredential?: () => Effect.Effect /** * Longest channel description this substrate stores, in characters. The * description tests build their over-length input from it rather than diff --git a/packages/testing/contract/identity.ts b/packages/testing/contract/identity.ts index 6f9d30d..f78ab3a 100644 --- a/packages/testing/contract/identity.ts +++ b/packages/testing/contract/identity.ts @@ -1,7 +1,13 @@ /** The shared contract's tests for IdentityPort. */ import { expect, test } from 'bun:test' -import { decodeBotNameSync, decodeDisplayNameSync, UnknownIdentity } from '@commy/core/ports' +import { + decodeBotNameSync, + decodeChannelNameSync, + decodeDisplayNameSync, + decodeMessageBodySync, + UnknownIdentity, +} from '@commy/core/ports' import { Cause, Effect, Exit, Option } from 'effect' import type { ContractEnv } from '../contract.ts' @@ -234,4 +240,54 @@ export const identityContract = (env: () => ContractEnv): void => { expect(error).toBeInstanceOf(UnknownIdentity) }), )) + const postHello = (): Effect.Effect => { + const channel = env().seedChannel('credential-check') + return channel.pipe( + Effect.flatMap((ref) => env().comms.publisher.post(ref.name, decodeMessageBodySync('hello'))), + ) + } + test('identity.credentialRejected is false while the bound credential has not been refused', () => + Effect.runPromise( + Effect.gen(function* () { + yield* postHello() + expect(yield* env().comms.identity.credentialRejected()).toBe(false) + }), + )) + test('identity.credentialRejected stays false when a write is refused for another reason', () => + Effect.runPromise( + Effect.gen(function* () { + yield* Effect.flip( + env().comms.publisher.post( + decodeChannelNameSync('no-such-channel'), + decodeMessageBodySync('hello'), + ), + ) + expect(yield* env().comms.identity.credentialRejected()).toBe(false) + }), + )) + test('identity.credentialRejected is true once the substrate has refused the bound credential', () => + Effect.runPromise( + Effect.gen(function* () { + const revoke = env().revokeBoundCredential + if (revoke === undefined) return + const ref = yield* env().seedChannel('credential-check') + yield* revoke() + yield* Effect.flip(env().comms.publisher.post(ref.name, decodeMessageBodySync('hello'))) + expect(yield* env().comms.identity.credentialRejected()).toBe(true) + }), + )) + test('identity.credentialRejected clears when the seat releases and binds again', () => + Effect.runPromise( + Effect.gen(function* () { + const revoke = env().revokeBoundCredential + if (revoke === undefined) return + const me = yield* env().comms.identity.currentIdentity() + const ref = yield* env().seedChannel('credential-check') + yield* revoke() + yield* Effect.flip(env().comms.publisher.post(ref.name, decodeMessageBodySync('hello'))) + yield* env().comms.identity.release() + yield* env().comms.identity.acquire(decodeBotNameSync(me.name)) + expect(yield* env().comms.identity.credentialRejected()).toBe(false) + }), + )) } diff --git a/packages/zulip/adapter.ts b/packages/zulip/adapter.ts index f2b6606..cc1a392 100644 --- a/packages/zulip/adapter.ts +++ b/packages/zulip/adapter.ts @@ -108,6 +108,7 @@ import { ApiKey, BotEmail, decodeUserUploadPath, + isCredentialRejected, isTransientZulipError, makeZulipHttp, ZulipApiError, @@ -631,6 +632,26 @@ export const zulipAdapter = ( ), ) const boundRef = yield* SynchronizedRef.make>(Option.none()) + const credentialRejectedRef = yield* Ref.make(false) + + // Every call the bound credential makes, the events-queue poll included, + // goes out through this, so the first refusal of the key itself is + // recorded wherever it happens rather than only when a write loses. + const recordRejection = (call: Effect.Effect): Effect.Effect => + call.pipe( + Effect.tapError((e) => + e instanceof ZulipApiError && isCredentialRejected(e) + ? Ref.set(credentialRejectedRef, true) + : Effect.void, + ), + ) + const watchBoundHttp = (inner: BotHttp): BotHttp => ({ + get: (path, schema, params) => recordRejection(inner.get(path, schema, params)), + post: (path, schema, body) => recordRejection(inner.post(path, schema, body)), + patch: (path, schema, body) => recordRejection(inner.patch(path, schema, body)), + delete: (path, schema, body) => recordRejection(inner.delete(path, schema, body)), + uploadRaw: (filename, data) => recordRejection(inner.uploadRaw(filename, data)), + }) const requireBound = (): Effect.Effect => SynchronizedRef.get(boundRef).pipe( @@ -1188,6 +1209,7 @@ export const zulipAdapter = ( // need a coherent error — dying matches the port // contract of "throws when unbound". currentIdentity: () => requireBound().pipe(Effect.map((b) => b.identity)), + credentialRejected: () => Ref.get(credentialRejectedRef), // check-bound -> mint -> set is one atomic SynchronizedRef transition: // the effectful mint round-trip runs while the ref is locked, so two // concurrent acquires can never both observe `none` and double-mint. @@ -1227,7 +1249,9 @@ export const zulipAdapter = ( config.hostHeader, ), ).pipe(Effect.provideService(HttpClient.HttpClient, httpClient)) - const http = wrapBotHttp(innerHttp, recipientDirectory, minted.userId) + const http = watchBoundHttp( + wrapBotHttp(innerHttp, recipientDirectory, minted.userId), + ) const ident: Identity = { id: yield* decodeIdentityId(String(minted.userId)), name: yield* decodeDisplayName(name), @@ -1247,6 +1271,7 @@ export const zulipAdapter = ( http, origin: minted.origin, } + yield* Ref.set(credentialRejectedRef, false) return [ { identity: ident, credentials, origin: minted.origin }, Option.some(next), diff --git a/packages/zulip/http.ts b/packages/zulip/http.ts index 7ce972c..afffb8b 100644 --- a/packages/zulip/http.ts +++ b/packages/zulip/http.ts @@ -292,6 +292,13 @@ const classifyEnvelope = ( }) } +/** + * Whether the realm refused the request's credential itself, as opposed to + * refusing the request. Zulip answers an invalidated or deactivated bot key + * with 401 and no other request failure. + */ +export const isCredentialRejected = (err: ZulipApiError): boolean => err.status === 401 + const TRANSPORT_FAILURE_STATUS = 0 /** diff --git a/packages/zulip/in-process-contract.ts b/packages/zulip/in-process-contract.ts index 19fe6dd..2a73337 100644 --- a/packages/zulip/in-process-contract.ts +++ b/packages/zulip/in-process-contract.ts @@ -91,6 +91,10 @@ export const inProcessContractFactory: AttachmentContractFactory = async () => { Effect.asVoid, Effect.orDie, ), + revokeBoundCredential: () => + adapter.identity + .currentIdentity() + .pipe(Effect.flatMap((me) => realm.revokeApiKey(Number(me.id)))), channelDescriptionLimit: CHANNEL_DESCRIPTION_LIMIT, threadNameLimit: TOPIC_LIMIT, // The realm and its adapters hold nothing outside the heap, so there is diff --git a/packages/zulip/in-process-realm.ts b/packages/zulip/in-process-realm.ts index fc4825d..7b5ad99 100644 --- a/packages/zulip/in-process-realm.ts +++ b/packages/zulip/in-process-realm.ts @@ -68,6 +68,8 @@ export interface InProcessRealm { /** Deactivate the member with this user id, as an administrator would. */ readonly deactivateUser: (userId: number) => Effect.Effect readonly setPresence: (userId: number, status: PresenceStatus) => Effect.Effect + /** Invalidates the user's API key, so the next call made with it is refused. */ + readonly revokeApiKey: (userId: number) => Effect.Effect } const REALM_HOST = 'zulip.example.com' @@ -1382,5 +1384,19 @@ export const makeInProcessRealm: Effect.Effect = Effect.gen(func ...s, presence: HashMap.set(s.presence, userId, status), })), + revokeApiKey: (userId) => + newKey.pipe( + Effect.flatMap((apiKey) => + SubscriptionRef.update(stateRef, (s) => + Option.match( + findUser(s, (u) => u.userId === userId), + { + onNone: () => s, + onSome: (user) => putUser(s, { ...user, apiKey }), + }, + ), + ), + ), + ), } })