Skip to content

fix(webhook): resolve a canonical turn scope for webhook agent actions#357

Merged
anconina merged 4 commits into
comisai:mainfrom
roeikosh:fix/webhook-turn-scope-authority
Jul 26, 2026
Merged

fix(webhook): resolve a canonical turn scope for webhook agent actions#357
anconina merged 4 commits into
comisai:mainfrom
roeikosh:fix/webhook-turn-scope-authority

Conversation

@roeikosh

@roeikosh roeikosh commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Description

Every webhook-triggered agent action fail-closes before the agent runs. The turn aborts with
step:"context-authority", errorKind:"precondition", and returns "The request could not be
prepared safely because its conversation authority was unavailable."
Observed in production on
1.0.55; still reproducible on 1.0.56.

Root cause — a layer mismatch, not a defect at the throw site. #344 made a canonical turn scope
a hard precondition in the executor:

// packages/agent/src/executor/pi-executor/pi-executor.ts
const executionTurnScope = tryGetContext()?.turnScope;
if (executionTurnScope === undefined || !executionConversationRef?.ok) { /* abort */ }

Every turn initiator was updated to resolve a turn identity and thread its scope through — gateway
RPC/MCP, the OpenAI-compatible routes, cron, heartbeat, notification, task-extraction,
proactive-scheduler — except the webhook path. A webhook action hand-builds a
NormalizedMessage and calls getExecutor().execute() directly, bypassing the orchestrator inbound
pipeline that resolves identity for real channels, so nothing ever resolved a scope for it. Its
request context carried tenantId, userId, sessionKey, agentId, trustLevel and
deliveryOrigin — but no turnScope.

Real channel messages are unaffected (the inbound pipeline resolves their scope). Webhooks are
fully dead: no drive starts, and action: "agent" mappings are inert.

Fix. Resolve the turn identity in the webhook handler, as every sibling initiator already does,
and thread its scope into the request context. The resolved identity becomes the single authority
for the turn: its display session key and its turn scope describe the same conversation, so the
session view and the memory/context authority cannot diverge.

Security properties, chosen deliberately:

  • The principal is the operator-configured mapping, sha256-hashed for delimiter safety — never
    payload-controlled. A request body cannot forge or widen a conversation's authority even though
    sessionKey templates interpolate request data.
  • The rendered session key is JSON-encoded into the conversation identity rather than
    concatenated, so a crafted payload carrying the : session-key delimiter cannot split the
    identity and address another conversation.
  • trustLevel stays guest.
  • Per-subject conversation isolation is preserved: repeat events for one subject continue the same
    conversation, different subjects stay partitioned.
  • Identity-resolution failure logs with a hint naming the keys to check, emits the
    diagnostic:webhook_delivered failure event, and rejects. The endpoint's existing suppressError
    keeps the detached turn from surfacing an unhandled rejection, matching the documented contract
    that agent turns never affect the response status.

Behavior change. Webhook sessions now display the fully-qualified resolved session key (the
rendered key appears inside it) with a webhook--prefixed principal, instead of a hand-built
<tenant>:agent:<agent>:webhook:<rendered>. This is precisely what makes the session view and the
turn authority agree. Documented in docs/reference/webhooks.mdx.

Third and fourth commits — the delivery origin must name the same channel as the turn scope.
Review of the fix above found the resolved identity and the delivery origin built beside it
describing the conversation on different channels: the endpoint carried the control-plane origin
kind with a JSON-wrapped conversation id, while the origin carried "webhook" and the rendered key.
The runtime treats those two as one authority and rejects the pair wherever they disagree, so the
turn cleared the executor precondition and then fail-closed further in:

  • BackgroundTaskOriginSchema rejects it with "delivery origin must agree with the resolved turn
    authority"
    — that schema gates backgroundTaskManager.promote() and task-extraction capture, so a
    webhook turn could not background a long-running tool.
  • resolveLeaseRequestPrincipal rejects it with "capability lease principal invalid" — a jailed
    session.spawn is denied, which is the drive class this PR exists to enable.
  • The autonomy wiring drops the durable principal, silently disabling durable orchestrate runs for
    webhook turns.

The cron and durable-resume initiators derive the origin from the resolved endpoint for exactly
this reason. This branch now does the same: the webhook surface is named as its own origin kind, the
rendered session key becomes the endpoint's conversation id, and the origin is derived from that
endpoint. The JSON wrapper turned out to be unnecessary for authority — the conversation reference
digests every field length-delimited, so payload data cannot forge a field boundary, and a rendered
key carrying the session-key separator resolves to its own conversation. The endpoint now reads
webhook everywhere an operator looks.

Second commit — observability. Diagnosing this took a raw log grep, which it should not have. A
turn that aborts before recording a session never reaches the session index, so obs.explain
returned session_not_found blaming "a typo, expired, or older than the horizon", and
system-health reported 0 degraded while the entire pipeline was down — a healthy-looking report
for a dead deployment. The verdict now names the pre-execution-abort possibility and points at the
evidence such an abort does leave: the ERROR carrying its step and errorKind in the daemon log.

Related Issue

N/A: found in production rather than filed — every ADO work-item webhook silently aborted before its
drive started. Happy to open a tracking issue if the linked-issue requirement for behavior/security
changes should apply here rather than the N/A escape.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactor

Checklist

  • Targeted checks for the changed area pass — 94 tests across the four affected files, on the
    current rebase base
  • Full repository validation passes (pnpm validate) — see the note below; run on the pre-rebase
    commit, with only pre-existing environment failures. After the follow-up commits: docs:check,
    build (type-check) and the @comis/daemon (6601 tests) + @comis/orchestrator (1052 tests)
    projects pass on the rebased head, plus test/architecture/generic-runtime-boundary.test.ts
  • New or changed behavior has a test that demonstrated the RED state before the production change
  • Documentation updated (docs/reference/webhooks.mdx)
  • No secrets or credentials committed
  • Security implications considered — principal binding, delimiter safety, and trust level are
    covered above and pinned by tests
  • The change is focused on one concern

On pnpm validate: docs:check, build:clean, cycles, cycles:refs and lint:security pass.
test:coverage reports 60 failures across 15 files, all pre-existing and environmental, none in
a package this diff touches: 14 files are real-bwrap jail/sandbox/egress *.linux.test.ts needing
a privileged bwrap plus network egress, and @comis/web src/views/security.test.ts fails 23/23 on
TypeError: localStorage.removeItem is not a function. Verified by running the same files on
unmodified main in a clean worktree — identical failures. This run predates the rebase onto
current main; I re-ran only the targeted 94 tests after rebasing and can re-run the full gate on
request.

RED Test Proof

1. The webhook turn carries no canonical turn scope
packages/daemon/src/wiring/setup-gateway-routes.test.ts

Before the production patch:

