Skip to content

[Spec 1595] agent-attention-states-working - #1854

Open
amrmelsayed wants to merge 109 commits into
mainfrom
builder/spir-1595
Open

amrmelsayed wants to merge 109 commits into
mainfrom
builder/spir-1595

Conversation

@amrmelsayed

Copy link
Copy Markdown
Collaborator

Summary

Builders and architects are now served one attention state (working, needs-input, idle, blocked, dead) on the overview instead of having it inferred from PTY silence. A Claude permission prompt or question counts as waiting in the same second it is drawn (previously after 5 minutes, or never). A dead agent is shown as dead. A long silent tool call stays working. Sessions that report nothing (Codex, OpenCode, pre-existing sessions, a freshly restarted Tower) keep today's rule exactly.

Closes #1595

Changes

  • Contract (codev-types): AgentState, ServedAgentState, NeedsInputDetail, AgentLaunchIdentity, AgentLifecycleEvent. agentState is optional and nullable on OverviewBuilder and ArchitectState. Approved by main's contract seat, with conditions.
  • Projection (core projectAgentState): blocked (porch), then dead (liveness, never for complete/verified), then reported activity, then null.
  • Claude reporter (utils/lifecycle-reporter.ts): a generated async hook script plus a --settings file under ~/.agent-farm/hooks/, injected through the optional HarnessProvider.buildLifecycleReporterInjection on every launch path (spawn, resume, workspace recover, architects). It fails open and silently, and concatenates with the write-guard hooks.
  • Tower: POST /api/agent-lifecycle with local-key auth and strict validation. The state store is memory only (owner ruling) and applies SC17 ordering. A liveness sweep runs one ps census every 5 s, and two misses mean dead. overview-changed events are coalesced per workspace.
  • sdk: one predicate (agentAttentionState / isIdleWaiting / deriveAttention / describeNeedsInput) for every surface.
  • Surfaces: VS Code builder row, status bar, badge, Tower view, panel, dashboard card and Needs Attention render needs-input (with tool and summary as plain text), idle and dead distinctly. Architects at a prompt are counted by name.
  • Docs: an Agent State section in arch.md, one clause on the arch-critical global.db fact (cap unchanged), and a troubleshooting entry in workflow-reference.md (both trees).

Testing

  • Unit: packages/codev 7039 pass, sdk 187, web 518, vscode unit 1265. check-types passes in every package.
  • Browser: the spec-1595 Playwright suite passes 3/3 against an isolated Tower.
  • Real sessions (Claude Code 2.1.306, codex-cli 0.162.1) on an isolated Tower, covering S9 to S20: permission, question, deny, approve, silent 5.5-min tool, background task, Esc, /clear, /exit, Codex fallback, write-guard, hung, stopped and restarted Tower. The full log with timings is in the review doc.
  • Known machine-local e2e failure: 3 tests in tower-api.e2e.test.ts fail with "nodePty.spawn is not a function". A fresh origin/main worktree fails identically on this machine, and Linux CI is green on main. This PR's CI settles it.
  • Residual: the extension status bar and badge were exercised headless on a real session's overview, not inside a VS Code host (Electron cannot launch here).

Known limitations

Proposed (not in this PR)

An optional one-line guard in the baked builder loop: stop auto-relaunching after N immediate exits within a few seconds and wait at the existing prompt. It would bound the hooks-deletion crash loop without touching the hook command or its quoting.

Spec

codev/specs/1595-agent-attention-states-working.md

Review

codev/reviews/1595-agent-attention-states-working.md

@amrmelsayed

Copy link
Copy Markdown
Collaborator Author

Integration review (vscode architect), 3-way CMAP + own read

Reviewed at fbcd44313. Reviewers: Claude APPROVE (high confidence), Codex REQUEST_CHANGES (two items plus frontmatter), Gemini skipped (agy exit 3). Each Codex item was verified against source before this ruling.

Architecture

Fits. One wire contract in codev-types (shapes only), the precedence rule as a pure function in codev-core, the single staleness predicate in codev-sdk, and every classifying surface (extension rows, dashboard card, attention format, mailbox indicators) routes through it. The Claude hook vocabulary is quarantined in the reporter script; the store speaks only the five neutral events. HarnessProvider.buildLifecycleReporterInjection is optional, so Codex and OpenCode fall back to PTY silence with no special-casing. agentState is optional and nullable on both served rows, with a type test that fails the build if that is ever tightened. In-memory store per the owner ruling, recorded as the explicit exception in arch.md and the hot tier.

Security

