# AMQ Bridge Decision Log

This document preserves decisions, open questions, and expert feedback so the roadmap work is not lost.

## Confirmed decisions

### AMQ is transport, not scheduler

AMQ should remain a thin transport. AMQ Bridge owns collaboration semantics:

- protocol envelope
- pending queue
- message lifecycle
- actionability policy
- reply disambiguation
- scheduling into Pi turns
- observability/events

### Default root is user-global

Default AMQ root is:

```text
~/.amq-bridge/mail
```

Reason: Pi sessions may run from different working directories. A cwd-relative `.agent-mail` caused split-brain roots.

Overrides remain:

```bash
PI_AMQ_ROOT=/path/to/shared/mail pi
```

or:

```json
{ "root": ".agent-mail" }
```

### Reply-latest is unsafe

Replying to the latest message is unsafe when multiple actionable messages are pending.

Decision:

```text
If more than one actionable pending message exists, require messageId.
```

Injected AMQ tasks must include explicit message id and thread.

### Single-active injected AMQ task

Default scheduler policy:

```text
one active actionable AMQ task per Pi session
```

Additional messages are queued until the active message is replied to or resolved.

### Housekeeping is non-actionable

Messages like attach, detach, heartbeat, routine status, busy, queued should not trigger Pi work by default.

### Peer content is untrusted

Peer AMQ messages are untrusted data, not user/developer/system instructions.

Injected prompts must say this explicitly and require AMQ tools for peer replies.

### Evidence-first UX

Tool/command outputs must include:

- id
- from
- to
- kind
- subject
- thread
- root
- body preview

Assistant claims are not enough for E2E assertions.

### UX is release-critical

Status/root/id visibility is not polish. It is required for correctness and user trust.

## Open decisions

### `/send` semantics

Problem: current `/send` defaults to `kind=status`, but roadmap says status is non-actionable.

Options:

1. `/send` defaults to actionable `question`.
2. Add `/ask` for actionable requests and `/tell` for FYI/status.
3. Keep `/send`, but require explicit `kind` when ambiguous.

Recommendation from UX review:

```text
Introduce /ask and /tell. Keep /send as compatibility alias with explicit kind warning.
```

### AMQ delivery semantics

Problem: if `drain` is destructive, crash after drain but before durable local queue write can lose messages.

Need decide one:

1. Use AMQ peek/claim/ack if available.
2. Use non-destructive list/read until Bridge queue accepts message.
3. Document at-most-once semantics after drain.
4. Add local WAL immediately after drain and test crash boundary.

This is a release blocker for any reliability claim.

### Global root security posture

Default global root solves cross-cwd, but crosses repos/trust domains.

Need decide warnings and controls:

- show global root warning on attach/status
- support project-local root for sensitive work
- root fingerprint exchange later
- artifact redaction defaults

### Reply fallback behavior

Fallback from failed `amq reply` to normal `send` is useful but risky.

Safe only if original message metadata is known:

- original from
- original thread
- original subject
- original id

If metadata missing, fail closed.

## Expert consensus

### Staff engineering consensus

- Correctness first: explicit ids, durable queue, one-active scheduler.
- Do not build multi-peer before one-peer async safety is solid.
- Delivery contract around AMQ drain/ack must be explicit.

### AI systems consensus

- Prompts alone are insufficient.
- Mechanical guardrails required:
  - scheduler
  - explicit message id
  - untrusted peer wrapper
  - AMQ-only peer replies
- Peer messages can be prompt-injection vectors.

### Gossip/protocol consensus

- Treat messages like distributed events:
  - dedupe
  - lifecycle states
  - stale/late answers
  - supersede/cancel
  - conflict visibility
- Avoid silent overwrite.

### Normal user consensus

Users need to see:

- who am I?
- who is peer?
- what root?
- what message id?
- did send/reply actually happen?
- what should peer run?

Output like `? -> aadil` destroys confidence.

### Pi power-user consensus

Pi-native UX needs:

- richer TUI status line
- `/status` with root/pending/active details
- no stray `Working...` from attach/status
- install/reload/status quickstart
- artifact safety notes

### Security consensus

Release blockers:

- wrong-peer send prevention
- root visibility
- redaction/artifact warnings
- peer content untrusted
- global root cross-repo warning

## Canonical docs

