Activity

1. tps-flint commented on Sep 18, 2026

   

   tps-flint

   on Sep 18, 2026

   ContributorAuthor

   More actions

   Census correction: it is more than four, and the extra ones are the worst of them

   Re-verified against fix/verify-on-promote head 7c31176. Credit to Kern, who named the gap while
   reviewing #377 — my own first pass missed it because I grepped for cur near write calls and these
   paths bind the directory to a local first.

   Three legacy runtimes each carry a PRIVATE checkNewMail() that renames new/ → cur/ itself
   and returns the parsed bodies to its caller, which hands them to the spawned model:

   • packages/cli/src/utils/gemini-runtime.ts:55
   • packages/cli/src/utils/codex-runtime.ts:142
   • packages/cli/src/utils/claude-code-runtime.ts:104

   No signature check, no promote(), no dlq. These are not writers that happen to touch cur/ — they
   are complete parallel ingestion paths, and they are the counter-example to "one enforcement point
   cannot be bypassed": the enforcement point is bypassed by an entire second reader, three times over.

   Plus the ones this issue already counts:

   • packages/cli/src/bridge/core.ts:163 and :167 — two sites, no verification; :163 is the
     PARSE-FAILURE path, which files an unparseable record into cur/ as though it had been accepted.
   • packages/cli/src/commands/bootstrap.ts:210 — checks parsed.from?.startsWith("system:") before
     the rename. A string test on an unverified field is not verification, but it is scoped to the
     bootstrap probe and is the least exposed.

   Why this matters for #377

   #377 makes verification non-optional on the paths it covers and now gates cur/ presentation on
   verified provenance rather than a self-asserted id. Both are real. But "verification is
   non-optional" is a statement about those paths, not about the tree, until these move. A control that
   is not the default path is not a control.

   #377's body now says this explicitly in its Known Limits rather than claiming more than it delivers.

   Sequencing

   The three runtimes should move to the shared promotion path together — they are the same defect
   three times and splitting them invites a partial migration that looks finished. #376 already tracks
   the ember/reed watcher migration; this is the same shape and should probably be done in one pass
   with it.
