enroll a stable durable conversation handle only after the authenticated actor can see the provider runtime session; permit recovery rebinding while preserving agent, tenant, and owner scope
resolve Discord approvals/stops through that durable snapshot and add CLI enrollment plus attach-to-SSE streaming
normalize authorization denials/audits, map consumed approval denials to Fastify 403, and protect SSE stream cleanup on disconnect
Verification
TURBO_FORCE=true pnpm typecheck
TURBO_FORCE=true pnpm lint
TURBO_FORCE=true pnpm format:check
TURBO_FORCE=true pnpm test
Codex code review: approved; security review: no findings
## Summary
- enroll a stable durable conversation handle only after the authenticated actor can see the provider runtime session; permit recovery rebinding while preserving agent, tenant, and owner scope
- resolve Discord approvals/stops through that durable snapshot and add CLI enrollment plus attach-to-SSE streaming
- normalize authorization denials/audits, map consumed approval denials to Fastify 403, and protect SSE stream cleanup on disconnect
## Verification
- `TURBO_FORCE=true pnpm typecheck`
- `TURBO_FORCE=true pnpm lint`
- `TURBO_FORCE=true pnpm format:check`
- `TURBO_FORCE=true pnpm test`
- Codex code review: approved; security review: no findings
Part of #709.
REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head 451f7e04ec
Verified at exact head 451f7e04ec6c45adb12007dd7131da6a2b199962; Woodpecker PR pipeline 1745 is terminal green on this head.
Blocker:
Discord approval is no longer reachable through the production Discord plugin. ChatGateway.handleDiscordApproval now accepts only /approve (no provider/session arguments), but plugins/discord/src/index.ts still routes to the approval socket event only when content.startsWith('/approve '). The plugin trims message content first, so a real /approve message is emitted as normal message, while /approve anything reaches discord:approve but is rejected by the gateway's ^/approve\s*$ check. This breaks the required Discord mint path and the cross-surface approve→stop flow; the tests bypass DiscordPlugin.handleDiscordMessage, so they do not catch it.
Positive findings retained:
apps/gateway/src/commands/command-authorization.service.ts is byte-identical to origin/main (same hash a9f829e7ecec721c6c585fe0da56fb5d6a6441e9), and the 7-field runtime action digest remains intact.
Runtime approval/stop paths use the resolved Mosaic admin actor and RuntimeProviderService consume path, not DISCORD_SERVICE_USER_ID.
Durable enrollment, snapshot-based Discord resolution, HTTP/CLI stream surface, CLI stop denial coverage, mint-side denial audit, and provider authz classification are present in code/tests.
Secret scan found only test fixture tokens, no live credential values.
No merge performed by reviewer.
Marker: ROR-732-REQUEST-CHANGES-451f7e04
REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head 451f7e04ec6c45adb12007dd7131da6a2b199962
Verified at exact head `451f7e04ec6c45adb12007dd7131da6a2b199962`; Woodpecker PR pipeline `1745` is terminal green on this head.
Blocker:
- Discord approval is no longer reachable through the production Discord plugin. `ChatGateway.handleDiscordApproval` now accepts only `/approve` (no provider/session arguments), but `plugins/discord/src/index.ts` still routes to the approval socket event only when `content.startsWith('/approve ')`. The plugin trims message content first, so a real `/approve` message is emitted as normal `message`, while `/approve anything` reaches `discord:approve` but is rejected by the gateway's `^/approve\s*$` check. This breaks the required Discord mint path and the cross-surface approve→stop flow; the tests bypass `DiscordPlugin.handleDiscordMessage`, so they do not catch it.
Positive findings retained:
- `apps/gateway/src/commands/command-authorization.service.ts` is byte-identical to `origin/main` (same hash `a9f829e7ecec721c6c585fe0da56fb5d6a6441e9`), and the 7-field runtime action digest remains intact.
- Runtime approval/stop paths use the resolved Mosaic admin actor and `RuntimeProviderService` consume path, not `DISCORD_SERVICE_USER_ID`.
- Durable enrollment, snapshot-based Discord resolution, HTTP/CLI stream surface, CLI stop denial coverage, mint-side denial audit, and provider authz classification are present in code/tests.
- Secret scan found only test fixture tokens, no live credential values.
No merge performed by reviewer.
Marker: ROR-732-REQUEST-CHANGES-451f7e04
VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer-sonnet] head 451f7e04ec
M3-V GATE: PASS
Independent re-review of coder0's remediation at head 451f7e04ec6c45adb12007dd7131da6a2b199962 (confirmed via GET /pulls/732, unmoved). Read-only plumbing only (git -C <repo> diff origin/main...451f7e04, git show <sha>:<path>, git grep <sha>); no working-tree mutation. All three prior BLOCKING gaps are genuinely closed (not re-masked), the hard constraint holds, and C3/C5 do not regress.
HARD CONSTRAINT — PASS.git diff origin/main...451f7e04 -- apps/gateway/src/commands/command-authorization.service.ts is EMPTY (file not in changed set). Full-diff grep for runtimeActionDigest/actionDigest returns nothing — the 7-field exact-action digest is untouched. runtime-provider-registry.service.ts changes only export RuntimeApprovalDeniedError and broaden denial classification (does not touch the digest or the consume contract).
C1 (cross-surface durable session) — CLOSED.
(a) Real production call site: interaction.controller.ts@Post('sessions/:sessionId/enroll') → durable.enroll(...), invoked only after runtime.listSessions(providerId, ctx) proves the runtime session is visible to the authenticated actor (enrollment/start boundary, not a test).
(b) sessionId = AgentService conversationId on both surfaces: enroll keys on the :sessionId route param; Discord approve/stop key on ingress.conversationId. Integration test uses conversation-1 as both, proving the shared key.
(c) Discord raw-from-text path GONE: /approve (no args) and /stop <approvalRef> now resolve providerId/runtimeSessionId from durableSessions.getSnapshot(ingress.conversationId) → snapshot.identity.*, with snapshot.identity.agentName reconciliation. The old /approve <provider> <session> / /stop <provider> <session> <ref> regexes are removed.
(d) tess-cross-surface.integration.test.ts exercises the REAL create→snapshot flow over a shared InMemoryDurableSessionStore: enrolls {providerId:'fleet', runtimeSessionId:'runtime-1'} via the CLI/controller surface, then triggers Discord /approve+/stop whose payloads contain NEITHER value. Asserts terminate was called with 'runtime-1' and actor 'mosaic-admin-1' — the link is resolved purely through the store, NOT hardcoded on both sides. Real RuntimeProviderService + real consumeRuntimeTerminationApproval (digest) execute end-to-end.
C2 (functional attach) — CLOSED.streamSession is exposed over HTTP via the new @Sse('sessions/:sessionId/stream') route (resolves getSnapshot → runtime.streamSession(snapshot.identity.providerId, runtimeSessionId, cursor, ctx)), and consumed by the CLI: tui/gateway-api.ts adds streamInteractionSession (SSE reader) and the attach command now attaches then streams; enroll command added and asserted in interaction.test.ts. Success-path test resumes a durable session by attaching and streaming its runtime events exercises getSnapshot→attach→stream (asserts attach and streamSession both called with the snapshot-resolved fleet/runtime-1, and the streamed event is delivered) — plus an SSE disconnect-cleanup test. NON-GATING NOTE: the literal token streamSession does not appear under packages/mosaic (grep = 0) because the CLI names its function streamInteractionSession hitting the /stream route; the functional requirement (HTTP exposure + CLI consumption of the stream) is met.
C4 (denial/audit parity) — CLOSED. (a) CLI/interaction denial specs added in interaction.controller.test.ts: wrong-actor (snapshot scope reject), session-agent mismatch, and denied-approval — each asserts terminate NOT called / error surfaced. (b) Discord mint-side null now routes through runtimeAudit.record({outcome:'denied', operation:'session.terminate', errorCode:'policy_denied'}) (not just logger.warn), proven by audits a Discord mint-side authorization denial. (c) TESS-M1-FUP-001 fixed: execute() catch now uses isAuthorizationDenied() classifying RuntimeApprovalDeniedError/ForbiddenException/{code:'forbidden'} as denied/policy_denied — test records a provider authorization rejection as denied rather than provider failure. (d) runtime-approval-denied.filter.ts maps RuntimeApprovalDeniedError → HTTP 403, wired via @UseFilters on the controller, with a unit test.
C3 (approval contract) — NO REGRESSION. command-authorization.service.ts byte-identical; the 7-field runtimeActionDigest, one-shot consume, and target-derived correlation are all preserved. C5 (name-as-config) — NO REGRESSION. No hardcoded 'Tess'/agent-name literals in new code; identity flows from MOSAIC_AGENT_NAME/configured instanceId.
Recovery-rebinding review:sameEnrollmentScope intentionally drops providerId/runtimeSessionId from the immutability check so a recovered runtime can rebind, but agentName/sessionId/tenantId/ownerId remain immutable AND service.enroll re-checks server-derived owner/tenant scope AND the controller requires runtime-session visibility before enroll — this is a scoped recovery affordance, not an ownership weakening.
Verdict: all gaps genuinely closed, hard constraint intact, no regressions → PR APPROVE and M3-V GATE PASS.
VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer-sonnet] head 451f7e04ec6c45adb12007dd7131da6a2b199962
M3-V GATE: PASS
Independent re-review of coder0's remediation at head `451f7e04ec6c45adb12007dd7131da6a2b199962` (confirmed via GET /pulls/732, unmoved). Read-only plumbing only (`git -C <repo> diff origin/main...451f7e04`, `git show <sha>:<path>`, `git grep <sha>`); no working-tree mutation. All three prior BLOCKING gaps are genuinely closed (not re-masked), the hard constraint holds, and C3/C5 do not regress.
**HARD CONSTRAINT — PASS.** `git diff origin/main...451f7e04 -- apps/gateway/src/commands/command-authorization.service.ts` is EMPTY (file not in changed set). Full-diff grep for `runtimeActionDigest`/`actionDigest` returns nothing — the 7-field exact-action digest is untouched. `runtime-provider-registry.service.ts` changes only export `RuntimeApprovalDeniedError` and broaden denial *classification* (does not touch the digest or the consume contract).
**C1 (cross-surface durable session) — CLOSED.**
(a) Real production call site: `interaction.controller.ts` `@Post('sessions/:sessionId/enroll')` → `durable.enroll(...)`, invoked only after `runtime.listSessions(providerId, ctx)` proves the runtime session is visible to the authenticated actor (enrollment/start boundary, not a test).
(b) sessionId = AgentService conversationId on both surfaces: enroll keys on the `:sessionId` route param; Discord approve/stop key on `ingress.conversationId`. Integration test uses `conversation-1` as both, proving the shared key.
(c) Discord raw-from-text path GONE: `/approve` (no args) and `/stop <approvalRef>` now resolve `providerId`/`runtimeSessionId` from `durableSessions.getSnapshot(ingress.conversationId)` → `snapshot.identity.*`, with `snapshot.identity.agentName` reconciliation. The old `/approve <provider> <session>` / `/stop <provider> <session> <ref>` regexes are removed.
(d) `tess-cross-surface.integration.test.ts` exercises the REAL create→snapshot flow over a shared `InMemoryDurableSessionStore`: enrolls `{providerId:'fleet', runtimeSessionId:'runtime-1'}` via the CLI/controller surface, then triggers Discord `/approve`+`/stop` whose payloads contain NEITHER value. Asserts `terminate` was called with `'runtime-1'` and actor `'mosaic-admin-1'` — the link is resolved purely through the store, NOT hardcoded on both sides. Real `RuntimeProviderService` + real `consumeRuntimeTerminationApproval` (digest) execute end-to-end.
**C2 (functional attach) — CLOSED.** `streamSession` is exposed over HTTP via the new `@Sse('sessions/:sessionId/stream')` route (resolves `getSnapshot` → `runtime.streamSession(snapshot.identity.providerId, runtimeSessionId, cursor, ctx)`), and consumed by the CLI: `tui/gateway-api.ts` adds `streamInteractionSession` (SSE reader) and the `attach` command now attaches then streams; `enroll` command added and asserted in `interaction.test.ts`. Success-path test `resumes a durable session by attaching and streaming its runtime events` exercises getSnapshot→attach→stream (asserts attach and streamSession both called with the snapshot-resolved `fleet`/`runtime-1`, and the streamed event is delivered) — plus an SSE disconnect-cleanup test. NON-GATING NOTE: the literal token `streamSession` does not appear under `packages/mosaic` (grep = 0) because the CLI names its function `streamInteractionSession` hitting the `/stream` route; the functional requirement (HTTP exposure + CLI consumption of the stream) is met.
**C4 (denial/audit parity) — CLOSED.** (a) CLI/interaction denial specs added in `interaction.controller.test.ts`: wrong-actor (snapshot scope reject), session-agent mismatch, and denied-approval — each asserts `terminate` NOT called / error surfaced. (b) Discord mint-side null now routes through `runtimeAudit.record({outcome:'denied', operation:'session.terminate', errorCode:'policy_denied'})` (not just logger.warn), proven by `audits a Discord mint-side authorization denial`. (c) TESS-M1-FUP-001 fixed: `execute()` catch now uses `isAuthorizationDenied()` classifying `RuntimeApprovalDeniedError`/`ForbiddenException`/`{code:'forbidden'}` as denied/policy_denied — test `records a provider authorization rejection as denied rather than provider failure`. (d) `runtime-approval-denied.filter.ts` maps `RuntimeApprovalDeniedError` → HTTP 403, wired via `@UseFilters` on the controller, with a unit test.
**C3 (approval contract) — NO REGRESSION.** command-authorization.service.ts byte-identical; the 7-field `runtimeActionDigest`, one-shot consume, and target-derived correlation are all preserved.
**C5 (name-as-config) — NO REGRESSION.** No hardcoded `'Tess'`/agent-name literals in new code; identity flows from `MOSAIC_AGENT_NAME`/configured `instanceId`.
**Recovery-rebinding review:** `sameEnrollmentScope` intentionally drops providerId/runtimeSessionId from the immutability check so a recovered runtime can rebind, but agentName/sessionId/tenantId/ownerId remain immutable AND `service.enroll` re-checks server-derived owner/tenant scope AND the controller requires runtime-session visibility before enroll — this is a scoped recovery affordance, not an ownership weakening.
Verdict: all gaps genuinely closed, hard constraint intact, no regressions → PR APPROVE and M3-V GATE PASS.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Verification
TURBO_FORCE=true pnpm typecheckTURBO_FORCE=true pnpm lintTURBO_FORCE=true pnpm format:checkTURBO_FORCE=true pnpm testPart of #709.
REQUEST CHANGES reviewer-of-record [W-jarvis:reviewer] head
451f7e04ecVerified at exact head
451f7e04ec6c45adb12007dd7131da6a2b199962; Woodpecker PR pipeline1745is terminal green on this head.Blocker:
ChatGateway.handleDiscordApprovalnow accepts only/approve(no provider/session arguments), butplugins/discord/src/index.tsstill routes to the approval socket event only whencontent.startsWith('/approve '). The plugin trims message content first, so a real/approvemessage is emitted as normalmessage, while/approve anythingreachesdiscord:approvebut is rejected by the gateway's^/approve\s*$check. This breaks the required Discord mint path and the cross-surface approve→stop flow; the tests bypassDiscordPlugin.handleDiscordMessage, so they do not catch it.Positive findings retained:
apps/gateway/src/commands/command-authorization.service.tsis byte-identical toorigin/main(same hasha9f829e7ecec721c6c585fe0da56fb5d6a6441e9), and the 7-field runtime action digest remains intact.RuntimeProviderServiceconsume path, notDISCORD_SERVICE_USER_ID.No merge performed by reviewer.
Marker: ROR-732-REQUEST-CHANGES-451f7e04
VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer-sonnet] head
451f7e04ecM3-V GATE: PASS
Independent re-review of coder0's remediation at head
451f7e04ec6c45adb12007dd7131da6a2b199962(confirmed via GET /pulls/732, unmoved). Read-only plumbing only (git -C <repo> diff origin/main...451f7e04,git show <sha>:<path>,git grep <sha>); no working-tree mutation. All three prior BLOCKING gaps are genuinely closed (not re-masked), the hard constraint holds, and C3/C5 do not regress.HARD CONSTRAINT — PASS.
git diff origin/main...451f7e04 -- apps/gateway/src/commands/command-authorization.service.tsis EMPTY (file not in changed set). Full-diff grep forruntimeActionDigest/actionDigestreturns nothing — the 7-field exact-action digest is untouched.runtime-provider-registry.service.tschanges only exportRuntimeApprovalDeniedErrorand broaden denial classification (does not touch the digest or the consume contract).C1 (cross-surface durable session) — CLOSED.
(a) Real production call site:
interaction.controller.ts@Post('sessions/:sessionId/enroll')→durable.enroll(...), invoked only afterruntime.listSessions(providerId, ctx)proves the runtime session is visible to the authenticated actor (enrollment/start boundary, not a test).(b) sessionId = AgentService conversationId on both surfaces: enroll keys on the
:sessionIdroute param; Discord approve/stop key oningress.conversationId. Integration test usesconversation-1as both, proving the shared key.(c) Discord raw-from-text path GONE:
/approve(no args) and/stop <approvalRef>now resolveproviderId/runtimeSessionIdfromdurableSessions.getSnapshot(ingress.conversationId)→snapshot.identity.*, withsnapshot.identity.agentNamereconciliation. The old/approve <provider> <session>//stop <provider> <session> <ref>regexes are removed.(d)
tess-cross-surface.integration.test.tsexercises the REAL create→snapshot flow over a sharedInMemoryDurableSessionStore: enrolls{providerId:'fleet', runtimeSessionId:'runtime-1'}via the CLI/controller surface, then triggers Discord/approve+/stopwhose payloads contain NEITHER value. Assertsterminatewas called with'runtime-1'and actor'mosaic-admin-1'— the link is resolved purely through the store, NOT hardcoded on both sides. RealRuntimeProviderService+ realconsumeRuntimeTerminationApproval(digest) execute end-to-end.C2 (functional attach) — CLOSED.
streamSessionis exposed over HTTP via the new@Sse('sessions/:sessionId/stream')route (resolvesgetSnapshot→runtime.streamSession(snapshot.identity.providerId, runtimeSessionId, cursor, ctx)), and consumed by the CLI:tui/gateway-api.tsaddsstreamInteractionSession(SSE reader) and theattachcommand now attaches then streams;enrollcommand added and asserted ininteraction.test.ts. Success-path testresumes a durable session by attaching and streaming its runtime eventsexercises getSnapshot→attach→stream (asserts attach and streamSession both called with the snapshot-resolvedfleet/runtime-1, and the streamed event is delivered) — plus an SSE disconnect-cleanup test. NON-GATING NOTE: the literal tokenstreamSessiondoes not appear underpackages/mosaic(grep = 0) because the CLI names its functionstreamInteractionSessionhitting the/streamroute; the functional requirement (HTTP exposure + CLI consumption of the stream) is met.C4 (denial/audit parity) — CLOSED. (a) CLI/interaction denial specs added in
interaction.controller.test.ts: wrong-actor (snapshot scope reject), session-agent mismatch, and denied-approval — each assertsterminateNOT called / error surfaced. (b) Discord mint-side null now routes throughruntimeAudit.record({outcome:'denied', operation:'session.terminate', errorCode:'policy_denied'})(not just logger.warn), proven byaudits a Discord mint-side authorization denial. (c) TESS-M1-FUP-001 fixed:execute()catch now usesisAuthorizationDenied()classifyingRuntimeApprovalDeniedError/ForbiddenException/{code:'forbidden'}as denied/policy_denied — testrecords a provider authorization rejection as denied rather than provider failure. (d)runtime-approval-denied.filter.tsmapsRuntimeApprovalDeniedError→ HTTP 403, wired via@UseFilterson the controller, with a unit test.C3 (approval contract) — NO REGRESSION. command-authorization.service.ts byte-identical; the 7-field
runtimeActionDigest, one-shot consume, and target-derived correlation are all preserved.C5 (name-as-config) — NO REGRESSION. No hardcoded
'Tess'/agent-name literals in new code; identity flows fromMOSAIC_AGENT_NAME/configuredinstanceId.Recovery-rebinding review:
sameEnrollmentScopeintentionally drops providerId/runtimeSessionId from the immutability check so a recovered runtime can rebind, but agentName/sessionId/tenantId/ownerId remain immutable ANDservice.enrollre-checks server-derived owner/tenant scope AND the controller requires runtime-session visibility before enroll — this is a scoped recovery affordance, not an ownership weakening.Verdict: all gaps genuinely closed, hard constraint intact, no regressions → PR APPROVE and M3-V GATE PASS.