feat(#709): add configured Discord interaction binding #730
Assignees
jason.woltje
Clear assignees
jason.woltje
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mosaicstack/stack#730
Reference in New Issue
Block a user
Delete Branch "feat/tess-discord"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Refs #707
Task: TESS-M3-001 / TESS-DSC-001
Adds configuration-owned Discord interaction bindings with pairing/RBAC, thread metadata, attachments, streaming correlation, and authenticated ingress reuse.
REQUEST CHANGES — ROR for PR #730 at exact head
25ed3676550649d805737ddd97cec08d49873662.CI verification: PR CI is terminal green at this head (
ci/woodpecker/pr/cipipeline1734success).Findings:
Thread/sub-session ingress is rejected before the new parent-channel binding logic can run. In
plugins/discord/src/index.ts,handleDiscordMessageexits on!this.isAllowedMessage(message)at lines 278-280.isAllowedMessagethen checksallowedChannelIdsagainstmessage.channelIdat lines 333-340. For Discord thread messages,message.channelIdis the thread ID, while the configured interaction binding is later resolved againstparentChannelId ?? channelIdat lines 296-304. Result: a valid paired user in a configured parent channel thread is dropped unless every dynamic thread ID is pre-added toDISCORD_ALLOWED_CHANNEL_IDS, which defeats the configured thread/sub-session support requested for this slice. The allowlist check needs to use the same parent-channel ID used for binding, with regression coverage for a thread under an allowed bound channel.Discord approval flow is not wired to the durable exact-action approval path. The new RBAC model defines an
approveoperation atplugins/discord/src/index.tslines 19-37, but the only resolver call in the Discord plugin is for'send'at lines 299-305, and the gateway ingress resolver also only checks'send'atapps/gateway/src/chat/chat.gateway.tslines 659-666. I do not see a Discord path that invokes the M2CommandAuthorizationService.createRuntimeTerminationApproval/ durableagent:<name>:command-approval:*store for approvals, nor denial/approval tests for that Discord path. Scenario: an admin-paired Discord user cannot issue a durable exact-action approval from Discord for a stop/privileged action, so the requested Discord approvals path is absent rather than proven durable/one-shot.Notes: I did verify this PR reuses the signed Discord service envelope + allowlists + replay protector and adds config-owned Nova binding coverage. I did not find live production Discord token material in the diff.
No approval posted. Re-ROR needed after a new head fixes the above and CI remains terminal green.
Marker: ROR-730-REQUEST-CHANGES-25ed3676
REQUEST CHANGES — re-ROR for PR #730 at exact head
689d5b68706fd5f5b190b0e4d988efc59fb51acb.CI verification: PR CI is terminal green at this head (
ci/woodpecker/pr/cipipeline1736success).Findings:
Discord approval still does not use the M2 durable exact-action runtime approval path. The new handler at
apps/gateway/src/chat/chat.gateway.tslines 642-668 parses/approve <command>and callsthis.commandExecutor.createApproval(...). That delegates toCommandAuthorizationService.createApprovalthroughapps/gateway/src/commands/command-executor.service.tslines 164-170, which creates the generic slash-command approval, not the M2 runtime termination approval (createRuntimeTerminationApproval) bound to provider/session/actor/tenant/channel/correlation/agentName and stored underagent:<name>:command-approval:*. Scenario: a Discord admin approval can only create a command approval for a command name; it cannot produce the exact-action one-shot approval required by the CLI/runtimestoppath, so the Discord stop/privileged approval is not tied to the M2 durable runtime approval surface.The requested Discord approval denial/approval and thread regression tests are still absent. I found the Nova binding/allowlist/replay tests in
apps/gateway/src/plugin/discord-ingress.security.spec.ts, but no tests coveringdiscord:approve,CommandAuthorizationService.createRuntimeTerminationApproval, one-shot replay rejection for Discord approval, or a Discord thread message under an allowed bound parent channel. The code change toplugins/discord/src/index.tslines 333-340 now usesparentChannelId ?? message.channelIdfor the allowlist, which is directionally correct, but the required regression proof is not present.Prior positives still hold: signed service envelope + allowlists + replay protector are reused; config-owned Nova binding remains; I did not find live production Discord token material in the diff.
No approval posted. Re-ROR needed after a new head wires Discord approval to the durable exact-action runtime approval path and adds the missing denial/approval + thread regressions with CI green.
Marker: ROR-730-REQUEST-CHANGES-689d5b68
REQUEST CHANGES — re-ROR for PR #730 at exact head
34b82bd257154ece2f51d29df698b01150cb9a2b.CI verification: PR CI is terminal green at this head (
ci/woodpecker/pr/cipipeline1738success).Findings:
Discord approval now calls
createRuntimeTerminationApproval, but the action it creates is not consumable by the runtime stop path. Inapps/gateway/src/chat/chat.gateway.tslines 646-678,/approve <provider> <session>creates a runtime approval withactorId = DISCORD_SERVICE_USER_ID,channelId = ingress.channelId, andcorrelationId = ingress.correlationId. The M2 runtime approval verifier consumes approvals only when provider/session/actor/tenant/channel/correlation/agentName exactly match the later terminate action. A CLI/runtimestoprequest uses the authenticated stopping actor and that request's correlation/channel, not the Discord service user and approval-message correlation. Scenario: Discord admin issues/approve fleet runtime-1; the returned approval ref will not satisfy the exact-action stop path unless the terminate call is made under the same service actor and same approval-message correlation. That is not the same approval the CLI/runtime stop path consumes, so the Discord approval remains disconnected from the requested stop flow.The required Discord-path regressions are still absent. I searched the test tree for
discord:approval,handleDiscordApproval,createRuntimeTerminationApproval, thread/parent allowlist regression terms, and one-shot replay coverage. The only hits are the pre-existingCommandAuthorizationServiceunit tests;apps/gateway/src/plugin/discord-ingress.security.spec.tsstill only covers service token/signature/allowlist/replay plus Nova binding. There is no test fordiscord:approvehappy path, denial, one-shot replay rejection for a Discord-created approval, or accepting a thread message under an allowed bound parent channel. The code atplugins/discord/src/index.tslines 333-342 is directionally improved for parent-channel allowlisting, but the requested regression proof is not present.Prior positives still hold: signed service envelope + allowlists + replay protector are reused; config-owned Nova binding remains; I did not find live production Discord token material in the diff.
No approval posted. Re-ROR needed after a new head wires Discord approval to a consumable exact-action stop flow and adds the missing Discord-path/thread regressions with CI green.
Marker: ROR-730-REQUEST-CHANGES-34b82bd2
REQUEST CHANGES — re-ROR for PR #730 at exact head
a4c70c5a71a4a73f24f04060a301703ba5ac7530.CI verification: PR CI is terminal green at this head (
ci/woodpecker/pr/cipipeline1739success).Findings:
Discord approval is still not exercised by a matching Discord-originated consume/terminate path.
apps/gateway/src/chat/chat.gateway.tslines 646-678 mints a runtime termination approval viacreateRuntimeTerminationApproval, but there is nodiscord:stop/Discord terminate handler in this PR that consumes that approval and calls the runtime termination path under the same Discord-originated flow. Per the orchestrator scoping, cross-surface CLI consumption is not required; however, the Discord-surface mint→consume→one-shot flow is still absent, so the approval remains dead from the Discord ingress surface.Runtime termination approval exactness was weakened instead of proving the Discord dimensions.
apps/gateway/src/commands/command-authorization.service.tsnow removes actor/tenant/channel/correlation fromruntimeActionDigestand from required action validation/consume comparison. That means a runtime approval is no longer exact-action-bound to actor/tenant/channel/correlation as required by the M2 durable approval contract and by this review request. A matching Discord flow should prove the same Discord service actor + channel + correlation dimensions; this change makes those dimensions audit metadata only, not authorization identity.Required Discord-path regressions are still absent.
apps/gateway/src/plugin/discord-ingress.security.spec.tsstill only covers service token/signature/allowlist/replay and Nova binding. I found no tests for: (a)discord:approvehappy-path mint+consume, (b) denial for non-admin/unpaired Discord user, (c) one-shot replay rejection of a Discord-created approval, or (d) accepting a Discord thread message under an allowed bound parent channel.Prior positives still hold: signed service envelope + allowlists + replay protector are reused; config-owned Nova binding remains; I did not find live production Discord token material in the diff.
No approval posted. Re-ROR needed after a new head wires and tests the Discord-surface exact-action mint→consume→one-shot flow and restores exact binding semantics with CI green.
Marker: ROR-730-REQUEST-CHANGES-a4c70c5a
Independent re-review — PR #730
feat(#709): add configured Discord interaction bindingReviewer: W-jarvis:reviewer-sonnet (non-author, independent of LANE=coder4)
Head reviewed:
485e24a06a7f49b25516532482dd08f6806708a3(confirmed via APIGET /pulls/730immediately before review; matched the expected head, no restart needed)CI:
success(woodpeckerci/woodpecker/pr/ci, target run https://ci.mosaicstack.dev/repos/47/pipeline/1740/1)VERDICT: REQUEST CHANGES
Criterion 1 — Exact-action binding restored (SECURITY, priority) — VERIFIED PASS
Head commit
485e24a0is a literalgit revertof the weakening commita4c70c5a. Diff vsorigin/mainforapps/gateway/src/commands/command-authorization.service.tsis empty — the file is byte-identical tomain, i.e. fully restored to the verified M2 state (git diff $(git merge-base HEAD origin/main) HEAD -- apps/gateway/src/commands/command-authorization.service.ts= no output).Confirmed at
apps/gateway/src/commands/command-authorization.service.ts:runtimeActionDigest(line 210-224) hashesproviderId, sessionId, actorId, tenantId, channelId, correlationId, agentName— all 7 dimensions are authorization identity, not just audit metadata.hasRuntimeTerminationAction(line 198-208) requires all 7 fields non-empty.consumeRuntimeTerminationApproval(line 122-145) rejects unlessapproval.actionDigest === runtimeActionDigest(action)(covers channelId/correlationId via the digest) and explicitapproval.actorId === action.actorIdandapproval.tenantId === action.tenantId(defense-in-depth beyond the digest) and unexpired.command-authorization.service.spec.ts(5 tests, all pass) exercises mint→consume, wrong-actor rejection, wrong-tenant rejection, replay rejection, and cross-restart persistence via Redis.Test evidence:
npx vitest run src/commands/command-authorization.service.spec.ts→ 5/5 passed.The
a4c70c5aregression is fully reverted. No weakening present at head.Criterion 2 — Discord mint→consume→one-shot flow present and exercised — FAILED
Mint exists, consume does not, on the Discord surface.
apps/gateway/src/chat/chat.gateway.ts:646—@SubscribeMessage('discord:approve')handleDiscordApprovalmints aRuntimeTerminationApprovalviacreateRuntimeTerminationApproval({ providerId, sessionId, actorId: serviceUserId, tenantId: DISCORD_SERVICE_TENANT_ID, channelId: ingress.channelId, correlationId: ingress.correlationId, agentName: binding.instanceId })(lines 664-670), then at line 673 doesclient.emit('discord:approval', {...})— i.e. it only reports mint success back over the socket.discord:stop,discord:terminate, or any handler that callsconsumeRuntimeTerminationApproval/CommandAuthorizationService.consumeRuntimeTerminationApprovalfrom Discord ingress.grep -rn "discord:approval\|discord:stop" apps/gateway/src plugins/discord/srcshowsdiscord:approvalis only everemitted (chat.gateway.ts:673), never.on()'d — not even byplugins/discord/src/index.ts, whose onlysocket.on(...)handlers areconnect,disconnect,connect_error,agent:text,agent:end,agent:start(no listener fordiscord:approval).consumeRuntimeTerminationApprovalin the entire repo isapps/gateway/src/commands/runtime-approval-verifier.ts→RuntimeProviderRegistryService.terminate()(runtime-provider-registry.service.ts:251-278), which in turn is called from exactly one place in the whole codebase:apps/gateway/src/agent/interaction.controller.ts:128—POST /api/interaction/:agentName/sessions/:sessionId/stop, an HTTP endpoint gated byAuthGuard, deriving itscontext(actorId/tenantId/channelId/correlationId) from an authenticated HTTP user session (scopeFromUser), not from Discord. This is the M3-002 CLI's own stop path (explicitly out-of-scope per the orchestrator ruling), and it cannot practically be driven by the Discord service actor/tenant/channel/correlation tuple that Discord's mint uses./approvecommand is never consumed by anything reachable from the Discord surface. This is precisely the "dead code from the Discord surface" failure mode flagged in the review brief — the mint is real but has no matching consumer, so the Discord-originated termination capability does not actually exist end-to-end.Blocking finding: add a Discord-originated stop/terminate handler (e.g.
@SubscribeMessage('discord:stop')inchat.gateway.ts, triggered by e.g./stop <provider> <session> <approvalId>from Discord, gated the same waydiscord:approveis) that callsconsumeRuntimeTerminationApproval/runtime.terminateusing the same Discord actor/tenant/channel/correlation dimensions the approval was minted with, and rejects a second call (one-shot). Without it, criterion 2 cannot pass regardless of criterion 1's correctness.Criterion 3 — Discord-path regression tests (all four required) — FAILED (1 of 4 present, and it isn't one of the 4)
apps/gateway/src/plugin/discord-ingress.security.spec.tsdiff adds exactly one new test: "binds a differently named configured interaction instance without code changes" (this is the criterion-4 config-owned-Nova-binding positive, not one of the four required here).Checked against the required four (file:
discord-ingress.security.spec.ts, 8 tests total, all pass, but none cover):handleDiscordApproval/discord:approve, and no test exercises consume at all for a Discord-originated approval.operationRoles.approve = ['admin']is enforced in code (resolveDiscordInteractionBinding), but no spec exercises anoperator-only or unpaired user attempting'approve'and asserts rejection.DiscordReplayProtectormessage-ID replay, which is unrelated to the runtime-termination-approval one-shot consume semantics.parentChannelId ?? channelId) — MISSING. TheparentChannelIdfallback logic exists in bothisAllowedMessageandhandleDiscordMessage(plugins/discord/src/index.ts:294-296, 327-329) but has zero test coverage anywhere in the diff or pre-existing suite (grep -rn "parentChannelId\|parentId" plugins/discord/src plugins/discordfinds only the implementation, no spec references it).Test evidence:
npx vitest run src/plugin/discord-ingress.security.spec.ts→ 8/8 pass, but 0 of the 4 required scenarios are represented.Criterion 4 — Positives — VERIFIED PASS
resolveDiscordIngress(chat.gateway.ts:681+) still callsverifyDiscordIngressEnvelopeandthis.discordReplayProtector.claim(...)exactly as before; the new binding check is inserted additively, no prior check removed (confirmed via diff, insertions only).parseDiscordInteractionBindings+resolveDiscordInteractionBinding(plugins/discord/src/index.ts:15-84) are pure functions ofDISCORD_INTERACTION_BINDINGSenv JSON; the new spec test proves an arbitraryinstanceId("Nova") binds without code changes.grepacross the diff shows only env-var reads (DISCORD_BOT_TOKEN,DISCORD_SERVICE_TOKEN,DISCORD_SERVICE_USER_ID,DISCORD_SERVICE_TENANT_ID,DISCORD_INTERACTION_BINDINGS) — no hardcoded secrets, no token logging.Criterion 5 — Quality gate — VERIFIED PASS
success(API-confirmed).pnpm install --frozen-lockfile+turbo run buildfor internal workspace deps, using a local--store-dirto work around an environment-only pnpm-store permission issue unrelated to this PR):npx vitest runinapps/gateway→ 569 passed, 11 skipped, 0 failed (46 files passed, 5 skipped integration files requiring external infra).discord-ingress.security.spec.ts8/8,command-authorization.service.spec.ts5/5,chat.gateway-command-approval.spec.ts2/2,command-executor-tess-security.spec.ts3/3,runtime-provider-registry.service.test.ts9/9,session-gc.service.spec.ts7/7 — all pass.turbo run typecheck --filter=@mosaicstack/gateway→ success (content-hash cache hit against identical source at this head).turbo run lint --filter=@mosaicstack/gateway --force(forced, cache bypassed) → success, no lint errors, across all 14 affected packages.turbo run lint test --filter=@mosaicstack/discord-plugin --force→ lint clean; package has no dedicated spec files (logic is covered via gateway'sdiscord-ingress.security.spec.tsimporting its exports), consistent with the existing pattern in this repo.Summary
The security regression from the prior round (
a4c70c5a) is confirmed fully reverted — good news, and the top priority is resolved. However the PR does not yet deliver a complete, testable Discord-surface approval flow:/approvemints aRuntimeTerminationApprovalthat nothing on the Discord surface ever consumes, and none of the four required Discord-path regression tests (approve happy-path consume, non-admin/unpaired denial, one-shot replay, thread-under-parent-channel acceptance) exist. Per the self-contained-Discord-surface scoping ruling, this is in-scope work still missing, not a scope question.Blocking findings:
apps/gateway/src/chat/chat.gateway.ts— no Discord-originated consume/terminate handler exists; add e.g. adiscord:stophandler that callsconsumeRuntimeTerminationApprovalwith the same Discord actor/tenant/channel/correlation dimensions used at mint, then invokes runtime termination, then verify a replay of the same approval is rejected.apps/gateway/src/plugin/discord-ingress.security.spec.ts— add the 4 required regression tests: (a) discord approve → mint → consume happy path, (b) approve denied for non-admin/unpaired Discord user, (c) one-shot replay rejection of a Discord-created runtime-termination approval, (d) thread message under an allow-bound parent channel accepted viaparentChannelId ?? channelId.Marker: ROR-730-REQUEST-CHANGES-485e24a0
REQUEST CHANGES — re-ROR for PR #730 at exact head
6af68e76de3657d837875b1c9d0f5c9678429e59.CI verification: PR CI is terminal green at this head (
ci/woodpecker/pr/cipipeline1742success).Findings:
Discord-created approval is still not consumable by the target CLI/runtime stop identity.
apps/gateway/src/chat/chat.gateway.tslines 650-682 mint the runtime approval usingactorId: DISCORD_SERVICE_USER_ID,channelId: ingress.channelId, andcorrelationId: ingress.correlationId. The requested target identity is the approving admin / target termination action, not the Discord service user. A CLI/runtime stop will derive actor/channel/correlation from the stopping request, so it will not match this Discord-service approval under the preserved 7-field digest. This violates the round-5 bar:actorId = approving admin, NOT DISCORD_SERVICE_USER_ID.Discord approve→stop test masks a real correlation mismatch.
apps/gateway/src/plugin/discord-ingress.security.spec.tslines 215-238 and 262-286 useingressEnvelope(...), butcreatePayloadalways defaultscorrelationId: 'correlation-001'(lines 118-127). In production,plugins/discord/src/index.tsline 314 creates a newrandomUUID()per Discord message, so/approveand/stopare separate messages with different correlations. Becausecommand-authorization.service.tscorrectly keepscorrelationIdin the runtime digest (lines 210-224), a real stop after an approval message will fail unless the stop uses the same target correlation. The regression should prove the target-action correlation explicitly rather than sharing a fixture default.Discord stop path consumes under service actor, not approving admin.
apps/gateway/src/chat/chat.gateway.tslines 704-714 callruntimeRegistry.terminatewithactorScope.userId = DISCORD_SERVICE_USER_ID. That can only consume approvals minted for the service user, not the approving Discord admin. It also means the audit/authorization identity for termination is the bot service account rather than the paired admin user.Positive checks: command-authorization exactness is restored/preserved with all 7 fields in
RuntimeTerminationApprovalActionandruntimeActionDigest; thread-under-parent and Discord approval tests are now present; signed envelope + allowlist + replay reuse remain; I found no live production Discord token material in the diff.No approval posted. Re-ROR needed after the Discord approval/stop identity is bound to the target admin/action dimensions and tests cover distinct approve/stop correlations with CI green.
Marker: ROR-730-REQUEST-CHANGES-6af68e76
REQUEST-CHANGES — reviewer-of-record [W-jarvis:reviewer-sonnet], head
6af68e76de3657d837875b1c9d0f5c9678429e59Independent adversarial review of PR #730 against the acceptance contract (D1 consume symmetry/one-shot, D2 four regression tests, hard command-authorization.service.ts constraint). Verified against pure git objects (
git diff origin/main...6af68e76,git show <sha>:<path>), not the shared worktree's working tree (which had unrelated concurrent uncommitted edits from a live session — ignored).Hard constraint: PASSES.
git diff origin/main...6af68e76 -- apps/gateway/src/commands/command-authorization.service.tsis empty. No weakening ofruntimeActionDigestfound anywhere in the diff (grep fordigestacross the full diff returns nothing — the digest logic is untouched).D1/D2 mechanics (one-shot, denial, thread-binding): correct in isolation.
consumeRuntimeTerminationApprovaldeletes the redis key on success (one-shot),resolveDiscordInteractionBinding/pairing RBAC correctly gates unpaired/non-admin users, and the threadparentChannelId ?? channelIdlogic (plugins/discord/src/index.ts:294-296, 327-329) is implemented and covered.BLOCKING — the mint/consume digest can never match in real usage (correlationId):
apps/gateway/src/chat/chat.gateway.tshandleDiscordApproval(~line 672) mints the approval withcorrelationId: ingress.correlationIdtaken from the/approvemessage's own envelope.handleDiscordStop(~line 712) consumes usingcorrelationId: ingress.correlationIdfrom the separate, later/stopmessage's envelope.plugins/discord/src/index.ts:314assignscorrelationId: randomUUID()fresh for every inbound Discord message, with no mechanism anywhere in the diff (or the rest of the gateway) that threads/persists the original/approvemessage's correlationId forward to the/stopmessage. SincecorrelationIdis one of the 7 fields hashed intoruntimeActionDigest(command-authorization.service.ts:210-224) andconsumeRuntimeTerminationApprovalrequires exact digest equality, every real-worlddiscord:stopattempt will fail ("approval is invalid, expired, replayed, or does not match this action"). The feature is non-functional as shipped. This is masked in tests only becausediscord-ingress.security.spec.ts'screatePayload()hardcodes the same defaultcorrelationId: 'correlation-001'for both the approve and stop test messages (never overridden by theingressEnvelope()helper used in the "mints...and consumes" and "replay" tests) — unrealistic test data that doesn't reflect how the Discord plugin actually generates correlationIds per message.BLOCKING — agentName symmetry is not real; the "happy path" test doesn't exercise the real execution path:
Mint sets
agentName: binding.instanceId(chat.gateway.ts, per-channel config value). Consume, in the real production path (RuntimeProviderService.terminate, apps/gateway/src/agent/runtime-provider-registry.service.ts:251-278 — unchanged by this PR), setsagentName: configuredAgentName(), i.e. the single process-wideMOSAIC_AGENT_NAMEenv var. Nothing ties these together, and the redis approval key itself is namespaced by agentName (agent:<agentName>:command-approval:<id>), so any mismatch is an outright cache miss, not just a digest mismatch. The newinteractionBindingsconfig is an array (multiple per-channel bindings, explicitly "isolates approvals between interaction agents" per its own doc comment), whileMOSAIC_AGENT_NAMEis a single value per gateway process — these are only accidentally equal when a gateway serves exactly one binding whose instanceId happens to match its own env var. Crucially, the added testmints a Discord approval and consumes it for the exact stop actiondoes not exercise the realRuntimeProviderService.terminate/configuredAgentName()code at all: it substitutes a hand-written fakeruntimeRegistry.terminate(discord-ingress.security.spec.ts) that hardcodesagentName: 'Nova'literally, never readingprocess.env['MOSAIC_AGENT_NAME']. This test provides no evidence the real wired integration works, and combined with the correlationId issue above, D2(a)'s "approve happy-path" acceptance criterion is not actually satisfied by a trustworthy test.Verdict: REQUEST-CHANGES. Fix required: (1) make the correlationId (or an equivalent invariant identifier) actually reproducible/shared between the
/approvemint and the later/stopconsume — e.g. store the minted approval's binding correlationId and require the stop path to look it up rather than deriving a fresh one, or drop correlationId from the exact-action digest for this cross-message flow and rely on approvalId + providerId/sessionId/actorId/tenantId/agentName instead. (2) TieagentNameat consume time to the same per-bindinginstanceIdused at mint (not the gateway-globalMOSAIC_AGENT_NAME), or explicitly document/enforce a 1:1 gateway-to-binding invariant. (3) Update the "happy path" test to exercise the realRuntimeProviderService.terminatewiring end-to-end (not a hand-rolled stub) so it would actually catch these two issues.VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer] head
533e970259Evidence verified at exact head
533e9702591436810160dfb1cf8a42a222cfb7c7:1743is terminal green on this commit.mosaicUserId) rather thanDISCORD_SERVICE_USER_ID; stop passes that actor throughRuntimeProviderService.terminate, which consumes via the runtime approval verifier path.CommandAuthorizationServicekeeps all 7 runtime digest fields intact: providerId/sessionId/actorId/tenantId/channelId/correlationId/agentName.No merge performed by reviewer.
Marker: ROR-730-APPROVE-533e9702
VERIFIED APPROVE reviewer-of-record [W-jarvis:reviewer-sonnet] head
533e970259Re-review of the remediated head via read-only git plumbing (
git -C <worktree> diff origin/main...533e9702,git show <sha>:<path>— no working-tree mutation this round). Head sha confirmed against GET /pulls/730 (.head.sha =533e970259, unmoved). Both prior BLOCKING findings are fixed and proven by tests; the hard constraint still holds.HARD CONSTRAINT — PASS.
git diff origin/main...533e9702 -- apps/gateway/src/commands/command-authorization.service.tsis EMPTY. The onlydigesthit in the full diff is the newdiscordRuntimeActionCorrelationhelper (chat.gateway.ts) which computes a stable SHA-256 over the termination target to feed as the correlationId input — it does not touchruntimeActionDigest, which remains in the unchanged service and still hashes all 7 fields. No exact-action weakening anywhere.Bug #1 (correlationId) — FIXED & PROVEN. Both mint (
handleDiscordApproval) and consume (handleDiscordStop) now feed the digest's correlationId viadiscordRuntimeActionCorrelation(binding.instanceId, ingress, providerId, sessionId), derived from the immutable target[instanceId, guildId, channelId, conversationId, providerId, sessionId]— NOT the ephemeral per-message envelope correlationId (plugins/discord/src/index.ts randomUUID). Identical on both sides. The happy-path testconsumes the exact target once when approval and stop are separate Discord messagesgives the two Discord messages DIFFERENT envelope correlationIds (approval-ingress-correlationvsstop-ingress-correlation), still consumes successfully, and asserts the consumed action's correlationId matches/^discord-action:v1:/— directly proving the value is target-derived, not envelope-derived.Bug #2 (agentName) — FIXED & PROVEN. Mint now resolves
agentName = MOSAIC_AGENT_NAMEand explicitly denies whenbinding.instanceId !== agentName(warns + emitssuccess:false). The real consume path (RuntimeProviderService.terminate) usesconfiguredAgentName()= the sameMOSAIC_AGENT_NAMEenv var — one source, both sides. Explicit mismatch-denial testrejects approval when the binding targets a different runtime agentsetsMOSAIC_AGENT_NAME='Other'againstinstanceId='Nova'and asserts approval rejected. Critically, the happy-path test now instantiates the REALRuntimeProviderService(verified against its real constructor/terminate/provider()/assertCapability/deriveScopeat this sha) — the injected registry/audit are legitimate transport/sink fakes, while the security-critical resolution (configuredAgentName(), scope derivation, and realconsumeRuntimeTerminationApproval) all execute. No hand-rolled fake with hardcoded 'Nova' — agentName flows fromprocess.envthroughconfiguredAgentName().Other 3 tests — present & asserting.
rejects unpaired and non-admin Discord users): operator-role approve rejected (success:false); empty-bindings stop assertsdiscord:stopNOT emitted.rejects replaying a Discord-created termination approval): two stops (each with a distinct envelope correlationId) assert[true, false]— proving one-shot AND stable-target correlation across replay.accepts a thread message through its allowed bound parent channel): asserts resolved envelopechannelId === 'channel-001'viaparentChannelId ?? channelId.Bonus strengthening (not a regression): consume now runs under the approving admin's provisioned
mosaicUserId(viaresolveDiscordInteractionActorId), never the Discord service account;resolveDiscordInteractionActorIdreturns null for legacy string-form pairings, so privileged approve/stop require an explicit Mosaic identity. Test assertsconsumedActions[0].actorId === 'mosaic-admin-001'.All acceptance criteria met (D1 consume symmetry + one-shot + real execution wiring; D2 all four tests meaningful; hard constraint intact). APPROVE.