Skip to content

SAFETY: a gate-approval notification crosses workspaces and can tell a builder its gate was approved when it was not #264

Description

@pseudoseed

What happened

While running spec 250 phase 10's Playwright suite, the builder spir-250 in
workspace /Users/chris/dev/codev-1455 received, twice:

Gate pr approved — please run porch next to advance.

No gate in that project was approved. The gate that was approved belonged to a
throwaway porch project inside a Playwright fixture's temp workspace under
$TMPDIR, created and destroyed by the test.

This is a safety defect rather than a noisy one. A gate is the human-authority
boundary: every other false notification wastes a turn, this one, if an agent
trusts it, advances a project through a gate no human approved. The
--a-human-explicitly-approved-this apparatus exists to stop exactly that, and a
notification that arrives without it is a channel around it.

1. Where the recipient is resolved, and on what

Two mechanisms compose into a cross-workspace delivery.

The target is the bare porch PROJECT ID, not a builder id and not a
workspace-qualified address.
packages/codev/src/commands/porch/index.ts:1267:

notifyTerminal({
  target: state.id,                      // <- the porch project id, e.g. "250"
  message: gateApprovedMessage(gateName),
  worktreeDir: workspaceRoot,
});

which runs afx send <projectId> "<message>" --raw
(packages/codev/src/commands/porch/notify.ts:66-71).

The workspace is taken from the SENDING PROCESS'S CWD, not from the project
whose gate was approved.
packages/codev/src/agent-farm/commands/send.ts:96-113
walks up from process.cwd(): a path matching ^(.+)/\.builders/[^/]+ yields that
prefix as the workspace, otherwise it walks up looking for .codev/config.json or
.git.

Within that workspace the agent is matched by TAIL, with leading zeros
stripped.
packages/codev/src/agent-farm/servers/tower-messages.ts:387-392 (live
terminals) and :510-511 (registry fallback):

const strippedAgent = stripLeadingZeros(agent).toLowerCase();
if (builderId.toLowerCase().endsWith(`-${strippedAgent}`)) { /* match */ }

So project id 250 addresses builder spir-250 — and pir-250, and
bugfix-250 — wherever exactly one such id exists in the resolved workspace.

Neither hop carries the identity of the workspace whose status.yaml was
actually written. A porch approval for a project in workspace A, run by a process
whose cwd resolves to workspace B, notifies a builder in workspace B whose id
merely ends in -<projectId>.

2. Porch state was NOT changed, and the message was false

Two independent checks, both taken after the messages arrived:

$ python3 -c "read codev/projects/250-*/status.yaml"
phase: implement | plan phase: phase_10
  spec-approval -> approved
  plan-approval -> approved
  pr -> pending          <-- the gate the message named
  verify-approval -> pending

$ porch next 250
status: tasks | phase: implement | plan_phase: phase_10

porch next still returns the phase the builder was already on. The message was
false in the only sense that matters: the named gate was not approved for the
project the recipient owns.

3. Correct behaviour on receipt, which is also the mitigation

Verify against porch and refuse to advance. The builder did: it read
status.yaml directly, ran porch next, found pr still pending and the phase
unchanged, said so, and continued the phase it was on. It did not run
porch done, and no state moved.

That is the only reason this is a nuisance rather than an incident, and it should
not be the only control. A notification is a hint; porch state is the fact. The
message text itself invites the opposite — "please run porch next to advance"
is an instruction, not a hint, and it carries no project id, no workspace, and
nothing a recipient can check it against.

4. Reproducer

packages/codev/src/__tests__/e2e/spec-250-approval.spec.ts (spec 250, phase 10),
with packages/codev/src/__tests__/e2e/spec-250-agent-host.ts. Its third test
approves a gate through codev-agent's capability path, which calls
porch approve, which fires this notification.

Minimal shape:

  1. Create a porch project in a temp workspace with a .codev/config.json and a
    pending gate named pr, with a project id whose digits match the tail of a
    builder id registered in a DIFFERENT, live workspace (here 250 vs
    spir-250).
  2. Approve that gate through porch approve from a process whose cwd resolves —
    by the send.ts rules above — to the live workspace.
  3. The live workspace's builder receives "Gate pr approved — please run
    porch next to advance."

Note notifyTerminal no-ops under a vitest runner
(isUnderTestRunner(), notify.ts:62), so the sibling vitest e2e
(spec-250-t3code-approval.e2e.test.ts) does the same approval and sends
nothing. Playwright sets no VITEST_* variables, so it does.

Suggested direction (not implemented here)

Deliberately not fixed in spec 250 — it is porch/tower, and folding it into a
fork PR would put an unrelated change in it.

  • Address the builder by its canonical builder id, resolved from the project's
    own workspace, rather than by the project id plus the sender's cwd.
  • Carry the workspace path and project id in the message, so a recipient can
    check it is the addressee before acting.
  • Consider whether the tail match belongs on this path at all. It is a
    convenience for a human typing afx send 250; on an authority-adjacent
    notification it is a way to reach the wrong agent silently.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/porchProtocol orchestrator

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions