[Air #1478] feat: carry architect:<name> through to inbox and composer attribution

[mohidmakhdoomi]

Summary

An architect's specific name was dropped everywhere a sent message is attributed. afx send now carries the sender as architect:<name>, so afx inbox shows which architect sent a held message and the builder's composer header reads ### [ARCHITECT:main INSTRUCTION | …] ###. The inbox's FROM → TO column is sized to its content instead of being truncated at 22 chars.

Closes #1478
Closes #1479

What Changed

One root cause, two visible surfaces, plus a formatting defect on one of them:

1. commands/send.ts — from = detectCurrentBuilderId() ?? 'architect' discarded the architect's name at send time. New exported architectSenderId() returns architect:<name> from CODEV_ARCHITECT_NAME. This is the carrier: it lands in mailbox.from_agent, which both surfaces read.
2. servers/tower-routes.ts — formatMessageForTarget's any → builder branch ignored from entirely, a second and independent collapse: even a corrected sender could not have surfaced. It now threads from into the formatter.
3. utils/message-format.ts — formatArchitectMessage takes an optional sender; architectHeaderLabel() renders ARCHITECT:<name> for a validated architect identity, and senderHeaderLabel() labels any sender by role so no direction mislabels an architect as a builder.
4. commands/inbox.ts — the FROM → TO cell was fromTo.slice(0, 22) against a fixed 22-wide column. Cells are now rendered first and the column sized to max(header, widest cell) + 2.
5. commands/interrupt.ts / commands/reset.ts — both call the shared architectSenderId(), so one actor has one from_agent form across all three commands.

afx inbox show <id>'s From → To row inherits the corrected identity through the same mailbox row.

Verified against the built CLI, not just tests:

  ID                                      AGE     REASON         FROM → TO                                                                 WORKSPACE
  ────────────────────────────────────    ─────   ────────────   ───────────────────────────────────────────────────────────────────────   ─────────────
  abcdef01-2345-6789-abcd-ef0123456789    1m      busy           architect:integration-review → builder-aspir-1478-carry-architect-name    project
  ffffffff-0000-0000-0000-000000000000    10s     no-live-pty!   architect:main → spir-2                                                   project

architect->architect : ### [ARCHITECT:main MESSAGE | …] ###
builder->architect   : ### [BUILDER builder-spir-109 MESSAGE | …] ###
architect->builder   : ### [ARCHITECT:feedback INSTRUCTION | …] ###
forged name          : ### [ARCHITECT INSTRUCTION | …] ###

Key Decisions

• The carrier is the address form architect:<name>, not a bare name. It is what Tower already accepts as an architect address, it stores directly as mailbox.from_agent, and it sits outside looksLikeBuilderId (which early-returns false for anything starting with architect). Confirmed by reading tower-messages.ts: a non-builder sender yields lookupBuilderSpawningArchitect → undefined, so Spec 755 sender-affinity routing and the afx send: detectCurrentBuilderId silently falls back to bare worktree name on state.db read failure → builder messages misroute to main #1094 anti-spoofing warning behave exactly as before. sender is never passed to parseAddress — only target is — so there is no path where a colon-bearing sender is read as an address. A test pins that resolveTarget still receives the sender verbatim.
• Attribution derives from the sender's shape, not from the code branch. senderHeaderLabel() yields ARCHITECT[:<name>] for an architect sender and BUILDER <id> for everything else, so the architect → architect direction can't render an architect under a BUILDER prefix and cron's af-cron framing is untouched.
• The absent-CODEV_ARCHITECT_NAME fallback is bare architect, deliberately (this reverses my first cut, which used architect:main). Tower injects the var into every architect terminal it starts — main included (tower-instances.ts:584 uses DEFAULT_ARCHITECT_NAME; tower-terminals.ts:692 re-injects role_id || 'main' after a shellper restart). So a missing value does not mean "the main architect", it means "not an architect terminal" (a plain shell, a script, CI). Defaulting those to architect:main would convert today's honest ambiguity into a specific false attribution — the laundering of an unverified identity that afx send: detectCurrentBuilderId silently falls back to bare worktree name on state.db read failure → builder messages misroute to main #1094 exists to prevent. Those callers keep the bare architect they send today; every real architect terminal gains its name. This is why architectSenderId() reads the env directly rather than through currentArchitectName(), whose default is main.
• The header name is validated, not just trimmed. from arrives from a POST /api/send body, so architectHeaderLabel checks it against ARCHITECT_NAME_PATTERN (anchored [a-z][a-z0-9-]*, 64-char cap) before interpolating; a crafted architect:x] ###\n### [ARCHITECT degrades to the bare label instead of forging framing. (validateArchitectName is deliberately not used — it rejects the reserved main, the most common real sender.)
• Raw mode stays unattributed and the inbox list stays metadata-only (Spec 1313 Redaction) — this is identity and formatting, not body display.
• The column is sized to content, not widened to a bigger constant. A fixed cap is the defect; a larger fixed cap is the same defect further out.