- `docs/roadmap.md` — final roadmap and implementation plan
- `docs/expert-review.md` — expert architecture/security/product synthesis
- `docs/ux-review.md` — UX/DX/TUI/user synthesis
- `docs/decision-log.md` — this durable decision ledger

## Identity architecture decision

User rejected mutating `before_agent_start` system prompt as conflict-prone with other extensions.

Decision:

- Do not mutate the system prompt for AMQ identity.
- Persist attachment identity as Pi session custom entry: `amq-bridge-state`.
- Keep legacy `.amq-bridge/sessions/<key>/state.json` only as cache/migration support for current inbox loop and one-shot smoke flows.
- Add factual context via Pi `context` event as `customType: amq-bridge-context`, `display: false`.
- Provide `amq_bridge_status` tool so agent/user can inspect identity, peers, primary peer, and root.
- Attach is responsible for making the agent aware of identity; no guided skill required.

Current v2 state shape:

```ts
{
  version: 2,
  attached: boolean,
  self?: string,
  root?: string,
  peers: Record<string, { handle: string, attachedAt: string, status?: string }>,
  primaryPeer?: string,
  updatedAt: string
}
```

Detach for now appends `attached:false`, stops watcher, and removes legacy cache. Full protocol detach semantics remain open.

## Dogfood feedback checkpoint

Live `sugar` ↔ `coffe` dogfood, observed by `chai`, added these durable learnings:

- `AMQ for signal, doc for substance` is the best collaboration pattern for long-form work.
- Role/identity confusion is a top UX bug; status must say “You are X, peer is Y”.
- `sent ? -> peer` / `replied ? -> peer` is unacceptable; evidence fields are mandatory.
- Users need delivery/read/pending evidence; otherwise they inspect filesystem or resend.
- Inbox needs newest-first, read/unread state, timestamps, and cleanup.
- Delayed replies are indistinguishable from routing failure without observability.
- Stale local bridge state can send via wrong root/wrong sender; status/send output must surface root/self and fail closed on mismatch.
- `/reload` can break live attached sessions; add dedicated `/reload` E2E to verify extension reload while attached, status badge rendering, and persisted state restore.
- Coffe loop root cause: identity uncertainty plus inbox messages without read/unread/dedupe semantics. Repeated old messages looked actionable forever, causing repeated inbox checks and replies. Required guards: processed message ids, `--unread`, mark-read/resolve, newest-first/timestamps, one-active-message scheduler, and max repeated no-new-message checks.
- Current session shows stale AMQ context (`You are: amq`, peer `go`, root `.agent-mail`). This is separate from coffe's loop but same risk class: stale identity can mislead agents. Context must be suppressible/cleared by detach and guarded by freshness/source checks.
- Dynamic handles can exist as directories while `config.json` still warns unknown; roster/config behavior needs decision.

## Next checkpoint

Before coding more features, implement Phase 1:

1. Fix release-gate scripts and assertions.
2. Add verbose evidence output.
3. Add append/dedupe inbox buffer.
4. Restrict ambiguous replies.
5. Suppress housekeeping auto-turns.
6. Add agent behavior contract to docs/skill.

## Receive path: notify-pull (ADR 0002)

Confirmed — two peer reviews, zero remaining blockers.

Decision: receive = `amq watch` (one-shot, fsnotify) → `amq list --new` (envelope) → `amq read --id` (body on demand). No drain ingestion, no poll loop, no inbox.json buffer. Maildir `new/cur` is read/unread truth; bridge keeps bounded `processedIds` (injection dedupe) + `activeId` (one-active scheduler) in session state. `amq_bridge_resolve` closes the lifecycle; reply always requires explicit id. Send gains `priority`; urgent wake gated on roster source (never handshake). `/amq-bridge send` default kind: status → question. Migration: one-time `cur/` note on first attach (marker file).

Key verified CLI facts that shaped it: watch is one-shot per event (existing | new_message) then exits 0 — respawn is the main loop; `--session` + `--root` mutually exclusive; no `fyi` kind; envelope lacks `to`/`root`; sender-side read receipts unavailable.

### Review-3 clarifications (ADR 0002 §8c, must-fix before coding)

- Watcher key = `${root}:${me}` (owner session is the only injector; secondary = read-only)
- Wake-trust vocabulary: trusted = attach|manual|presence-connected; untrusted = handshake|message|discover|no-source
- ADR 0002 scope = pi-extension receive path; sidecar/echo-peer keep legacy drain (follow-up)
- processedIds/activeId in pi custom state; persist before triggerTurn
- Newest-first display (CLI is oldest-first); migration marker at bridge-dir level, N=20
- WatchEvent.messages = re-listed envelopes; exit codes 0/4/3/signal mapped
- SKILL.md + docs/message-kinds.md alignment added to deliverables