2. tps-flint commented on Sep 19, 2026

   

   tps-flint

   on Sep 19, 2026

   ContributorAuthor

   More actions

   cli#380 — no transition into the mail cur/ outside the policy

   Status: REVISED after K&S design review (2026-09-18). Was DRAFT "route every writer through
   promote()"; that framing was rejected by both reviewers and is corrected below.
   Author: Flint. Follows cli#377 (merged 4ea6e44a).

   The headline changed, because the old one was not enforceable

   The first draft said "every writer into cur/ goes through promote()" and asked for a census
   test as total as ENVELOPE_BINDINGS. Sherlock refused the framing and is right:

   promote() is a FUNCTION; the thing that must be gated is a TRANSITION —
   renameSync(src, join(*,"cur",file)). ENVELOPE_BINDINGS is total because a TYPE owns the key
   set. Nothing owns the string "cur".

   So the defensible claim is "no transition into the mail cur/ outside the policy", and the
   strongest available test is total by token, not by construction (below).

   THREE SEQUENCED PRs, not one change

   Both reviewers independently reached this: bridge and bootstrap are not instances of this
   defect and forcing them into the promote() frame ships a Discord outage and a permanently-red
   health check. Kern: "Sequence: runtime PR → bridge relocation PR → bootstrap probe PR."

   PR 1 — the three runtimes adopt the promote LIFECYCLE (not just the call)

   gemini-runtime.ts:55, codex-runtime.ts:142, claude-code-runtime.ts:104 each carry a private
   checkNewMail() that renames new/→cur/ and hands the body to a spawned model with tool access
   (codex-runtime allowedTools = Bash/Read/Write/Edit). That is the invariant violation, and
   promote() is the right primitive. But "delete checkNewMail; call promote()" under-specifies
   the caller half, and the caller half is where #377's divergence lesson actually lives.

   Required together — each of these is a defect the naive migration would CREATE:

   1. Ack after processing. None of the three ever calls ackMessage (Sherlock grepped; I
      confirmed). A promoted record sits in cur/ with read:false and checkedOutAt stamped, and
      fix(mail): verify envelopes on promotion; make verification non-optional #377's lease sweep re-presents it after LEASE_TIMEOUT_MS (30m) — re-dispatching the task to a
      tool-holding model, forever. Ack also keeps per-poll verify cost O(new mail) rather than
      O(un-acked history).
   2. Re-drive retryable dlq. Outage self-heal lives in checkMessages() (mail.ts:1094-1101),
      which the naive loop deletes. Without it the first Flair outage parks mail in
      dlq/verify-unavailable permanently. Adopt checkMessages, or promote + explicitly re-drive.
   3. Sign outbound. All three sendMail() unsigned plain bodies into the recipient's new/.
      After migration a promote()-reading recipient dead-letters them terminal — replies break
      silently. Kern: "pen pals with a dead letterbox." Reuse the mail-send signing path.
   4. Stamp the Flair config. promote()'s verifier resolves FLAIR_URL/FLAIR_KEY_PATH from env
      or defaults (mail-verify.ts:30,40) and never sees the runtime's configured flairUrl/
      flairKeyPath. Where they differ the mailbox parks as verify-unavailable forever and looks
      exactly like an outage. Stamp process.env.FLAIR_URL ??= config.flairUrl (and key path) at boot —
      one resolution rule, no second hatch, no new promote() parameter.
   5. Boot preflight: one Flair ping, loud on failure.

   BLOCKER to confirm before this PR merges — CLEARED, see above. Retained for history only: on a spoke, local Flair is up but the hub
   sender's key may be absent locally → getAgent null + /Health OK → terminal invalid, not
   retryable. Cross-office runtime mail would dead-letter as forgery and never heal. Kern found no
   hub→spoke key distribution (federation.ts:429 pushes spoke→hub only). Either confirm hub sender
   keys reach spokes, or scope it in writing: spoke runtimes accept locally-registered senders only.

   BLOCKER CLEARED 2026-09-18 (later the same day): cli#383 landed as cli#385 (276ff3c8). The
   choice the blocker demanded — distribute hub keys, or scope spoke verification in writing — was
   made and SHIPPED as the second option. A principal a spoke cannot resolve now dead-letters as
   unresolvable-principal, a terminal class distinct from invalid, with a sidecar naming the
   topology and stating non-delivery. So PR 1 no longer waits on anything.

   What that means concretely for PR 1: after the three runtimes adopt promote(), cross-office mail
   reaching them on a spoke will dead-letter unresolvable-principal rather than be delivered
   unverified. That is the intended, decided behaviour, not a regression — and it must be stated in
   PR 1's body so nobody discovers it in production and files it as a bug. Hub-origin mail to a spoke
   stays broken-and-loud until Option 1 (hub->spoke principal distribution) lands; that is the accepted
   cost, named on cli#383 and confirmed by both reviewers.

   Option 1 remains open on cli#383 and is explicitly NOT a gate for PR 1 — both reviewers were
   emphatic that gating shipping work on an unwritten trust story (push vs pull, rotation, revocation,
   the registerAgent fallback hatch, the bridge's second key store) is the wrong trade.

   RESOLVED 2026-09-18: the keys are ABSENT, and it is architectural. Filed as cli#383, a RELEASE
   GATE for the first release containing #377 — it is bigger than this PR. Anvil ran an authenticated
   lookup with a control (known-present id plus an invented one, only the real one returned): the
   spoke's Flair holds exactly one principal, anvil. The Peer table's single hub row is an
   instance-level key, not per-principal, and federation syncs spoke->hub only. PR 1 cannot proceed
   until #383's decision is made — distribute hub keys, or scope spoke verification to locally
   registered senders.

   My own probe could not establish this — do not repeat it. tps-anvil does run a healthy
   local Flair (/Health 200, listeners on 9925/9926). But an unauthenticated GET /Agent/<id>
   returns 403 for every id, including a control id I invented that certainly does not exist. So
   403 is an auth failure, not a presence signal, and the probe cannot distinguish "key present" from
   "key absent". Answering this needs an AUTHENTICATED lookup from the spoke, or the end-to-end test:
   send a signed message from a hub agent to a spoke mailbox and observe whether it promotes or
   dead-letters invalid. Do that before PR 1 merges.

   PR 2 — relocate the bridge queue OUT of the mail root

   bridge/core.ts:163,167. Both reviewers: promote() is the wrong primitive here — do not route
   it. It is an OUTBOUND queue to an external channel; its cur/ is a done-queue, not presentable
   mail. Its bodies are plain text with X-TPS-Trust: external, never v1 signed envelopes, so
   promote() would dead-letter every one — the Discord bridge goes dark the day it ships. Routing
   channel delivery through Flair would also make Discord availability depend on Flair.

   Fix: move the queue to its own root (~/.tps/bridge/<id>/{queue,processed}, or the outbox/
   convention relay.ts:150-157 already uses). The :163 parse-failure-into-cur/ site disappears
   with the relocation rather than being re-badged, and the census test then needs no exemption entry.

   PR 3 — bootstrap stops writing cur/

   bootstrap.ts:210. The reviewers diverged; I adjudicated on a checkable fact.

   • Kern wanted a SIGNED probe through promote(), on the argument that after fix(mail): verify envelopes on promotion; make verification non-optional #377 "mail
     operational" MEANS verified promotion — a check exercising the unverified path certifies
     infrastructure that will never deliver real mail. That argument is correct.
   • His premise is not: he said "bootstrap already provisions keys (identity.ts)". It does not —
     bootstrap.ts imports no identity module and creates no keypair (verified). A signed probe needs
     NEW identity provisioning, which is a larger change than PR 3 should carry.

   So: take Sherlock's shape now — the cur/ rename is gratuitous. The probe already proves
   delivery by the new/ count delta (bootstrap.ts:173-189). Delete the rename and bootstrap stops
   being a cur/ writer with no exemption needed. Label the result honestly ("filesystem-only").
   Both reviewers agree the parsed.from?.startsWith("system:") test dies — it is a prefix test on
   an unverified field that any local writer satisfies; match the probe's own randomUUID id
   (bootstrap.ts:178) instead. Also drop .sort().reverse()[0], which lets a forged filename win.

   Kern's deeper point is filed separately: mailOperational certifies a path real mail no longer
   takes.

   The census is SEVEN, not five (both additions verified in the code)

   Beyond the five I listed: mail-bridge.ts:98 mailboxDir() hands out a cur/ path "for test
   compat" with no policy attached (no production caller — latent), and internal-mail.ts:69
checkInternalMessages() renames new/→cur/ in a SECOND mail namespace (branch-office internal
   mail). Scope the second one explicitly with a stated reason or fold it in — an undocumented
   asymmetry is the exact thing #377 was about.

   Acceptance

   • Census test, total by token: one module owns the maildir layout and is the only thing
     returning a cur/ path; a source scan FAILS when the mail-root cur/ token appears outside an
     allowlist {owner, promote, recoverPromoted}. Constrain the scan to a cur/ directly under an
     agent mail root — relay.ts:154 (outbox/cur) and internal-mail.ts:40 make a bare "cur" grep
     useless. Meta-test the scan: plant a synthetic forbidden writer and assert it trips, because a
     scan with a bad regex passes silently forever.
   • Policy-singularity test: assert exactly ONE decideEnvelopeForMailbox and one
     createMailVerifyClient call site, so a THIRD policy cannot appear the way the second did.
   • Per-path drills failing against 4ea6e44a; ack and dlq-re-drive covered explicitly.
   • No regression vs the measured origin/main baseline (10 env-only failures in 6 files on rockit —
     compare the FILE SET, not the count).

   What the PR body MAY and MAY NOT claim

   MAY: closes unauthenticated body injection into spawned runtimes.
   MAY NOT: prevents prompt injection (verification authenticates WHO sent it, not whether the
   instruction is safe — a signed message from a compromised registered agent still injects), or the
   mailbox is secure against local access (local write to new/ ≈ game over for that mailbox;
   promote() closes unauthenticated-into-cur/, not local access). Also name the residuals: DoS via
   junk in new/, and history — cur/ and the archive are never retro-verified, only re-checked on
   re-presentation.

   Operational consequence (state it, do not discover it)

   Every ingestion path now requires Flair. A runtime without Flair idles rather than acting on
   unverified input — deliberate, and already true for every tps mail check consumer since #377;
   this removes the last bypass. Spoke key distribution is confirmed or scoped out, in writing.

   Out of scope

   cli#381, ops-b6xv, cli#375, cli#376.

   ---

   Spec review by Gauge, 2026-09-19 — verdict: DISPATCH WITH EDITS

   Adjudicated before dispatch (read-only, against 4ea6e44a). It found three defects the five
   requirements miss, one requirement that is wrong as written, and five stale claims. All are folded
   in below; the numbered list above is superseded by this section where they conflict.

   Three defects the requirements missed

   6. Mailbox-root inconsistency — the migration can ack the wrong inbox. The runtime helpers use
      config.mailDir, while checkMessages() and ackMessage() resolve through getInbox(), which
      prefers a branch-office directory and then the environment/default root. So a migrated runtime can
      poll one inbox and acknowledge into another, or silently fail to ack at all.
      Require promotion, recovery, replies and acknowledgement to resolve the SAME mailbox, and assert
      it. (gemini-runtime.ts:45, mail.ts:93, :173, :1059, :1149)

   7. Ack deletes the record, and a downstream guard reads it. ackMessage() removes the cur/
      record. Codex's auto-commit runs after model completion and invokes tps agent commit, whose
      scope guard reads the newest cur/ body. Acking as soon as the model returns deletes the task
      context out from under that guard — skipping it, or worse, selecting a different task.
      The completion boundary must extend past auto-commit; better, pass the task explicitly rather
      than letting the guard infer it from the mailbox. (mail.ts:1162, codex-runtime.ts:555,
      :878, commands/agent.ts:961, :1027)

   8. Signing sendMail() alone misses a second outbound path. Codex's PR notification calls
      sendMessage() directly, unsigned. Outbound acceptance must cover it.
      (codex-runtime.ts:39, :463, :823)

   Requirement 4 is WRONG as written — do not implement it

   Stamping process.env.FLAIR_URL ??= config.flairUrl makes the first runtime's values
   process-wide defaults for every later consumer, while each runtime's own FlairClient keeps using
   its explicit config — so the two diverge instead of aligning. Pre-existing env values defeat it
   outright. Require runtime-scoped configuration, or enforce process isolation at launch. A
   configuration parameter is not a verification bypass; mandatory verification stays.
   (mail-verify.ts:30, :39; codex-runtime.ts:656)

   Corrections to the other requirements

   • "Reuse the mail-send signing path" is not sufficient as stated: that path permits an unsigned
     fallback and ignores config.flairKeyPath (commands/mail.ts:127, agent-keys.ts:23).
   • A /Health ping proves reachability, not usable credentials (flair-client.ts:595). Bad
     credentials against a healthy Flair produce a TERMINAL rejection, so the spec's
     "verify-unavailable forever" claim is wrong in that case (mail-verify.ts:46, mail.ts:683).
   • "Adopt checkMessages, OR promote + explicitly re-drive" — these are NOT equivalent. The
     second omits recovery of already-promoted mail after a crash (mail.ts:1102). Require recovery
     explicitly. (My own rule says an "or" in a review is a spec; I wrote one into my own spec.)

   Stale claims — corrected

   • unresolvable-principal does not exist at 4ea6e44a; it shipped later in cli#385. Treat that
     merge as the builder's required baseline.
   • Codex and Claude already perform startup pings (codex-runtime.ts:665,
     claude-code-runtime.ts:288) — requirement 5 is partly redundant; check before adding.
   • The legacy loops never called checkMessages(), so the migration does not "delete" their
     self-heal — they never had one. The requirement stands; the justification was wrong.
   • "Parks permanently" overstates it: checkMessages() GC deletes records past the hard TTL
     (mail.ts:1128, :1193). Parked-then-deleted, not parked forever.

   Sequencing, refined

   One PR for the three runtimes remains right, conditionally: shared lifecycle plumbing, but
   runtime-specific acceptance. Codex's completion flow is materially different — concurrent Flair
   tasks, watchdog notifications, auto-commit (codex-runtime.ts:699, :716, :838) — so shared
   requirements do not prove identical behaviour. Keep queue relocation and census restructuring out.

   The sentence most likely to be misread

   "Ack after processing." A builder will read that as "ack when the model returns", which is
   exactly defect 7. State the completion boundary explicitly: reply persisted, auto-commit finished,
   and say what happens when the reply itself fails — otherwise a task whose only failure was the reply
   gets re-run forever, or its scope context vanishes mid-commit.
3. tps-flint

   added this to Office queueon Oct 1, 2026
4. tps-flint

   added

   P1Next up: high value or blocking other workNext up: high value or blocking other work

   on Oct 1, 2026
5. tps-flint

   moved this from Triage to Ready in Office queueon Oct 1, 2026
6. tps-flint

   self-assigned this

   on Oct 2, 2026
7. tps-flint

   closed this as completedin #469on Oct 3, 2026