Uh oh!
There was an error while loading. Please reload this page.
feat(branch): tps branch init --agent <id> persists agentId in conf (ops-hs19) - #282
Conversation
…ops-hs19)
Adds a --agent flag to `tps branch init` that persists the local agent
identity in branch.conf.json. The branch daemon reads conf.agentId via
the existing getLocalAgentId() precedence (TPS_AGENT_ID env → conf.agentId
→ hostname fragment) so subsequent `tps branch start` invocations pick up
the right identity without needing a per-launch env var.
## Problem
The branch daemon's incoming-mail handler routes by `body.to` if a maildir
for that name already exists; else it falls back to `localAgentId`. When
`localAgentId` is computed from `hostname()` (the default when neither
TPS_AGENT_ID env nor conf.agentId is set), this produces an inconsistent
first-message-fallback pattern:
- First message for `body.to=reed` arrives → `~/.tps/mail/reed/` doesn't
exist yet → falls back to `~/.tps/mail/tps-reed/` (the hostname).
- The user runs `tps mail check reed` (creating `~/.tps/mail/reed/`).
- Second message for `body.to=reed` arrives → `inboxExists("reed")` is now
true → lands at `~/.tps/mail/reed/`.
Result: two maildirs for the same logical agent, mail split across them,
watcher consuming only one path.
Surfaced live on tps-reed during Reed Phase 2 dogfood 2026-05-16. Filed as
ops-hs19.
## Fix
`tps branch init --agent reed --listen 33744 --host localhost --transport ws`
writes `{agentId: "reed"}` to `branch.conf.json`. The daemon's
`getLocalAgentId()` already reads conf.agentId at position 2 in its
precedence chain, so no daemon-side code change is needed. The path is
end-to-end consistent: first message and all subsequent messages route to
`~/.tps/mail/<agentId>/`.
## Tests
- New: `writeBranchConf persists agentId when --agent supplied`
- New: `writeBranchConf omits agentId when not supplied (back-compat)`
- Existing 5 branch tests pass.
## Migration for existing branch offices
```sh
# On the VM:
jq '. + {agentId: "<your-agent>"}' ~/.tps/branch.conf.json > /tmp/c.json \
&& mv /tmp/c.json ~/.tps/branch.conf.json
tps branch stop && tps branch start
```
Applied on tps-reed today; PING-PONG verified mail now lands at /reed/
consistently from message #1.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tps-kern
left a comment
There was a problem hiding this comment.
Architecture review — PR #282
Scope assessed
Three files, small surface: adds --agent <id> flag to tps branch init, persists agentId to branch.conf.json, zero daemon-side changes needed.
Findings
Precedence chain: ✅ Preserved.
getLocalAgentId() precedence is TPS_AGENT_ID env → conf.agentId → hostname. The PR adds agentId to the conf, which sits at position 2 — below env, above hostname. The existing comment at branch.ts:243 documents this correctly. No reordering, no ambiguity. If someone sets TPS_AGENT_ID=override at tps branch start time, it still wins over whatever was persisted in conf. The env override path remains the escape hatch.
Back-compat: ✅ Clean.
agentId is optional throughout: optional in BranchArgs, optional trailing param in writeBranchConf, optional in readBranchConf return type. When --agent is omitted, no key is written to conf (test confirms conf.agentId is undefined). The daemon's getLocalAgentId() already handles a missing conf.agentId by falling through to hostname. Existing branch offices that never ran --agent continue to work identically.
Caller impact: ✅ None.
writeBranchConf was previously a private (non-exported) function with a single internal caller: runInit. That caller is updated to pass args.agent as the 5th arg. The export addition is for testability only — the new tests import it directly. No other callers exist in the codebase. The trailing-optional-param pattern means even if someone added a 4-arg call, it would still compile and work.
Test coverage: ✅ Covers both paths.
writeBranchConf persists agentId when --agent supplied (ops-hs19)— verifiesagentId: "reed"is written to confwriteBranchConf omits agentId when not supplied (back-compat)— verifies the key is absent when flag is omitted
Both tests validate the full conf shape, not just agentId. The 3-arg call in the second test proves the trailing-optional signature works without the new param.
Design rationale: ✅ Correct approach.
The problem (ops-hs19) is a genuine maildir-splitting bug: when localAgentId falls back to hostname() and body.to differs, the first message routes to ~/.tps/mail/<hostname>/ while subsequent messages route to ~/.tps/mail/<body.to>/ once that directory exists. Persisting agentId at init time resolves the root cause — the daemon always routes to the same maildir from message #1. The fix is at the right layer: the init/persist path, not the daemon routing logic, because the daemon already has the correct precedence chain.
Usage string: ✅ Updated.
The tps branch init usage string now includes [--agent <id>]. Matches the actual flag.
Note
Verified: --agent persists in branch.conf.json; existing precedence env → conf → hostname unchanged; back-compat preserved when --agent omitted; trailing-optional agentId param breaks zero callers (private function, single internal caller updated). Daemon-side getLocalAgentId() already reads conf.agentId at position 2 — zero daemon changes, which is the right outcome for an init-time config fix.
APPROVE
tps-sherlock
left a comment
There was a problem hiding this comment.
Security review — PR #282
Scope assessed
Three files: packages/cli/bin/tps.ts, packages/cli/src/commands/branch.ts, packages/cli/test/branch.test.ts.
Findings
1. agentId is local-only — PASSagentId is persisted to branch.conf.json and consumed only by getLocalAgentId() inside the daemon's runBranch(). It is not included in the join token URL (buildJoinToken() does not transmit agentId). No trust-boundary bypass risk.
2. Precedence env→conf→hostname preserved — PASSgetLocalAgentId() explicitly checks process.env.TPS_AGENT_ID first, then conf.agentId, then hostname(). No regression. The env override remains dominant, so there is no auth-bypass class here.
3. AgentId sanitization downstream — PASS
The maildir basename derived from agentId is gated by assertValidAgentId, which delegates to sanitizeIdentifier and demands an exact round-trip:
constsafe=sanitizeIdentifier(agent);if(!agent||safe!==agent){thrownewError(`Invalid agent id: ${agent}`);}Path traversal (../, shell chars, null bytes, etc.) are all rejected before the maildir path is assembled. Quirky inputs cannot escape.
4. Back-compat when --agent omitted — PASSwriteBranchConf accepts an optional agentId?: string. When omitted, conf.agentId is absent or undefined in the JSON. The new unit test confirms the field is absent in the legacy path. No breakage.
Note
Verified: --agent persists in branch.conf.json; existing precedence env → conf → hostname unchanged; back-compat preserved when --agent omitted. All threat-model questions answered satisfactorily.
APPROVE
Summary
Adds a
--agent <id>flag totps branch initthat persists the local agent identity tobranch.conf.json. Closes the maildir naming inconsistency surfaced during Reed Phase 2 dogfood: without an explicit agentId, the branch daemon's mail handler falls back tohostname()for the first message, then routes tobody.tofor subsequent messages once the maildir exists — producing two parallel maildirs for the same logical agent.Problem (from live tps-reed dogfood)
Branch daemon's incoming-mail handler at
branch.ts:323:When
localAgentIddefaults tohostname()(no TPS_AGENT_ID env, no conf.agentId), the first message routes to~/.tps/mail/<hostname>/as fallback. Once any local code creates~/.tps/mail/<body.to>/(e.g., the user runstps mail check reed), subsequent messages route there instead. Mail splits across two maildirs; watchers consuming one path miss messages addressed to the other.Repro on tps-reed 2026-05-16:
body.to=reed→~/.tps/mail/tps-reed/new/(fallback)body.to=reed→~/.tps/mail/reed/new/Filed as ops-hs19.
Fix
tps branch init --agent <id>writes{agentId: "<id>"}tobranch.conf.json. The daemon'sgetLocalAgentId()already reads conf.agentId at position 2 in its precedence chain (env → conf → hostname), so the routing decision is now consistent from message #1: maildir is~/.tps/mail/<agentId>/always.No daemon-side code change required. The fix is purely on the init/persist path.
Tests
writeBranchConf persists agentId when --agent supplied (ops-hs19)— verifies the conf writewriteBranchConf omits agentId when not supplied (back-compat)— verifies existing zero-flag behavior is preservedMigration for existing branch offices
Applied on tps-reed live during testing; PING round-trip verified mail lands at
/reed/consistently from message #1.Updated provisioning recipe
The branch-office provisioning flow (memory:
reference_branch_office_provisioning_recipe.md) gets one more flag on thetps branch initline. Documenting the new shape:Test plan
bun test test/branch.test.ts test/outbox.test.ts— 13 passbun run build— dist regenerated🤖 Generated with Claude Code