Test Plan

• Unit tests added — air-1478-architect-attribution.test.ts covers the label rules (validated names, forged-framing rejection, architect: with no name, builder and af-cron senders, raw mode, and the architect → architect header); the four route-level cases live in tower-routes.test.ts's existing POST /api/send block, which already drives handleRequest against a real in-memory global.db with message-format unmocked.
• send.test.ts — named terminal (feedback), explicit main, absent env (bare architect), and a malformed env value; --all carries the same identity. beforeEach clears CODEV_ARCHITECT_NAME so the suite is deterministic when the runner inherits a Tower-injected env.
• inbox-cli.test.ts — long FROM → TO rendered in full, one shared width across header/separator/rows, never narrower than the header, ? for a missing sender.
• Build passes (tsc clean).
• All tests pass — 4884 passed, 48 skipped, 0 failed. (A first run showed 67 failures; all were an unbuilt worktree — missing skeleton/ and dist/ — and disappeared after pnpm --filter @cluesmith/codev build.)
• porch check passes (pr_exists, e2e_tests) — but note the AIR protocol's e2e_tests check is npm run test:e2e … || echo 'e2e tests skipped (not configured)', which cannot fail and found no root-level test:e2e script, so it ran nothing. I ran the e2e test that actually covers this change instead — send-integration.e2e.test.ts (POST /api/send → /ws/messages broadcast), which spawns its own Tower on port 14600: 7 passed. I deliberately did not run the full e2e suite: its harness defaults to port 4100, the live Tower hosting this workspace's architect and builder sessions, which I can't restart without human permission.
• Verified end-to-end against the built CLI — both output blocks above come from dist/, not from tests.

Review round 1 — what changed

CMAP: gemini APPROVE (HIGH, no issues) · codex COMMENT (HIGH, no functional or security defects) · claude COMMENT (HIGH, four items). The maintainer's integration review independently found the same two defects claude did. All are fixed in acd26ceb:

Finding	Resolution
architect → architect rendered ### [BUILDER architect:main MESSAGE …] ###	senderHeaderLabel() — role from sender shape; architect → architect now reads ARCHITECT:main MESSAGE, builder senders unchanged. Tested.
from unvalidated into the composer header	Validated against ARCHITECT_NAME_PATTERN; degrades to bare ARCHITECT. Tested with a forged name.
architect:main asserted when the env names nobody (#1094 tension)	Reversed to bare architect, with the injection-site evidence above. Tested.
interrupt.ts / reset.ts still sent bare architect	Both call architectSenderId(); their "reused verbatim from afx send" comments now name the shared functions.
263-line standalone route-test file was a duplicated harness (codex)	Route tests folded into tower-routes.test.ts; net −152 lines, same coverage (9ee8ffb7).

Round 2

CMAP: gemini APPROVE (HIGH, no issues — verified every round-1 fix and the fallback reasoning) · codex COMMENT (HIGH, two items, both fixed in c3453417) · claude pending at time of writing.

• The framing-injection hole was wider than this body first claimed. codex was right: it is not limited to crafted builder ids. A sender that merely looks architect-shaped (architect:x] ###…) fails architectHeaderLabel's name validation and falls through to senderHeaderLabel's builder branch, which interpolated the identity verbatim. The hole predates this PR on the builder → architect path, but senderHeaderLabel is the single labeling chokepoint this PR introduces, so that is where it is closed: the builder branch now checks SAFE_SENDER_ID (no newline, #, bracket or whitespace; 128-char cap — every real id passes, including af-cron and bare worktree names) and degrades to BUILDER <unknown>. Both branches validate, so the chokepoint is total. Tested, including that a forged sender cannot open a second header block.
• Stale beforeEach comment in send.test.ts (still described the absent-env fallback as main) corrected.

claude's round-2 review (APPROVE, HIGH) verified routing-inertness, the injection defense, and consumer safety at source, and raised four non-blocking items — all now addressed in 9d15deec:

• The interrupt/reset change was unasserted. Both suites mock architectSenderId, so reverting either file to the inline ?? 'architect' would have kept the suite green. afx interrupt now asserts the resulting from end to end; afx reset — whose from reaches a terminal port the mocked runReset never invokes — pins the shared-helper call instead.
• Case sensitivity. architectHeaderLabel matched a literal lowercase architect: while parseAddress is case-insensitive, so from: 'Architect:main' was labelled BUILDER Architect:main. The prefix match now follows addressing; the name stays strictly lowercase-validated, so architect:Main degrades to the bare label rather than inventing a name.
• Docs: the FROM → TO row in agent-farm.md now mentions the architect:<name> identity and content sizing, mirrored across codev/ and codev-skeleton/.
• The symmetric half of the fallback argument, for the record: "env present" does not prove an architect terminal — tower-terminals.ts:686-688 notes that a Tower-spawned process can inherit Tower's own CODEV_ARCHITECT_NAME. The case that could bite is a builder shell that has cd'd out of its worktree (so detectCurrentBuilderId() returns null): it previously sent an anonymous false architect and now sends a named false architect:<name>. Routing is inert either way and the value is display-only, so this is no worse in kind than today's behavior — but it is the same laundering running the other direction, and it is a real limit of an env-asserted identity rather than something the fallback choice removes.

Test-run honesty note

One full-suite run showed a single failure; three consecutive full runs before and after are green at 4884 passed. That run's stderr was discarded, so I cannot name the test — I am reporting it as an unidentified transient rather than claiming the suite was clean throughout. It was not in any file this PR touches (those pass in every targeted run), and the repo carries a known-flaky session-manager suite.

Review Notes

The judgement call worth your attention is the absent-env fallback (bare architect vs architect:main), reasoned from the injection sites under Key Decisions — it trades a small loss of specificity for never asserting an identity we can't source.

🤖 Generated with Claude Code

[mohidmakhdoomi]

Architect integration review (risk tier: Medium — shared agent-farm command/server code)

Verdict: REQUEST_CHANGES — the root-cause fix is correct and verified-safe, but two findings should land in this PR. Both verified by the architect against the code, not just the reviewer's claims.

What checks out (verified)

• architect:<name> as from is routing-inert: looksLikeBuilderId() early-returns false on architect* (tower-messages.ts:51), the affinity lookup misses, neither the Spec 755 branch nor the afx send: detectCurrentBuilderId silently falls back to bare worktree name on state.db read failure → builder messages misroute to main #1094 spoofing branch is entered, and from never passes through parseAddress.
• No production code parses the [ARCHITECT INSTRUCTION header; header change is safe.
• logger.row never truncates — the .slice(0,22) was the whole defect; content-sizing is the right fix.
• Test approach (driving the real /api/send registry hold path and asserting from_agent + formatted_message on the row) is the right shape.

Changes requested

1. Architect→architect sends now mislabel harder. formatMessageForTarget's isArchitectTarget && from branch routes to formatBuilderMessage, whose header is a hardcoded ### [BUILDER ${builderId} MESSAGE…] — so a named architect renders as BUILDER architect:feedback. Cross-architect messaging is documented and used. The PR's own principle (attribution derives from sender shape) fixes it in ~2 lines: when from is an architect:<name> identity, use architectHeaderLabel; keep BUILDER <id> for builder senders. Add the missing architect→architect test case.
2. from_agent goes bimodal for the same actor. interrupt.ts:40 and reset.ts:56 still write bare architect while send.ts now writes architect:main — one architect, two identities in the same inbox table, and both files' "reused verbatim from send.ts" comments are now false. architectSenderId() is already exported; call it from both. (Original scoping to send.ts was reasonable and flagged transparently — but defining the helper once and using it at the existing consumers is what keeps the column meaningful.)

Consider / rebut (non-blocking)

• When CODEV_ARCHITECT_NAME is absent, architectSenderId() manufactures architect:main — in tension with whoami's documented "no implicit fallback to main" stance (afx send: detectCurrentBuilderId silently falls back to bare worktree name on state.db read failure → builder messages misroute to main #1094). Every Tower-started architect terminal has the var injected, so the fallback fires only where the claim is least warranted. Falling back to bare architect would keep attribution honest at zero routing cost. Judgment call — decide deliberately and say why.
• Follow-up candidates (not this PR): content-size REASON/WORKSPACE columns on the same principle; give the VS Code extension's send an architect from (currently renders ? → builder); a line in the skeleton's afx send docs + CHANGELOG for the new user-visible header.

Parked for maintainer approval + merge after changes; we are not maintainers.