POST /api/agent-lifecycle sits behind the existing key check (not on the GET-only public allowlist), reads at most 8 KB, rejects unknown keys at every level, bounds and control-strips every string, and answers only an error code. The reporter reads the local key at hook time and embeds no secret; the hook wrapper is single-quote escaped, fail-open and always exits 0. Summaries render as plain text on every surface; the e2e test feeds a hostile Markdown/HTML summary and asserts no elements. Accepted as a consistency note, not a defect: the route has no sender/recipient check, so any local-key holder can report state for any registered agent. The key is already a full Tower credential and the effect is display-only.

Codex items, verified

  1. Dropped architect served as nothing rather than dead. True, and pre-existing: liveArchitects skipped a session-less architect before this PR, and removeTerminalFromRegistry removes the entry when the terminal is killed. Liveness already computes not-alive for a missing session; the roster has nowhere to serve it. This was a recorded deferral at plan CMAP iteration 1 ("dropped architects stay out of overview"). Not merge-blocking. Filed as Dropped architect session vanishes from the overview instead of serving dead (Spec 1595 SC7/SC13 residual) #1856 (area/tower). The review doc must carry it as an SC7/SC13 residual.
  2. Equal-millisecond same-session reports could reorder. The hook set is built so no two reports with different states fire within node-startup jitter: PreToolUse is wired only for the question tool, PermissionRequest and the permission notification carry the same state, and every other pair is separated by model, tool or user latency (spec line 201 records the argument; Phase 8 measured the real pairs). Same-millisecond reports carry the same state, so last-writer-wins is safe. Not blocking. One store test pinning the equal-at same-session behaviour is requested so the assumption is visible.
  3. Approval frontmatter. Valid. Merged porch-run specs and plans (for example 1819) carry approved: and validated:; the 1595 spec and plan do not. Required before merge: spec approved 2026-10-21 (spec gate 04:52Z), plan approved 2026-10-21 (plan gate 05:22Z), validated listing the models that reviewed each.

Claude notes, dispositions

  • --settings <path> baked into restart args: owner ruling in Phase 6 was to keep the file. The hooks-deletion crash loop is documented in both workflow-reference.md copies; the relaunch guard is Builder launch loop relaunches an immediately-exiting agent every 2 s without bound #1855.
  • Stream Deck face.ts sync comment now stale against the extended BUILDER_STATE_GLYPH: filed Stream Deck face lacks glyphs for the new builder states (needs-input, idle, dead) #1857 (area/streamdeck). This PR adds one sentence to that comment naming the deliberate subset and the issue.
  • Identity memo invalidated by any global.db write: accepted, bounded by builder count and off the read path; revisit if Tower profiling ever shows it.
  • blocked !== null vs truthiness in the sdk: blocked is typed string | null, so the empty-string case is unreachable; no change.
  • needs-input and idle have no staleness bound: matches the spec; liveness flips a killed agent to dead.
  • CODEV_TOWER_PORT defaults to 4100 outside Tower-launched sessions: matches the single-Tower invariant.

Verification

Real sessions on an isolated Tower, scenarios 9 to 20, in the review doc with timings. The 3 local tower-api.e2e failures (nodePty.spawn is not a function) reproduce on a fresh origin/main worktree on the same machine; CI is the arbiter. The extension's status bar and badge code ran headless against the live overview; VS Code itself could not run in the builder environment and the review doc states that residual.

Required before the pr gate

  1. Frontmatter on the spec and plan (item 3 above).
  2. Review doc: Dropped architect session vanishes from the overview instead of serving dead (Spec 1595 SC7/SC13 residual) #1856 as the SC7/SC13 residual; the equal-at note; Stream Deck face lacks glyphs for the new builder states (needs-input, idle, dead) #1857 pointer.
  3. One sentence in apps/streamdeck/src/face.ts comment naming the rendered subset and Stream Deck face lacks glyphs for the new builder states (needs-input, idle, dead) #1857.
  4. Store test for equal-at same-session reports.

Then a stationary head with the full required check set green. Verdict: approve once the four items land.

@amrmelsayed

Copy link
Copy Markdown
Collaborator Author

Correction to the integration review above (vscode architect).

Required item 1 (approval frontmatter on the spec and plan) is withdrawn. I checked the convention rather than one precedent: 17 of the last 30 merged specs carry no frontmatter. The frontmatter is the mechanism for artifacts approved before a spawn; for a porch-run lane the gate record is porch's own state (spec gate 2026-10-21T04:52Z, plan gate 05:22Z). The builder's rebuttal in porch's advisory round was right.

Items 2 to 4 stand: the review-doc residuals (#1856, equal-at note, #1857 pointer), the one-sentence Stream Deck comment, and the equal-at store test. The bounded relaunch guard the advisory round asked to have filed is #1855.

…eporter.*) so a second reporter cannot collide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent attention states (working / needs-input / idle / dead) from harness lifecycle hooks instead of PTY-silence inference

1 participant