### Detach semantics (ADR 0002 §8c #9)

- Watcher aborted by `${root}:${me}` key; AbortSignal SIGTERMs watch child
- Detach keeps processedIds, resets activeId
- migration marker at bridge-dir level, survives detach rmSync
- Owner detach with secondary sessions attached: mail waits in new/, delivered on next attach (existing event); no watcher promotion

### Completeness sweep findings (ADR 0002 §8c #10-13)

- Owner arbitration: watcher ownership marker `.amq-bridge/watchers/<root-hash>-<me>.owner` (pid+session); first-come owner, secondary read-only, stale-pid takeover. attach + connect share startWatch() helper.
- Reply records processed id (reply-without-read leaves msg in new/) + triggers dequeue.
- Contract assert = required subset; waitForMessages/drainInbox stay as extra transport methods for non-Pi legacy path.
- session_shutdown aborts by (root,me) key + removes owner marker if self.

### Review-4 lifecycle fixes (ADR 0002 §8d — supersedes conflicting 8c items)

- Shared per-(root,me) state file ~/.amq-bridge/state/<root-hash>-<me>.json: {owner{pid,startedAt,sessionKey}, processedIds cap500, activeId, migrated0002}. Root-hash keying (cross-cwd), atomic write, O_EXCL claim or pid+startedAt staleness.
- Detach + session_shutdown both release owner; detach keeps processedIds; attachments cleanup owner-only.
- Claim only when watcher runs (TUI+attached) — never at generic restore.
- One watcher per session; attach/connect abort old (root,*) watchers of this session.
- Secondary = read-only: send/reply/resolve blocked; inbox/read/status/attach/detach allowed.
- Reply: record processed + persist BEFORE CLI send (at-least-once).
- Migration check on watcher start; flag in shared file (once per root).
- Exit-3: notify once, stop loop, release claim.
- Accepted: kill-9 watch-child orphan leak (follow-up), resumed-session marker check (sessionKey+pid+startedAt), connect promotes to owner.

### Review-5 protocol completion (ADR 0002 §8e — supersedes §8d wording)

- Claim/release protocol: O_EXCL lock carries {pid,startedAt,sessionKey}; dead-pid lock → unlink+retry; live-pid → wait ≤2s → secondary; write-then-unlink order; conditional owner clear (owner.sessionKey==self); lock scope = ownership transitions + migration flag only; appends lock-free via single-writer invariant.
- sha256(root) hex for state/lock filenames (never lossy char-map).
- Upgrade bootstrap: first claim unions legacy injected.json ids into processedIds (no pre-upgrade re-inject); legacy migrated-0002 marker → set flag, skip note. Create-on-claim, corrupt file → backup+recreate.
- Memory = cache; shared file = truth; recompute on claim.
- Connect promotion succeeds only when owner gone/stale; never displaces live owner.
- Accepted: kill-9 orphan (C12 graceful-scoped), persist-then-inject skip window, resume/fork first-bite race, no fsync.
- New criteria 14-16: concurrent claim, owner kill-9 takeover, upgrade boundary.
- Stale §2/§7/§8-main text: later-section-wins; cleanup during implementation.

### T10 validation checkpoint — 2026-08-02

- `npm test`: 130/130 pass; `npx tsc --noEmit`: clean.
- Four local Pi smokes passed: basic, tool, PTY, and multipeer. Assertions use envelope IDs plus explicit `amq_bridge_read`; no inbox buffer reads.
- Cross-CWD real-Pi e2e passed with isolated per-run `PI_AMQ_ROOT`, explicit read/reply IDs, subject/body checks, and tool-result-gated waits.
- Real-Pi multi-round e2e passed once end-to-end with isolated root. Later reruns reached protocol steps but were provider-rate-limited (`429`); no bridge assertion failure observed in those attempts.
- T1–T9 commits are present on `feat/adr-0002-notify-pull`; stale `acceptance-report.json` and `review-findings.json` remain untracked and are excluded from commits.
- Accepted residuals remain: kill-9 watch-child orphan window, no fsync power-loss guarantee, and provider-dependent real-Pi dogfood repeatability.