× resolves a canonical turn scope for the webhook turn (the executor's context-authority precondition)
× isolates webhook conversations per rendered session key

AssertionError: the webhook turn must carry a canonical turn scope: expected undefined to be defined
 ❯ src/wiring/setup-gateway-routes.test.ts:271:77
    270|     const turnScope = observedContexts[0]?.turnScope;
    271|     expect(turnScope, "the webhook turn must carry a canonical turn sc…

 Tests  2 failed | 53 passed (55)

After: Tests 55 passed (55).

The assertion encodes the exact production precondition — scope present and a constructible
conversation ref — so it fails for the same reason the live turn did.

2. The session_not_found verdict hides a pre-execution abort
packages/daemon/src/api/obs-handlers/obs-explain.test.ts

Before the production patch (source stashed, test kept):

× an unresolvable traceId yields a session_not_found marker, not a silent empty report

AssertionError: expected 'traceId did not resolve to any sessio…' to match /abort|before .*record|pre-execution/i

 Tests  1 failed | 1 passed | 27 skipped (29)

After: Tests 29 passed (29).

3. New helper unit testsgateway-session-principal.test.ts: per-subject conversation
isolation, a mapping-bound principal shared across subjects, mappings separated from each other and
from the gateway- principal namespace, and a delimiter-bearing rendered key (azdo:1:peer:forged"])
staying intact inside the conversation identity.

4. The delivery origin and the turn scope named different channels
packages/daemon/src/wiring/setup-gateway-routes.test.ts

Before the production patch (the test commit is separate, so this RED state is reproducible from
e44eaf3 alone):

× binds the webhook delivery origin to the SAME endpoint its turn scope resolved

AssertionError: expected 'webhook' to be 'control-plane' // Object.is equality
 ❯ src/wiring/setup-gateway-routes.test.ts:349:40

 Tests  1 failed | 55 passed (56)

After: Tests 56 passed (56). The assertion checks the pair field by field and parses it through
BackgroundTaskOriginSchema — the same schema the background-task manager uses — so it fails for the
same reason the downstream seams would.

Evidence and Residual Risk

  • Contribution path: integration — the daemon's own /hooks/* webhook surface and the turn
    identity it hands the executor.
  • Evidence level: deterministic tests (RED→GREEN above) plus a live production run. The live
    failure was observed end-to-end: poller POST → HTTP 200 {"received":true}, session
    …:webhook:azdo:<id> created, workspace and tools assembled, InputSecurityGuard pass, then the
    context-authority ERROR ~25ms in, with no tmux session, no branch and no PR as ground truth. The
    counterpart mechanism was confirmed live by driving the same task through /api/chat, whose
    executeAgent path resolves a turn identity: it completed normally and produced a fully-qualified
    resolved session key, isolating the missing turnScope as the sole difference.
  • Tested profile: Linux (Ubuntu, kernel 6.8), Node 22, unsafeDisableSandbox: true on the live
    host, provider openai-codex via OAuth, tenant default, existing ~/.comis state; unit tests on
    the same host against the current rebase base.
  • Residual risk:
    • The live evidence exercises one mapping shape (action: "agent" with a templated sessionKey).
      Preset mappings (gmail, github) are covered by unit tests only.
    • The changed display session key is asserted by shape (^webhook-[a-f0-9]{64}$, and the rendered
      key appearing inside the resolved key) rather than pinned to an exact literal, so a future change
      to conversation formatting would not fail these tests.
    • The agreement between the delivery origin and the turn scope is proven by unit test against the
      real schema, not by a live drive: the three downstream seams (background promotion, capability
      lease principal, durable principal) are each verified through the predicate they enforce rather
      than by exercising a webhook-triggered background task, jailed spawn and durable run live.
    • Any operator tooling that string-matches the old
      <tenant>:agent:<agent>:webhook:<rendered> session key will need updating; the rendered key is
      still a substring, so grep-by-subject keeps working.
    • Not proven: behavior under two concurrent deliveries for the same subject (isolation across
      different subjects is tested), and the deliver: true channel-delivery branch of a mapping.

@roeikosh
roeikosh requested a review from anconina as a code owner July 26, 2026 11:54
Roei K and others added 4 commits July 26, 2026 15:55
A webhook agent action dispatches straight to the executor, bypassing the
orchestrator inbound pipeline that resolves turn identity for real channels.
The executor requires a canonical turn scope and fail-closes without one, so
every webhook-triggered turn aborted at step:"context-authority" before the
agent ran.

Resolve the turn identity in the webhook handler, as the gateway, cron,
heartbeat and notification turn initiators already do, and thread its turn
scope into the request context. The resolved identity is the single authority
for the turn: its display session key and turn scope describe the same
conversation, so the session view and the memory/context authority cannot
diverge.

The principal is bound to the operator-configured mapping (hashed for
delimiter safety) and the rendered session key is JSON-encoded into the
conversation identity, so a crafted payload can neither forge nor widen a
conversation's authority even though the template interpolates request data.
Per-subject conversation isolation is preserved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A turn that fail-closes before it records a session never reaches the session
index, so `obs.explain` cannot distinguish a dead trigger from a mistyped
reference. The verdict offered only "typo, expired, or older than the horizon",
which sends an operator to re-check the reference while a real failure goes
unexamined — the live cost when a whole webhook-driven pipeline was down and
explain reported nothing but a missing ref.

Name the pre-execution-abort possibility in the detail and add a next step that
routes to the evidence such an abort DOES leave: the ERROR carrying its step and
errorKind in the daemon log.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… resolved

A webhook turn's delivery origin and turn scope are one authority downstream.
BackgroundTaskOriginSchema rejects the pair when they disagree ("delivery origin
must agree with the resolved turn authority"), which gates background-task
promotion and task extraction; the capability lease principal rejects it for a
jailed session.spawn; and the autonomy wiring drops the durable principal. A turn
built with a hand-made origin therefore starts and then fail-closes at each of
those seams.

Assert the observed context's origin agrees with its scope field by field AND
that the pair parses through the schema the background-task manager uses, so the
agreement is pinned where the wiring builds it rather than in a helper.
…ts turn

The resolved turn identity described the conversation on a different channel than
the delivery origin built beside it: the endpoint carried the control-plane origin
kind and a JSON-wrapped conversation id, while the origin carried "webhook" and
the rendered key. Every consumer that pairs the two rejects that turn — background
task promotion and task extraction (BackgroundTaskOriginSchema), the capability
lease principal for a jailed session.spawn, and the durable principal in the
autonomy wiring — so a webhook turn cleared the executor precondition and then
fail-closed at each of those seams.

Name the webhook surface as its own origin kind and use the rendered session key
as the endpoint's conversation id, then derive the delivery origin FROM the
resolved endpoint, as the cron and durable-resume initiators do. The JSON wrapper
is unnecessary for authority: the conversation reference digests each field
length-delimited, so payload data cannot forge a field boundary — a rendered key
carrying the session-key separator resolves to its own conversation. The endpoint
now reads "webhook" everywhere an operator looks, and the session key keeps the
rendered key as a substring.
@anconina
anconina force-pushed the fix/webhook-turn-scope-authority branch from ac11065 to 4ca87af Compare July 26, 2026 13:06
@anconina
anconina merged commit 11a5253 into comisai:main Jul 26, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants