From 85994c174c9c28251d1f0852a016cec1c91db906 Mon Sep 17 00:00:00 2001 From: oscarlehuu Date: Mon, 10 Aug 2026 16:10:12 +0930 Subject: [PATCH] docs(plans): implementation plans for issues #117, #118, #119, #121 Four plan directories produced via scout-then-plan (ak:issue-to-plan workflow, one headless session per issue), each validated and red-teamed, then cross-reviewed together: - 20260810-docs-truth-gate-audit (#117, 5 phases) - 20260810-hermes-profile-editing (#118, 10 phases) - 20260810-profile-lifecycle-hardening (#119, 7 phases) - 20260810-evidence-thread-log (#121, 9 phases) Cross-plan review outcomes (also posted per-issue as comments): decision numbers allocated D-029 archive / D-030 write-through / D-031 evidence (D-028 is channel-first, #122); execution order 117-p01 -> #119 -> #118 for the shared doc files; #121 DoD corrected to honor D-020 (no upstream PRs from this fork). Signed-off-by: oscarlehuu --- .../ISSUE-COMMENT.md | 64 ++++ ...hase-01-state-truth-and-anti-drift-rule.md | 103 +++++ ...ase-02-pr-112-north-star-reconciliation.md | 92 +++++ .../phase-03-gate-e2e-shard-evidence-audit.md | 115 ++++++ .../phase-04-smoke-shard-gate-decision.md | 99 +++++ .../phase-05-worktree-hygiene.md | 136 +++++++ plans/20260810-docs-truth-gate-audit/plan.md | 174 +++++++++ .../ISSUE-COMMENT.md | 59 +++ .../phase-01-spike-evidence-tag-roundtrip.md | 81 ++++ .../phase-02-office-prompt-rule.md | 98 +++++ .../phase-03-red-contract-tests.md | 96 +++++ .../phase-04-cli-evidence-flag.md | 96 +++++ .../phase-05-desktop-evidence-card.md | 109 ++++++ .../phase-06-owner-accept-reject.md | 91 +++++ .../phase-07-upstream-generic-half.md | 86 +++++ .../phase-08-decisions-and-state.md | 84 ++++ .../phase-09-live-probes-verification.md | 109 ++++++ plans/20260810-evidence-thread-log/plan.md | 266 +++++++++++++ .../ISSUE-COMMENT.md | 59 +++ .../phase-01-spike-hermes-config-soul.md | 75 ++++ .../phase-02-red-contract-tests.md | 73 ++++ .../phase-03-runtime-capability-descriptor.md | 92 +++++ .../phase-04-profile-model-ipc.md | 102 +++++ .../phase-05-profile-soul-ipc.md | 99 +++++ .../phase-06-model-write-through-ui.md | 83 ++++ .../phase-07-soul-editor-ui.md | 81 ++++ .../phase-08-description-optionality.md | 82 ++++ .../phase-09-docs-and-state.md | 108 ++++++ .../phase-10-verification-evidence.md | 99 +++++ plans/20260810-hermes-profile-editing/plan.md | 308 +++++++++++++++ .../ISSUE-COMMENT.md | 70 ++++ .../phase-01-spike-readiness-and-archive.md | 148 ++++++++ .../phase-02-readiness-model-backend.md | 140 +++++++ .../phase-03-preflight-attention-routing.md | 123 ++++++ .../phase-04-archive-restore-backend.md | 145 +++++++ .../phase-05-readiness-surfacing-ui.md | 123 ++++++ .../phase-06-offboarding-archive-ui.md | 136 +++++++ .../phase-07-verification-and-docs.md | 134 +++++++ .../plan.md | 358 ++++++++++++++++++ 39 files changed, 4596 insertions(+) create mode 100644 plans/20260810-docs-truth-gate-audit/ISSUE-COMMENT.md create mode 100644 plans/20260810-docs-truth-gate-audit/phase-01-state-truth-and-anti-drift-rule.md create mode 100644 plans/20260810-docs-truth-gate-audit/phase-02-pr-112-north-star-reconciliation.md create mode 100644 plans/20260810-docs-truth-gate-audit/phase-03-gate-e2e-shard-evidence-audit.md create mode 100644 plans/20260810-docs-truth-gate-audit/phase-04-smoke-shard-gate-decision.md create mode 100644 plans/20260810-docs-truth-gate-audit/phase-05-worktree-hygiene.md create mode 100644 plans/20260810-docs-truth-gate-audit/plan.md create mode 100644 plans/20260810-evidence-thread-log/ISSUE-COMMENT.md create mode 100644 plans/20260810-evidence-thread-log/phase-01-spike-evidence-tag-roundtrip.md create mode 100644 plans/20260810-evidence-thread-log/phase-02-office-prompt-rule.md create mode 100644 plans/20260810-evidence-thread-log/phase-03-red-contract-tests.md create mode 100644 plans/20260810-evidence-thread-log/phase-04-cli-evidence-flag.md create mode 100644 plans/20260810-evidence-thread-log/phase-05-desktop-evidence-card.md create mode 100644 plans/20260810-evidence-thread-log/phase-06-owner-accept-reject.md create mode 100644 plans/20260810-evidence-thread-log/phase-07-upstream-generic-half.md create mode 100644 plans/20260810-evidence-thread-log/phase-08-decisions-and-state.md create mode 100644 plans/20260810-evidence-thread-log/phase-09-live-probes-verification.md create mode 100644 plans/20260810-evidence-thread-log/plan.md create mode 100644 plans/20260810-hermes-profile-editing/ISSUE-COMMENT.md create mode 100644 plans/20260810-hermes-profile-editing/phase-01-spike-hermes-config-soul.md create mode 100644 plans/20260810-hermes-profile-editing/phase-02-red-contract-tests.md create mode 100644 plans/20260810-hermes-profile-editing/phase-03-runtime-capability-descriptor.md create mode 100644 plans/20260810-hermes-profile-editing/phase-04-profile-model-ipc.md create mode 100644 plans/20260810-hermes-profile-editing/phase-05-profile-soul-ipc.md create mode 100644 plans/20260810-hermes-profile-editing/phase-06-model-write-through-ui.md create mode 100644 plans/20260810-hermes-profile-editing/phase-07-soul-editor-ui.md create mode 100644 plans/20260810-hermes-profile-editing/phase-08-description-optionality.md create mode 100644 plans/20260810-hermes-profile-editing/phase-09-docs-and-state.md create mode 100644 plans/20260810-hermes-profile-editing/phase-10-verification-evidence.md create mode 100644 plans/20260810-hermes-profile-editing/plan.md create mode 100644 plans/20260810-profile-lifecycle-hardening/ISSUE-COMMENT.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-01-spike-readiness-and-archive.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-02-readiness-model-backend.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-03-preflight-attention-routing.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-04-archive-restore-backend.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-05-readiness-surfacing-ui.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-06-offboarding-archive-ui.md create mode 100644 plans/20260810-profile-lifecycle-hardening/phase-07-verification-and-docs.md create mode 100644 plans/20260810-profile-lifecycle-hardening/plan.md diff --git a/plans/20260810-docs-truth-gate-audit/ISSUE-COMMENT.md b/plans/20260810-docs-truth-gate-audit/ISSUE-COMMENT.md new file mode 100644 index 00000000000..d31015594a7 --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/ISSUE-COMMENT.md @@ -0,0 +1,64 @@ +## Issue-to-Plan Handoff — #117 + +Plan: `plans/20260810-docs-truth-gate-audit/plan.md` (5 phases, docs + CI-posture +only). Validate: **PASS**. Red-team: **5 findings, all applied**. + +### Phases + +| # | Title | Effort | Depends on | DoD | +| - | ----- | ------ | ---------- | --- | +| 01 | `STATE.md` truth refresh + anti-drift rule | M | — | 1, 2 | +| 02 | PR #112 reconciliation against the north star | S | — | 3 | +| 03 | Gate / E2E-shard evidence audit (read-only) | M | — | 4 (evidence) | +| 04 | Founder decision: smoke shards required or advisory | S | 03 (+ #114 triage, soft) | 4 (decision) | +| 05 | Worktree hygiene with an unmerged-work safety gate | S | — | 5 | + +01–03 are independent and touch disjoint files. All 5 DoD checkboxes are mapped. + +### Key design decisions + +- **Item 3 is already answered by the code — no investigation phase needed.** The + Gate genuinely ignores the smoke shards, on purpose: `nuncio-crew-ci.yml:258` + sets `continue-on-error: true`, `:320` omits the job from `gate.needs`, and + `nuncio-crew-ci-contract.test.mjs:150` *asserts* that omission. The in-line + comment at `:248-251` names #36/#37. So PR #114's SUCCESS-over-red-shards is + designed behavior, not a mis-reported run. The real defect is that + `docs/crew/CI.md:15-23` never mentions the smoke job, so a green gate reads as + "E2E passed". Phase 03 documents it; Phase 04 asks whether to change it. +- **Recommendation: keep the shards advisory until #109 and #110 close.** Making + a known-broken lane required red-walls every desktop PR without fixing a test. + The founder decides; the plan does not flip the gate. +- **Item 4's premise is wrong and the plan corrects it.** + `.worktrees/bring-hermes-chat-into-crew` is *not* merged residue: its branch has + 6 commits not on `main` and not on PR #114's head. `git cherry` is unusable here + (squash-merge rewrites patch-ids, so it flags even the commit that merged as + #113). Phase 05 splits into a safe `git worktree prune` of the 24 prunable + `buzz-ae-e2e-*` entries, and a **gated** removal that defaults to no action. +- **Scope kept honest:** Phase 01 also fixes stale claims the issue did not list — + the Buzz pin at `STATE.md:94-95` says `0.5.3` while `upstream-buzz.json` says + `0.5.7`, and `:72` claims Settings shows `v0.5.3` while the app is `0.5.7`. +- **PR #112: recommend close-with-reason**, after copying its #110 root-cause + bisect onto issue #110 so the evidence survives. + +### Seams (all Crew-owned — 0 upstream lines expected) + +- Gate aggregation: `.github/workflows/nuncio-crew-ci.yml:317` / `:320` +- Gate pass/fail rule: `desktop/scripts/check-nuncio-crew-ci-results.mjs:6` +- RED contract seam: `desktop/src/testing/nuncio-crew-ci-contract.test.mjs:130-156` +- Agent obligations: `docs/crew/AGENT-WORKING-AGREEMENT.md:81` +- Merge-contract doc: `docs/crew/CI.md:15-23` +- Temp-worktree source: `desktop/tests/e2e/helpers/twoRelayHarness.ts:36` (read-only) + +Upstream's `.github/workflows/ci.yml:295` *does* aggregate smoke — inherited, +read-only, not Crew's to change. All PRs target `Nuncio-hq/crew` (D-020). +D-025 generic-ACP check: **N/A, explicitly** — no wire contract, event kind, or +engine-specific behavior is introduced. + +### Open questions + +1. Should the anti-drift rule get a CI guard, or stay review-visible prose only? + (Issue asks for a rule; plan ships prose + a decision entry.) +2. The `buzz-ae-e2e-*` leak recurs from the e2e harness. Prune only, or file a + separate issue to fix the harness cleanup? +3. Confirm PR #112 should be closed rather than revised — it belongs to another + session's branch. diff --git a/plans/20260810-docs-truth-gate-audit/phase-01-state-truth-and-anti-drift-rule.md b/plans/20260810-docs-truth-gate-audit/phase-01-state-truth-and-anti-drift-rule.md new file mode 100644 index 00000000000..442c5ee14f4 --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/phase-01-state-truth-and-anti-drift-rule.md @@ -0,0 +1,103 @@ +--- +phase: 01 +title: STATE.md truth refresh + anti-drift rule +status: pending +priority: high +effort: M +dependencies: [] +--- + +# Phase 01 — `STATE.md` truth refresh + anti-drift rule + +- **Issue:** #117 — problem item 1; DoD checkboxes 1 and 2 +- **PR scope:** docs only. No code, no CI config, no runtime behavior. +- **Files:** `docs/crew/STATE.md`, `docs/crew/AGENT-WORKING-AGREEMENT.md`, + `docs/crew/DECISIONS.md` +- **Upstream files touched:** none (all three verified absent from `upstream/main`) + +## Context + +`docs/crew/STATE.md:16` claims the implementation slices below it "remain the +code truth for what is built today", and `IDENTITY.md:39` sends every agent there +for current fork state. The file is wrong in at least six places, so agents +sequence work off false state. This has recurred with no rule preventing it. + +## Stale-claim inventory (verified 2026-08-10) + +| `STATE.md` | Claims | Observable reality | Source | +| ---------- | ------ | ------------------ | ------ | +| `:180-182` ("Current gate") | the `0.0.6` branch "is not merged", `crew-v0.0.6` "is not published", `0.0.5 → 0.0.6` updater relaunch pending | `crew-v0.0.6` published 2026-08-01; `crew-v0.0.9` is **Latest** since 2026-08-07 | `gh release list --repo Nuncio-hq/crew` | +| `:222` | "No `crew-v0.0.6` tag or public `0.0.6` artifact has been created" | same as above — four releases past it | `gh release list` | +| `:94-95` | Buzz source pin `0.5.3` at `3a96acea09b4…` | `0.5.7` at `f167818d25dd…` | `docs/crew/upstream-buzz.json` | +| `:72` | Settings displays `v0.5.3 · Local` | version is `0.5.7` | `desktop/package.json:4`, `desktop/src-tauri/tauri.conf.json:4` | +| attention/recovery line | absent | shipped through #108 (`6793c86da`) and #113 (`304173e42`); #114 open | `git log --oneline origin/main` | +| roles track | absent | issue #116 is the head; PR #120 open | `gh issue list`, `gh pr list` | +| Hermes track `:255-257` | "Next gates: Slice 2 …" | still accurate — Slice 2 not merged | verified, leave as-is | + +## Steps + +1. Rewrite `## Current gate` (`STATE.md:177-185`) to state the real release + position: releases published through `crew-v0.0.9` (2026-08-07), the + thread-worktree `0.0.6` line merged and released, and whatever updater + verification genuinely remains — do **not** carry the `0.0.5 → 0.0.6` phrasing + forward if the newer releases superseded it. If the updater relaunch was never + verified on any pair, say that plainly instead of dropping the obligation. +2. Fix `:222` in `## Current test gate` the same way. +3. Correct the Buzz source pin (`:94-95`) and the Settings version string (`:72`) + to match `upstream-buzz.json` and `desktop/package.json`. Prefer pointing at + `upstream-buzz.json` as the machine-readable source over restating the numbers, + so this line cannot drift again. +4. Add an attention/recovery line to the implementation record: merged through + #113; **PR #114 open as a follow-up at time of writing** (name it as in-flight, + not as shipped — see `AGENT-WORKING-AGREEMENT.md:40` on not hiding open work). +5. Add the Hermes track's current position (Slice 0–1 complete, Slice 2 next — + already at `:255-257`, verify rather than duplicate) and reference issue #116 + as the roles track head with PR #120 in flight. +6. Stamp `Last updated:` with the real merge-day date. +7. Add the anti-drift rule to the **implementation checklist** at + `AGENT-WORKING-AGREEMENT.md:81-87`, as a new checkbox in the existing list + style: + > - [ ] Shipped state changed (release published, slice merged, gate changed) + > → update [`STATE.md`](STATE.md) in the **same** PR +8. Append the rule to `docs/crew/DECISIONS.md` as the next free ID (**D-028** as + of 2026-08-10 — re-check the tail before writing, PR #120 may land one first). + Per `AGENT-WORKING-AGREEMENT.md:87`, a new sticky choice gets a decision entry. + Status Accepted, dated, linking the working agreement. + +## Contracts + +| Scenario | Expected result | Forbidden | +| -------- | --------------- | --------- | +| Agent reads `STATE.md` to sequence work | every release/version/merge claim matches live repo state on the merge date | inventing a release, slice, or verification that did not happen | +| Agent opens a PR that publishes a release or changes the gate | checklist tells them to update `STATE.md` in that PR | rule living only in a plan file or PR description | +| In-flight work (#114, #120) | named as open, with its state | described as shipped | + +## Validation + +Spot-check every assertion in the refreshed file against live state on the PR's +head — this is the issue's own verification bar ("no claim in the refreshed file +contradicts observable repo state"): + +```bash +gh release list --repo Nuncio-hq/crew --limit 10 +git log --oneline origin/main -10 +gh pr list --repo Nuncio-hq/crew --state open +cat docs/crew/upstream-buzz.json +grep -n '"version"' desktop/package.json +``` + +Then `just ci` on the branch. Docs-only paths mean the desktop jobs skip and +`NuncioCrew Gate` accepts the deliberate skips (`docs/crew/CI.md:11-13`). + +**This PR must itself satisfy DoD checkbox 1** — it changes `STATE.md`, so it is +trivially compliant with the rule it introduces; state that in the PR body. + +## Risk and rollback + +- **Risk:** the file is re-stale by merge time (PRs #114/#120 in flight). + Mitigation: write state as-of a named date with open PRs listed as open; re-run + the spot-check on the exact merge head. +- **Risk:** over-editing turns a state record into a changelog. Mitigation: keep + the existing section structure; change claims, not organization. +- **Rollback:** docs-only single PR — `git revert` restores prior text with no + runtime effect. diff --git a/plans/20260810-docs-truth-gate-audit/phase-02-pr-112-north-star-reconciliation.md b/plans/20260810-docs-truth-gate-audit/phase-02-pr-112-north-star-reconciliation.md new file mode 100644 index 00000000000..c92c2b7d7cf --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/phase-02-pr-112-north-star-reconciliation.md @@ -0,0 +1,92 @@ +--- +phase: 02 +title: PR #112 reconciliation against the north star +status: pending +priority: high +effort: S +dependencies: [] +--- + +# Phase 02 — PR #112 reconciliation against the north star + +- **Issue:** #117 — problem item 2; DoD checkbox 3 +- **PR scope:** either a revision commit on `docs/plans-open-issues`, or a close + with a recorded reason. No code either way. +- **Target repo:** `Nuncio-hq/crew` only (D-020). + +## Context + +PR [#112](https://github.com/Nuncio-hq/crew/pull/112) "docs(plans): execution +plans for the six open issues" was opened 2026-08-09T03:59Z. It adds seven +docs-only files: + +``` +plans/20260809-0355-open-issues-sequencing/plan.md (index, 78 lines) +plans/20260809-0400-e2e-shard4-revival/plan.md (#109) +plans/20260809-0405-channel-question-card/plan.md (#110) +plans/20260809-0410-file-size-ratchet-upstream-files/plan.md (#111) +plans/20260809-0415-channel-first-missions/plan.md (#102) +plans/20260809-0420-hermes-first-class-operations/plan.md (#104) +plans/20260809-0425-agent-attention-recovery/plan.md (#105) +``` + +It predates the north star: `FOUNDER-PRODUCT.md` and D-025/D-026/D-027 landed +2026-08-10 via #115 (`06107122b`), and issue #116 (agent roles) did not exist when +#112 was written. Its sequencing index is the risk — merging a *sequencing +authority* that predates the locked product direction commits Crew to an ordering +nobody re-checked. + +Note what #112 already got right and do not discard it: it carries a real bisect +for #110 (`AppShell.tsx` → `useLiveHomeFeedActions` subscription races the test's +readiness gate) and corrects the issue's own stated cause. That evidence is +worth keeping wherever it ends up. + +## The decision + +Two options. **Recommended: B.** + +| | A — revise and merge | B — close with recorded reason | +| - | -------------------- | ------------------------------ | +| Work | push a revision commit reconciling the index with `FOUNDER-PRODUCT.md`, D-025–D-027, and #116/#120 | comment on #112 linking the superseding issues, close, keep nothing on disk | +| Pro | preserves the #110 bisect and the per-issue plans in-tree | no stale sequencing authority; each issue keeps its own plan as it is planned | +| Con | requires pushing to a branch this session does not own; the index needs re-deciding against a product direction that changed under it; #105 and #108 have since merged, so parts are already historical | loses the #110 bisect unless it is copied into #110 first | +| Thin-fork / drift | a sequencing index is a stateful record, not evergreen authority (`documentation-management` rule) — keeping it invites future agents to treat it as law | matches how the other five issues are being planned today (one plan dir per issue, at planning time) | + +**Recommendation rationale:** #112's own body says #102/#104/#105 keep their specs +in the issue bodies and the plan files add only status, coupling, and ordering. +Ordering is exactly the part the north star and #116 invalidated, and status is +already stale (#105/PR #108 merged, #113 merged). What survives is the #110 +bisect — which belongs on #110 regardless. + +## Steps + +1. Re-read #112's index (`plans/20260809-0355-open-issues-sequencing/plan.md`) + against `docs/crew/FOUNDER-PRODUCT.md`, D-025/D-026/D-027, and issues + #116/#121. Write down each ordering claim that the north star changed. +2. **Before touching the branch**, confirm ownership — #112 was authored by a + different session. If it has a live owner, hand them this phase's finding + rather than pushing. +3. If B: copy the #110 root-cause bisect table into a comment on issue #110 so the + evidence survives, then close #112 with a comment naming the superseding + issues (#116, #117, #121) and stating the reason: *sequencing index predates + the locked founder product direction; per-issue plans are being produced at + planning time instead*. +4. If A: push one revision commit that rewrites the index against the north star + and drops the already-shipped entries (#105/#108, #113), then merge through + `NuncioCrew Gate`. +5. Record which option happened, and where, in this phase file's status line. + +## Validation + +- The resolution is **visible on the PR itself** (issue #117's stated bar): either + a merge commit, or a closing comment that names the superseding issues. +- If B: the #110 bisect is present on issue #110 before #112 closes. +- No product-code change in either option; no `block/buzz` PR (D-020). + +## Risk and rollback + +- **Risk:** pushing to a branch owned by another session mid-flight. Mitigation: + step 2's ownership check is a hard gate. +- **Risk:** closing loses evidence. Mitigation: step 3 preserves the bisect first. +- **Rollback:** a closed PR can be reopened; a revision commit can be reverted on + the branch before merge. diff --git a/plans/20260810-docs-truth-gate-audit/phase-03-gate-e2e-shard-evidence-audit.md b/plans/20260810-docs-truth-gate-audit/phase-03-gate-e2e-shard-evidence-audit.md new file mode 100644 index 00000000000..2e040904f9a --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/phase-03-gate-e2e-shard-evidence-audit.md @@ -0,0 +1,115 @@ +--- +phase: 03 +title: Gate / E2E-shard evidence audit (read-only) +status: pending +priority: high +effort: M +dependencies: [] +--- + +# Phase 03 — Gate / E2E-shard evidence audit (read-only) + +- **Issue:** #117 — problem item 3; DoD checkbox 4 (evidence half) +- **PR scope:** docs only — a verification record plus a `CI.md` correction. + **No workflow edit in this phase.** +- **Files:** `docs/crew/verification/0007-gate-e2e-shard-relationship.md` (new), + `docs/crew/CI.md` +- **Upstream files touched:** none. `.github/workflows/ci.yml` is inherited and is + **read-only** here. + +## Context and the already-answered question + +The issue asks: "Either the Gate does not aggregate the smoke shards, or it +reported on a different run." **The first is true, and it is deliberate.** This +phase does not re-discover that; it writes it down with citations and supplies +the flake numbers Phase 04 needs. + +Evidence, all in Crew-owned files: + +| Fact | Citation | +| ---- | -------- | +| Smoke shards cannot fail the job | `.github/workflows/nuncio-crew-ci.yml:258` — `continue-on-error: true` | +| The intent is recorded in-line | `nuncio-crew-ci.yml:248-251` — "Advisory only until the smoke suite's ~5 moving failures per green run are attributed and quarantined (#37) … Do not add this job to `gate.needs` or JOB_RELEVANCE until it can fail for real reasons — see issue #36" | +| The gate does not depend on them | `nuncio-crew-ci.yml:320` — `needs: [changes, desktop-fast, desktop-rust, macos-arm, project-relay, buzz-acp]` | +| The pass/fail rule has no smoke entry | `desktop/scripts/check-nuncio-crew-ci-results.mjs:6-12` — `JOB_RELEVANCE` | +| The posture is contract-tested | `desktop/src/testing/nuncio-crew-ci-contract.test.mjs:130-156`, notably `:150` `assert.doesNotMatch(ci, /needs\.desktop-smoke-e2e\.result/)` and `:155` the same for the gate helper | +| Upstream Buzz does the opposite | `.github/workflows/ci.yml:295` `needs: [changes, desktop-core, desktop-smoke-e2e]`, `:306-307` fails on non-success | + +So on PR #114, `NuncioCrew Gate` reporting SUCCESS while shards 1 and 3 were +FAILURE and shard 4 CANCELLED is **the designed behavior**, not a mis-reported +run. A red E2E state can and does merge to `main` — knowingly. + +The real defect is a **documentation gap**: `docs/crew/CI.md:15-23` lists the six +gate jobs and never mentions `Desktop Smoke E2E` at all, so a reader concludes +the gate covers everything that runs. Nothing in `docs/crew/` tells an agent that +a green gate proves nothing about E2E. + +## Flake-rate evidence to collect + +Phase 04 cannot be decided without numbers. Gather from `main` runs of +`NuncioCrew CI` (issue #109 already contains the bisect table through +`e41a1a6a4` — extend it, do not redo it): + +1. Per-shard conclusion for the last ~10 `main` runs (`success` / `failure` / + `cancelled`). +2. For shard 4: confirm the 30-minute-timeout cancellations continue, and how many + of the shard's 250 tests execute before the kill. +3. The currently-failing spec set per shard, and whether each is upstream-owned or + Crew-only. #109 records that all the timing-out specs are upstream-owned at + `desktop-v0.5.7` and that `project-outcomes.spec.ts` (the only Crew-only project + spec) is not among them. +4. Whether #114's own triage attributed its shard 1/3 failures to flake or to a + real regression — this is the input the issue defers to the #114 owner. If that + triage has not landed, record the audit as complete-with-one-open-input rather + than blocking; the workflow-config half needs nothing from #114. + +```bash +gh run list --repo Nuncio-hq/crew --workflow "NuncioCrew CI" --branch main --limit 10 \ + --json databaseId,headSha,conclusion,createdAt +gh run view --repo Nuncio-hq/crew --json jobs \ + --jq '.jobs[] | select(.name|startswith("Desktop Smoke E2E")) | {name, conclusion, startedAt, completedAt}' +``` + +## Steps + +1. Write `docs/crew/verification/0007-gate-e2e-shard-relationship.md` following + the numbering in `docs/crew/verification/` (0006 is the current tail). It + records: the question, the four workflow/script/test citations above, the + upstream contrast, the per-shard run table, and a verdict. +2. Fix the gap in `docs/crew/CI.md`: add `Desktop Smoke E2E` to the job table + (`:15-23`) with "Runs when: desktop paths change" and "Proves: **nothing that + blocks merge** — advisory, `continue-on-error`, excluded from the gate by + design (#36/#37)". Add one sentence under "Merge contract" stating plainly that + a green `NuncioCrew Gate` is not evidence that E2E passed — mirroring the + existing honest boundary at `CI.md:66-70` ("A green merge gate is not release + proof"). +3. Do **not** change any workflow, script, or test in this phase. + +## PASS / FAIL / INCONCLUSIVE (this phase is the evidence gate for Phase 04) + +- **PASS** — the record cites the exact lines proving the gate excludes smoke, the + contrast with upstream `ci.yml`, and a per-shard table covering at least the last + 10 `main` runs. `CI.md` no longer implies the gate covers E2E. +- **INCONCLUSIVE** — run history is unavailable or the #114 triage input is + missing; record what is known, name the missing input, and let Phase 04 wait. +- **FAIL** — evidence contradicts the citations above (e.g. the gate *does* + aggregate smoke on the live workflow). Then the issue's premise changes and + Phase 04 must be re-planned before anyone acts. + +## Validation + +- Every claim in the record resolves to a `path:line` or a `gh run` id — the + issue's bar: "reproducible: cites the workflow file lines and the concrete #114 + check-run evidence". +- `just ci` on the docs branch. +- The anti-drift rule from Phase 01 does not trigger here (no shipped-state + change) — but if Phase 04 later changes the gate, that PR does trigger it. + +## Risk and rollback + +- **Risk:** the record reads as an accusation that the gate is broken. It is not — + the posture is deliberate and recorded. Write it as "what the gate proves", per + `AGENT-WORKING-AGREEMENT.md:18-23` (plain, no CEO brief). +- **Risk:** scope creep into fixing #109/#110. Out of scope — those are their own + issues. +- **Rollback:** docs-only; revert. diff --git a/plans/20260810-docs-truth-gate-audit/phase-04-smoke-shard-gate-decision.md b/plans/20260810-docs-truth-gate-audit/phase-04-smoke-shard-gate-decision.md new file mode 100644 index 00000000000..4b233b6bb2e --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/phase-04-smoke-shard-gate-decision.md @@ -0,0 +1,99 @@ +--- +phase: 04 +title: Founder decision — smoke shards required or advisory +status: pending +priority: medium +effort: S +dependencies: ["03"] +--- + +# Phase 04 — Founder decision: smoke shards required or advisory + +- **Issue:** #117 — problem item 3; DoD checkbox 4 (decision half) +- **Soft dependency:** the #114 owner's flake-vs-real triage (Phase 03 step 4). + Absent it, the decision can still be made on #109/#110 evidence — say so. +- **Decision owner:** the founder. Agents present options; they do not flip the + gate. (`review-audit-self-decision` rule: an audit is input, not an order; the + advisory posture is an existing recorded choice — `nuncio-crew-ci.yml:248-251`.) + +## Why this is a decision and not a fix + +The current posture was chosen on purpose and is locked by a contract test. It is +also genuinely costly: `main` can go red in E2E and merge anyway. Both facts are +true at once, so this is a trade-off the founder owns. + +## Options + +| | A — keep advisory until #109/#110 close (**recommended**) | B — make shards required now | C — make required, but only shards 2 and 3 | +| - | --- | --- | --- | +| Change | none | add `desktop-smoke-e2e` to `gate.needs` and to `JOB_RELEVANCE`; drop `continue-on-error` | partial matrix in the gate | +| Effect today | `main` keeps merging with red E2E; the gap is at least documented (Phase 03) | **every desktop PR red-walls immediately** — shard 4 is cancelled at the 30-min timeout on 6/6 consecutive `main` runs (#109) and shard 1 has hard-failed since `25263120e` (#96, tracked as #110) | avoids the two known-broken lanes while restoring some blocking signal | +| Cost | the safety net stays off; the regression #109 describes (composer-clear atomicity during the v0.5.5 sync) could recur unseen | development stops until #109 and #110 are fixed | encodes "which shards are trustworthy" into CI — brittle, since sharding is by count and a spec's shard assignment moves whenever the suite changes | +| Reversibility | high | high but disruptive while red | medium — needs re-tuning on every suite change | + +**Recommendation: A.** Making a lane required while it is known-broken converts a +documented gap into a hard block on all desktop work, and it does not fix a single +test. The ordering #112 already argued for still holds: restore the lane +(#109 → #110), *then* make it required. The value of this issue's audit is that +the gap is now written down instead of implicit. + +Treat B as the right end state, gated on #109 and #110 closing. + +## Steps (only if the founder chooses B or C) + +**RED first — D-008 (`Spike → RED tests → implementation`).** The advisory posture +is asserted by an existing test, so the test changes before the workflow does: + +1. Rewrite `desktop/src/testing/nuncio-crew-ci-contract.test.mjs:130-156` to assert + the new contract — replace the two negative assertions + (`:150` `assert.doesNotMatch(ci, /needs\.desktop-smoke-e2e\.result/)` and `:155` + the same for the gate helper) with positive ones, and drop the + `continue-on-error: true` assertion at `:141`. +2. Run `pnpm -C desktop test` and **observe it fail**. Record the failure output — + that is the RED evidence. A test that passes before the change proves nothing. +3. Only then edit `.github/workflows/nuncio-crew-ci.yml`: remove + `continue-on-error: true` (`:258`), add `desktop-smoke-e2e` to `gate.needs` + (`:320`) and to the gate payload (`:329`), and replace the advisory comment at + `:248-251` with the new rationale. +4. Add `"desktop-smoke-e2e": "desktop"` to `JOB_RELEVANCE` in + `desktop/scripts/check-nuncio-crew-ci-results.mjs:6-12`. +5. Re-run `pnpm -C desktop test` — green. +6. Append the decision to `docs/crew/DECISIONS.md` at the next free ID (**D-029** + if Phase 01 took D-028 — re-check the tail). Record the trade-off and the + evidence, not just the outcome. +7. Update `docs/crew/CI.md` (the table Phase 03 corrected) and — because this + changes the merge gate — **update `docs/crew/STATE.md` in the same PR** per the + Phase 01 anti-drift rule. This is the rule's first real exercise. + +**Expected diff size:** ~6 lines in `nuncio-crew-ci.yml`, 1 line in +`check-nuncio-crew-ci-results.mjs`, ~10 lines in the contract test, plus docs. All +four files are Crew-only — **0 upstream lines**. Do not touch +`.github/workflows/ci.yml`; upstream's gate already aggregates smoke and its +behavior is not Crew's to change (D-020: nothing goes to `block/buzz`). + +## Steps (if the founder chooses A) + +1. Record the decision in `docs/crew/DECISIONS.md` **only if** the founder wants it + sticky. "Keep the status quo for now" may not warrant a decision entry; ask. +2. Otherwise, note the outcome in the Phase 03 verification record and close the + DoD item — the issue's wording is "founder decision recorded **if a change is + made**". +3. Consider filing the "make required" work as a follow-up issue blocked on #109 + and #110, so the end state is not lost. Ask before filing. + +## Validation + +- If B/C: contract test observed RED before the workflow edit, green after; a full + `NuncioCrew CI` run on the branch shows the gate now consuming the shard results. +- If A: the decision and its rationale are readable by the next agent without + re-deriving the evidence. +- Either way: no `block/buzz` PR, no upstream file edited. + +## Risk and rollback + +- **Risk (B/C):** immediate `main` red-wall. Mitigation: the recommendation is A; + if B is chosen anyway, sequence it after #109/#110 land. +- **Risk:** an agent applies B because "audits recommend hardening". Mitigation: + this phase requires an explicit founder choice; the plan's default is A. +- **Rollback:** revert the workflow/script/test commit — the gate returns to + advisory within one PR, and the contract test pins whichever posture is current. diff --git a/plans/20260810-docs-truth-gate-audit/phase-05-worktree-hygiene.md b/plans/20260810-docs-truth-gate-audit/phase-05-worktree-hygiene.md new file mode 100644 index 00000000000..f8dee589eab --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/phase-05-worktree-hygiene.md @@ -0,0 +1,136 @@ +--- +phase: 05 +title: Worktree hygiene with an unmerged-work safety gate +status: pending +priority: low +effort: S +dependencies: [] +--- + +# Phase 05 — Worktree hygiene with an unmerged-work safety gate + +- **Issue:** #117 — problem item 4 ("Minor"); DoD checkbox 5 +- **PR scope:** none. This phase changes **local git state only** — no repo files, + no commit, nothing to merge. +- **Split:** 05a is safe and unblocked. 05b is gated and may end in "do nothing". + +## Correction to the issue's premise (read before acting) + +The issue describes `.worktrees/bring-hermes-chat-into-crew` as "still checked out +on **merged** `fix/agent-attention-recovery-hardening`". Verified 2026-08-10: +**that branch is not fully merged.** + +``` +$ git rev-list --count origin/main..59ab743ef +6 +$ git merge-base --is-ancestor 59ab743ef origin/main → NOT an ancestor +$ git merge-base --is-ancestor 59ab743ef origin/fix/agent-attention-postmerge-audit → NO +``` + +The six commits ahead of `origin/main`: + +``` +59ab743ef fix(agent): preserve concurrent session authority +08beb03a6 fix(agent): fail closed on untrusted attention input +142e3463a fix(desktop): satisfy session size ratchet +47fc7b5d1 fix(agent): project reviewed receipt after publish +1d9a37221 fix(agent): hide completed receipt without active turn +17b4353bc fix(agent): close attention recovery gaps ← merged as #113 (304173e42, squashed) +``` + +`git cherry` marks **all six** as having no upstream equivalent, including +`17b4353bc`, whose content demonstrably merged as #113. That is the known +squash-merge blind spot: squashing rewrites the patch-id, so `git cherry` cannot +distinguish "merged via squash" from "never merged". **Do not use `git cherry` as +the safety check here.** The five commits above `17b4353bc` are post-#113 work, and +PR #114 (`fix/agent-attention-postmerge-audit`) is a *different* branch whose +commits carry different subjects — overlap is plausible but unproven. + +Removing this worktree and branch without reconciliation risks destroying work. +`UPSTREAM-SYNC.md:138-141` is explicit: destructive git recovery requires approval. + +## Current state (2026-08-10) + +- 38 worktrees registered on the main checkout. +- 26 under `$TMPDIR/buzz-ae-e2e-*/mission-worktree`, created by + `desktop/tests/e2e/helpers/twoRelayHarness.ts:36` (`mkdtemp(join(tmpdir(), + "buzz-ae-e2e-"))`) and leaked when a run aborts. **24 are marked `prunable`** + (directory already gone). Two — `…-On5C8H`, `…-VlMJct` — are **not** prunable, + i.e. their directories still exist and may belong to a live run. +- Several `/private/tmp/crew-*` review checkouts, including + `/private/tmp/crew-postmerge-audit` on PR #114's branch — **in use, leave alone**. +- `.worktrees/` holds this planning worktree plus `issue-116-agent-roles` (PR #120, + active), `brainstorm-coding-app`, `integrate-browser-and-mobile-emulator-into-crew`, + and the `bring-hermes-chat-into-crew` case above. + +Note the main checkout is currently on `docs/founder-product-north-star`, not +`main` — do not "fix" that as part of hygiene. + +## 05a — Safe prune (unblocked) + +`git worktree prune` only removes registrations whose directory is already gone. +It cannot delete a live checkout or any branch. + +```bash +cd /Users/a1241968/Desktop/Oscar/LilGroup/Nuncio/crew +git worktree list | grep prunable | wc -l # expect ~24 before +git worktree prune -v +git worktree list | wc -l # expect ~14 after +``` + +Leave the two non-prunable `buzz-ae-e2e-*` entries alone — a live e2e run may own +them. Re-check after any in-flight run finishes; they become prunable on their own. + +## 05b — Gated: `.worktrees/bring-hermes-chat-into-crew` + +**Default action: none.** Proceed only when all three hold: + +1. The #114 line has closed (merged or abandoned), so its branch is final. +2. A content reconciliation shows the five post-#113 commits are represented on + `origin/main` — compare trees over the touched paths rather than patch-ids: + ```bash + git diff --stat origin/main 59ab743ef -- desktop/src crates docs/crew + git log --oneline --name-only origin/main..59ab743ef + ``` + Anything left that is not on `main` is unmerged work. +3. The owner of that session confirms the branch is disposable. + +If any check fails, **stop and report** — do not remove. Preserving the branch +costs one stale directory; losing hardening work costs a re-audit. If all three +pass: + +```bash +git worktree remove /Users/a1241968/Desktop/Oscar/LilGroup/Nuncio/crew/.worktrees/bring-hermes-chat-into-crew +# branch deletion is a separate, later decision — the worktree can go while the branch stays +``` + +Do not pass `--force`. If `git worktree remove` refuses because the checkout is +dirty, that refusal *is* the safety gate working: report the dirty files. + +Also note `/private/tmp/crew-hardening-verify` sits on the same commit +(`59ab743ef`, detached). Same reasoning applies; it is a temp path and lower risk. + +## Validation + +- `git worktree list` shows no `prunable` entries afterwards. +- No branch was deleted in this phase. +- Every remaining worktree is either active work or explicitly justified. +- `git worktree list` on the main checkout still shows the PR #114 and PR #120 + worktrees intact. + +## Risk and rollback + +- **Risk:** data loss from removing unmerged work — the whole point of 05b's gate. + Mitigation: default is no action; three independent checks; no `--force`. +- **Risk:** pruning a live e2e run's worktree mid-test. Mitigation: `prune` cannot + touch existing directories; the two non-prunable entries are left alone. +- **Rollback:** a pruned registration is recreatable with `git worktree add`; the + underlying commits are unaffected. A removed *checkout* with uncommitted changes + is **not** recoverable — hence the gate. + +## Out of scope (flagged, not done) + +The temp-worktree leak recurs because the e2e harness does not clean up on abort +(`desktop/tests/e2e/helpers/twoRelayHarness.ts:36`). Fixing that would stop +recurrence, but the file is upstream-shared and the issue asks only for a prune. +Raise as its own issue if the founder wants the recurrence fixed. diff --git a/plans/20260810-docs-truth-gate-audit/plan.md b/plans/20260810-docs-truth-gate-audit/plan.md new file mode 100644 index 00000000000..901333729a3 --- /dev/null +++ b/plans/20260810-docs-truth-gate-audit/plan.md @@ -0,0 +1,174 @@ +# Docs truth + gate audit (issue #117) + +- **Status:** Planned — not started +- **Date:** 2026-08-10 +- **Issue:** [#117](https://github.com/Nuncio-hq/crew/issues/117) +- **Repo:** `Nuncio-hq/crew` only — no PR ever targets `block/buzz` (D-020) +- **Type:** docs truth + CI posture audit + repo hygiene. **No product feature.** +- **Branch (proposed):** `docs/state-truth-and-gate-audit` + (area-prefixed per `UPSTREAM-SYNC.md` § Feature branches; not a phase number) + +## Goal + +Make the docs agents plan from tell the truth, make "shipped state changed +without `STATE.md` updated" a review-visible violation, resolve the pre-north-star +plan PR #112 instead of letting it drift, write down what the merge gate does and +does not prove about E2E, and clean git worktree residue **without destroying +unmerged work**. + +## Outcome in founder language + +Today an agent that reads `docs/crew/STATE.md` is told `crew-v0.0.6` is +unpublished and the thread-worktree branch is unmerged. Both are false — +`crew-v0.0.9` has been the Latest release since 2026-08-07. Agents sequence work +off that file, so a stale file produces wrong plans. After this work: the file +matches what `gh release list` and `git log origin/main` show, a written rule +makes the next drift a review finding, and nobody has to guess whether a green +`NuncioCrew Gate` means E2E passed (it does not, on purpose). + +## Scope + +1. [ ] [Phase 01 — `STATE.md` truth refresh + anti-drift rule](phase-01-state-truth-and-anti-drift-rule.md) +2. [ ] [Phase 02 — PR #112 reconciliation against the north star](phase-02-pr-112-north-star-reconciliation.md) +3. [ ] [Phase 03 — Gate / E2E-shard evidence audit (read-only)](phase-03-gate-e2e-shard-evidence-audit.md) +4. [ ] [Phase 04 — Founder decision: smoke shards required or advisory](phase-04-smoke-shard-gate-decision.md) +5. [ ] [Phase 05 — Worktree hygiene with an unmerged-work safety gate](phase-05-worktree-hygiene.md) + +Phases 01–03 are independent and may run in any order or in parallel (they touch +disjoint files). Phase 04 depends on 03. Phase 05 is independent but its second +half waits on the #114 line closing. + +## Non-goals (from the issue, kept verbatim in intent) + +- Merging PR #114 — an in-flight session owns its exact-head reviews and + merge-verify. This plan only *consumes* its triage output (Phase 04) and waits + on its branch line (Phase 05). +- Any product feature work: roles (#116), Hermes Slice 2, mobile. +- Fixing the E2E failures themselves — that is #109 (shard 4 timeout) and #110 + (question card). This plan documents the gate's relationship to those lanes; it + does not repair the lanes. +- Changing the smoke suite, `playwright.config.ts`, or any spec. + +## DoD → phase mapping (issue #117) + +Every Definition-of-Done checkbox maps to at least one phase: + +| # | DoD checkbox | Phase(s) | +| - | ------------ | -------- | +| 1 | `STATE.md` refreshed and merged via a PR that itself follows the new anti-drift rule | 01 | +| 2 | Anti-drift rule present in `AGENT-WORKING-AGREEMENT.md` implementation checklist | 01 | +| 3 | PR #112 resolved (revised+merged or closed with recorded reason) | 02 | +| 4 | Gate vs E2E-shard relationship documented with evidence; founder decision recorded if a change is made | 03 (evidence + docs), 04 (decision) | +| 5 | Stale worktrees pruned | 05 | + +## Named Buzz / Crew seams + +The issue introduces no runtime mechanism, so there is no ACP, relay, or Nostr +seam. The seams it hangs off are the Crew-owned CI and docs surfaces: + +| Concern | Seam (`path:line`) | Ownership | +| ------- | ------------------ | --------- | +| Merge gate aggregation | `.github/workflows/nuncio-crew-ci.yml:317` (`gate:`), `needs:` list at `:320` | **Crew-only** (absent from `upstream/main`) | +| Gate pass/fail rule | `desktop/scripts/check-nuncio-crew-ci-results.mjs:6` (`JOB_RELEVANCE`) | **Crew-only** | +| Advisory smoke posture | `.github/workflows/nuncio-crew-ci.yml:248-262` (`continue-on-error: true`, comment naming #36/#37) | **Crew-only** | +| Gate contract tests (the RED seam) | `desktop/src/testing/nuncio-crew-ci-contract.test.mjs:130-156` | **Crew-only** | +| Shipped-state record | `docs/crew/STATE.md` | **Crew-only** | +| Agent obligations checklist | `docs/crew/AGENT-WORKING-AGREEMENT.md:81` (`## Implementation checklist (agent)`) | **Crew-only** | +| Merge-contract doc | `docs/crew/CI.md:15-23` (job table) | **Crew-only** | +| Decisions log | `docs/crew/DECISIONS.md` (last entry D-027) | **Crew-only** | +| Temp e2e worktree creator | `desktop/tests/e2e/helpers/twoRelayHarness.ts:36` (`mkdtemp(... "buzz-ae-e2e-")`) | upstream-shared — **read-only in this plan** | + +The upstream analogue of the gate is `.github/workflows/ci.yml:295-307`, which +*does* aggregate `desktop-smoke-e2e`. Crew's gate deliberately does not. That file +is inherited and **must not be edited** by this work. + +## Thin-fork budget (D-001, `UPSTREAM-SYNC.md`) + +**Zero upstream-file edits are planned. Expected upstream diff: 0 lines.** + +Every file this plan writes was verified absent from `upstream/main` +(`git cat-file -e upstream/main:` fails for each). New files land under +`docs/crew/` and `plans/`, which are Crew namespaces. If Phase 04 selects the +"make shards required" option, the edits are still Crew-only +(`nuncio-crew-ci.yml`, `check-nuncio-crew-ci-results.mjs`, and the Crew contract +test) — a `+3/-2`-scale change to the gate wiring, not an upstream touch. Should +any phase find itself wanting to edit an upstream file, stop and record why +before proceeding; nothing in the current evidence requires it. + +## Generic-ACP check (D-025) + +**Not applicable, explicitly.** This plan introduces no wire contract, event kind, +tag, session behavior, or assignment mechanism. Nothing here is engine-specific, +so nothing needs a "works for non-Hermes engines" proof and nothing needs a +Hermes-only label. Phase 01 must not add Hermes-only claims to `STATE.md` beyond +the ones already sourced from the Hermes track records. + +## Workflow gates (D-008: Spike → RED → implementation) + +| Phase | Behavior change? | Gate applied | +| ----- | ---------------- | ------------ | +| 01 | No — docs only | No spike. Verification is evidence spot-check against live repo state. | +| 02 | No — plan docs on someone else's branch, or a close | No spike. Verification is the PR's visible resolution. | +| 03 | No — read-only audit producing a written record | **This phase is the evidence gate** for Phase 04. It plays the spike role: one decision-changing question, defined PASS/FAIL/INCONCLUSIVE, reproducible citations. | +| 04 | **Yes, if the founder changes the gate** — CI merge semantics | Phase 03 evidence first, then **RED test first**: `nuncio-crew-ci-contract.test.mjs:130` currently *asserts* the advisory posture and will fail before the workflow change; it must be rewritten to assert the new contract and observed failing before the workflow edit lands. | +| 05 | No production behavior; destructive to local git state | Safety gate: content reconciliation before any branch/worktree removal (see phase). | + +## Risks + +| Risk | Mitigation | +| ---- | ---------- | +| **Deleting unmerged work in Phase 05.** The issue calls `.worktrees/bring-hermes-chat-into-crew` "merged-branch residue". It is not: its branch `fix/agent-attention-recovery-hardening` @ `59ab743ef` carries 6 commits with no patch-equivalent on `origin/main` **or** on PR #114's head `origin/fix/agent-attention-postmerge-audit`. | Phase 05 prunes only worktrees git itself marks `prunable`, and blocks removal of that checkout behind an explicit content reconciliation + owner confirmation. | +| `STATE.md` refreshed to a snapshot that is stale again by merge time (PRs #114, #120 are in flight). | Phase 01 writes state **as-of a stated date with named open PRs**, and re-runs the spot-check on the exact head before merge. | +| Anti-drift rule becomes unenforced prose. | Phase 01 puts it in the checklist agents already read at `AGENT-WORKING-AGREEMENT.md:81` and records it as a decision; enforcement stays review-visible (soft) by design — no CI guard is proposed, see Open questions. | +| Phase 04 makes shards required while #109/#110 are open → `main` red-walls immediately. | Phase 04's recommendation is explicitly **not** to flip while shard 4 is cancelling at the 30-minute timeout; the decision is the founder's, and Phase 03 supplies the flake numbers to make it informed. | +| Revising PR #112 (option A) means editing a branch this session does not own. | Phase 02 defaults to the close-with-reason option and requires an owner check before pushing to that branch. | + +## Validation + +Run `just ci` on the Phase 01/03 docs branch (docs-only paths — the path +classifier will skip desktop jobs; the gate accepts deliberate skips per +`CI.md:11-13`). Phase 04, if it changes the gate, requires the full desktop +lane plus a proven-RED-then-green contract test. + +## Plan quality passes + +### Validate pass — **PASS** + +| Check | Result | +| ----- | ------ | +| Every issue DoD checkbox mapped to a phase | PASS — 5/5, table above | +| Issue scope respected, nothing silently dropped | PASS — problem items 1–4 map to phases 01, 02, 03+04, 05 | +| Issue non-goals honored | PASS — #114 merge, product features excluded and restated | +| Every phase has files, steps, validation, rollback | PASS | +| Buzz/Crew seams named with `path:line` | PASS — seam table | +| Thin-fork budget stated with expected diff size | PASS — 0 upstream lines | +| Generic-ACP check (D-025) stated | PASS — explicitly N/A, no mechanism introduced | +| D-020 respected (no `block/buzz` PR) | PASS — stated in header and Phase 02/04 | +| Spike → RED → implementation ordering | PASS — Phase 03 is the evidence gate; Phase 04 is RED-first | +| Anti-drift rule applied to this plan's own PRs | PASS — Phase 01 and 04 both carry `STATE.md` obligations | +| No phase depends on an unavailable input | PASS — Phase 04 and Phase 05b declare the #114 dependency | + +### Red-team pass — 5 findings, 5 applied + +| # | Finding | Disposition | +| - | ------- | ----------- | +| R1 | **The issue's premise for item 4 is wrong.** "Merged-branch worktree residue" — the branch has 6 commits with no equivalent on `main` or on #114. A plan that just says "prune" would have licensed data loss. | **Applied.** Phase 05 split into a safe prune (05a) and a gated removal (05b) with a reconciliation command and a default of *do nothing*. | +| R2 | **The issue's item 3 asks a question the code already answers.** "Either the Gate does not aggregate the smoke shards, or it reported on a different run" — the first is true and deliberate: `nuncio-crew-ci.yml:248-251` says so, `:258` sets `continue-on-error: true`, `:320` omits it from `gate.needs`, and `nuncio-crew-ci-contract.test.mjs:150` *asserts* the omission. Planning a discovery investigation would have burned a phase re-finding this. | **Applied.** Phase 03 is scoped to *documenting* the known-answer plus gathering flake numbers, not to discovering whether a gap exists. Its PASS criteria say so. | +| R3 | `git cherry` was the obvious safety check for Phase 05 and it is **not sufficient** — it reports `+` for `17b4353bc` even though that commit's content merged as #113 (`304173e42`), because the squash changed the patch-id. A plan that trusted it would report false unmerged work forever and the checkout would never be cleaned. | **Applied.** Phase 05b specifies a tree/content diff over the touched paths plus owner confirmation, and explicitly records that `git cherry` is inconclusive under squash-merge. | +| R4 | Phase 01 could refresh only the four items the issue lists and still leave `STATE.md` lying. Scouting found stale claims the issue does not mention: the Buzz source pin at `STATE.md:94-95` says `0.5.3 @ 3a96acea` while `docs/crew/upstream-buzz.json` says `0.5.7 @ f167818d`, and `:72` says Settings displays `v0.5.3 · Local` while `desktop/package.json:4` is `0.5.7`. | **Applied.** Phase 01 carries an explicit stale-claim inventory including these, and its validation is "no claim contradicts observable state", not "the four listed items changed". | +| R5 | Phase 04 could be read as "make the shards required" — an audit-driven change to a posture the founder's CI already deliberately chose, while the two lanes are known broken (#109 cancelling at 30 min, #110 hard-failing). That would red-wall `main` and reverse a recorded choice without asking. | **Applied.** Phase 04 presents options with trade-offs and an explicit recommendation to **keep advisory until #109/#110 close**; the founder decides. The plan never treats the flip as the default. | + +## Open questions + +1. **Anti-drift enforcement strength.** The issue asks for a written rule + ("review-visible violation"). A CI guard is possible (fail a PR that changes + `.github/workflows/nuncio-crew-*.yml` or release files without touching + `STATE.md`) but the issue did not ask for one and it would produce false + positives. Phase 01 ships prose + decision only. Say if a guard is wanted. +2. **Recurrence of the temp worktrees.** The 26 prunable `buzz-ae-e2e-*` entries + come from `desktop/tests/e2e/helpers/twoRelayHarness.ts:36`, which leaks them + on aborted runs. Fixing the harness would stop recurrence, but that is an + upstream-shared file and outside this issue. Phase 05 prunes only; flag if the + harness fix should become its own issue. +3. **PR #112 disposition.** Phase 02 recommends close-with-reason. Confirm before + anyone pushes to a branch this session does not own. diff --git a/plans/20260810-evidence-thread-log/ISSUE-COMMENT.md b/plans/20260810-evidence-thread-log/ISSUE-COMMENT.md new file mode 100644 index 00000000000..49331142959 --- /dev/null +++ b/plans/20260810-evidence-thread-log/ISSUE-COMMENT.md @@ -0,0 +1,59 @@ +# Draft issue comment for #121 (not posted) + +## Plan ready — 9 phases + +Plan: `plans/20260810-evidence-thread-log/plan.md`. Planning only; nothing +implemented, nothing pushed. All 6 DoD checkboxes map to phases. + +| # | Phase | Effort | Depends | +|---|---|---|---| +| 01 | Spike — unknown `crew-evidence` tag round-trip | S | — | +| 02 | Office prompt rule + thin-fork accounting | M | 01 | +| 03 | RED contract tests (CLI, desktop, reactions) | M | 01 | +| 04 | CLI `--evidence ` | S | 03 | +| 05 | Desktop evidence card (four kinds) | L | 03 | +| 06 | Owner Accept/Reject via NIP-25 reactions | M | 05 | +| 07 | Upstream generic half — **blocked** | S | 02 | +| 08 | DECISIONS.md schema + limit, STATE.md | S | 04,05,06 | +| 09 | Live probes + Playwright evidence | M | 04,05,06,08 | + +## Key design decisions + +- **No `buzz-sdk` edit.** Tag appended to the built `EventBuilder` before + signing, so `builders.rs` and `kind.rs` keep zero additional Crew delta. +- **Upstream footprint: 5 files, ~46 lines.** `base_prompt.md` and `messages.rs` + have zero Crew delta today; both first edits justified and accounted. +- **`MessageRow.tsx` budget ≤8 lines** — it sits at 980/1000 against the ratchet. + D-022 forbids raising `MAX_LINES`; logic lives in Crew-owned files. +- **No new CLI for reading verdicts** — `buzz reactions get --event` already + ships, so DoD 4's agent-readable half needs verification, not code. +- **Prompt section capped at 18 lines, test-enforced** — the base prompt is paid + on every turn of every agent; a fat section breaks the issue's own frugality rule. +- **kind 46043 ignores `crew-evidence`** — the receipt card keeps its own ✅, so + no message shows two competing review affordances. +- **Generic-ACP (D-025): no Hermes-only behavior.** Honest limit: only the CLI + can emit the tag this slice, not the desktop composer or mobile. + +## Buzz seams named + +`base_prompt.md:46` (office rule) · `buzz-acp/src/lib.rs:1942` (prompt reaches +every engine) · `buzz-sdk/src/builders.rs:250` `FAILURE_NOTICE_TAG` (custom tag +on kind 9 precedent) · `buzz-cli/src/client.rs:590` (post-build tag append) · +`buzz-cli/src/lib.rs:398` + `commands/messages.rs:574` (flag surface) · +`formatTimelineMessages.ts:520` → `types.ts:50` (tag reaches renderer, zero +plumbing) · `MessageRow.tsx:368/415` (dispatch) · `MessageRow.tsx:407-408` + +`AgentReceiptMessageBody.tsx:45-50` (accept via reaction, already shipping) · +`buzz-cli/src/lib.rs:746-774` (`reactions get`) · `e2eBridge.ts:1153` (injection). + +## Open questions + +1. **Upstream PR conflict (blocks phase 07).** DoD 1 asks for a PR to + `block/buzz`; D-020, root `AGENTS.md` and `UPSTREAM-SYNC.md:17` forbid it. The + plan opens none — it writes a Crew-owned draft of the generic half. Keep + D-020, or record a scoped exception? +2. **✅ overload.** ✅ already means "reviewed" on agent receipts and would also + mean "accept" on evidence. Never collides on one event. Keep ✅/❌ as the issue + specifies, or pick a distinct pair? +3. `UPSTREAM-SYNC.md` has **no upstream-file-edit list** — phase 02 creates one. + +Validate: pass. Red-team: 9 findings, 8 applied, 1 escalated (question 1). diff --git a/plans/20260810-evidence-thread-log/phase-01-spike-evidence-tag-roundtrip.md b/plans/20260810-evidence-thread-log/phase-01-spike-evidence-tag-roundtrip.md new file mode 100644 index 00000000000..f4220d11cbc --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-01-spike-evidence-tag-roundtrip.md @@ -0,0 +1,81 @@ +--- +phase: 01 +title: Spike — unknown crew-evidence tag round-trip +status: planned +priority: P0 +effort: S +dependencies: [] +--- + +# Phase 01 — Spike: unknown `crew-evidence` tag round-trip + +Gate 1 of `docs/crew/DEVELOPMENT-WORKFLOW.md`. Investigation only. **No +production code.** + +## Decision-changing question + +Does an unknown, Crew-invented tag on an ordinary message kind survive publish → +relay ingest → storage → query → desktop timeline **unchanged**, and do surfaces +that do not understand it ignore it safely? + +If the answer is no, the entire wire design in issue #121 item 2 is invalid and +phases 02-09 must be re-planned around a body convention (like the agent-receipt +JSON payload) instead of a tag. Nothing else in this plan is worth building until +this is settled. + +## Why this is not already known + +`docs/crew/STATE.md` records that Buzz preserves unknown metadata tags on +**kind 30617** — an addressable repository-announcement event with a different +ingest path. Kind 9 messages carry thread counters, mention fan-out, and edit +overlays. The upstream precedent for a custom tag on kind 9 exists +(`crates/buzz-sdk/src/builders.rs:250` `FAILURE_NOTICE_TAG`) but that tag is +built by the SDK; this slice appends one post-build from the CLI +(`crates/buzz-cli/src/client.rs:590`). The combination is unproven. + +## Steps + +1. Start a local relay (`just relay`) against a scratch database. +2. Publish a kind-9 message carrying a hand-built `["crew-evidence","test-run"]` + tag. Use an existing tag-capable path (for example `buzz notes --tag`, or a + short throwaway script against `buzz-ws-client`) — **do not** add the + `--evidence` flag yet, that is phase 04. +3. Query the event back (`POST /query` or `buzz messages thread`) and diff the + returned tag array against what was published. +4. Confirm the tag reaches the desktop timeline model: check that + `formatTimelineMessages` (`desktop/…/lib/formatTimelineMessages.ts:520`) + leaves it on `TimelineMessage.tags`, in the running app or via a unit probe. +5. Regression check: publish a normal kind-9 message in the same thread and + confirm reply counters, mentions and rendering are unaffected. +6. Ignore-safety check: confirm the mobile Flutter client and the web client + render the tagged message as an ordinary message rather than erroring. +7. Record whether an edit of a tagged message preserves the tag + (`applyEditTagOverlay` behavior) — this decides whether an edited evidence + report keeps its card. + +## Deliverable + +`docs/crew/spikes/0015-evidence-tag-roundtrip.md` following +`docs/crew/templates/SPIKE.md`, ending with **`PASS`**, **`FAIL`**, or +**`INCONCLUSIVE`**. Next free spike id is 0015 (`docs/crew/spikes/` currently +ends at `0014-agent-attention-recovery.md`). + +## Acceptance criteria + +- The spike answers the question above with commands and observed output, not + reasoning from the code. +- Steps 3, 4 and 5 each have a recorded observation. +- The verdict line is one of `PASS` / `FAIL` / `INCONCLUSIVE`. + +## Exit conditions + +- **PASS** → phases 02 and 03 unblock. +- **INCONCLUSIVE** → narrow the question and re-spike; do not proceed on hope. +- **FAIL** → stop the plan and return to the founder with the alternative + (evidence as a structured body payload, like `KIND_AGENT_RECEIPT`), which + changes the CLI phase, the desktop parser, and the DECISIONS entry. + +## Risk + +Low. Read-only against a scratch relay; no repo files change except the new +spike document. diff --git a/plans/20260810-evidence-thread-log/phase-02-office-prompt-rule.md b/plans/20260810-evidence-thread-log/phase-02-office-prompt-rule.md new file mode 100644 index 00000000000..51d1ce74de3 --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-02-office-prompt-rule.md @@ -0,0 +1,98 @@ +--- +phase: 02 +title: Office prompt rule + thin-fork accounting +status: planned +priority: P0 +effort: M +dependencies: ["01"] +--- + +# Phase 02 — Office prompt rule + thin-fork accounting + +Delivers the behavioral half of DoD checkbox 1: every agent, on every engine, +learns to attach proportionate evidence when reporting completion. + +## Seam + +`crates/buzz-acp/src/base_prompt.md` — the office-level prompt embedded for +**every** runtime at `crates/buzz-acp/src/lib.rs:1942` +(`include_str!("base_prompt.md")`), unless the operator passes +`--no-base-prompt` or an override file. This is why the rule goes here and not +into per-agent Layer-3 descriptions, which may legitimately be empty. + +Placement: adjacent to `## Communication Patterns` (`base_prompt.md:46`), which +already carries the sibling office rules — callback mentions (`:56-59`) and +"never publish a bare acknowledgement" (`:80`) — and immediately before or after +`## Engineering Discipline` (`:123`), whose line `:133` ("Validate in the shape +the task demands — tests for code, source citations for research, a reproduced +workflow or artifact for UI work") this section makes concrete. + +## Files + +| File | Change | Budget | +| --- | --- | --- | +| `crates/buzz-acp/src/base_prompt.md` | one new self-contained section | **≤ 18 lines** | +| `crates/buzz-acp/src/lib.rs` | one prompt-assertion test | ~8 lines | +| `docs/crew/UPSTREAM-SYNC.md` | new "Upstream files Crew edits" section | ~15 lines | + +`base_prompt.md` currently has **zero Crew delta** (147 lines, identical to +upstream). This is the first Crew edit to it — see the plan's thin-fork table. + +## Steps + +1. Draft the "Evidence on completion" section. It must be self-contained (one + heading, no cross-references into other sections) so an upstream sync + conflict resolves by keeping and re-placing one block. Content: + - the evidence-defaults mapping from the issue, **compressed to a compact + list rather than a 7-row table** to hold the line budget; + - the three token rules — capture in place, text-first, excerpt don't dump; + - the proportionality escape hatch: when no cheap evidence exists, say what + is unproven and how to verify — never a decorative screenshot; + - the no-computer-use constraint; + - Crew tooling pointers: `just desktop-screenshot`, + `buzz messages send --file`, `buzz messages send --evidence `. +2. Add a prompt-assertion test in `crates/buzz-acp/src/lib.rs`, following the + upstream pattern at `upstream/main:crates/buzz-acp/src/lib.rs:3944` + (`shared_base_prompt_teaches_portable_agent_drafts`). Assert both that the + section exists **and that it is at most 18 lines** — the cap is the mitigation + for R-4 and must be machine-checked, not remembered. +3. Create the "Upstream files Crew edits" section in + `docs/crew/UPSTREAM-SYNC.md`. The issue instructs recording the edit in this + list; **the list does not exist yet** (R-7). Seed it with a table of file, + justification, and resolve hint, covering at minimum the files this slice + touches. Place it near the existing thin-fork rules (`:20-32`) and the + conflict policy (`:110-120`). +4. Record for `base_prompt.md`: justification "office-level behavioral rule + belongs in the office-level prompt"; resolve hint "self-contained Markdown + section — on conflict, keep it and re-place it after Communication Patterns". + +## Acceptance criteria + +- The section is ≤18 lines and the test enforces that bound. +- No Crew-specific vocabulary leaks into the *generic* half of the rule — the + Crew tooling pointers are clearly separated so phase 07's draft can be lifted + out cleanly. +- The rule mentions no engine by name. It must read identically for Hermes, + Claude and Codex (D-025). +- `docs/crew/UPSTREAM-SYNC.md` lists `base_prompt.md` with justification and + resolve hint. +- `cargo test -p buzz-acp` green. + +## Validation + +```bash +cargo test -p buzz-acp shared_base_prompt +just ci +``` + +## Anti-drift + +If this phase ships in a PR that changes shipped behavior, update +`docs/crew/STATE.md` in the **same** PR (#117). + +## Risk + +Medium. The prompt is paid on every turn of every agent; an over-long or +preachy section costs tokens forever and contradicts the issue's own frugality +principle. The line cap plus the test is the guard. Rollback is a one-section +revert with no data implications. diff --git a/plans/20260810-evidence-thread-log/phase-03-red-contract-tests.md b/plans/20260810-evidence-thread-log/phase-03-red-contract-tests.md new file mode 100644 index 00000000000..d8e30452a61 --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-03-red-contract-tests.md @@ -0,0 +1,96 @@ +--- +phase: 03 +title: RED contract tests (CLI, desktop, reactions) +status: planned +priority: P0 +effort: M +dependencies: ["01"] +--- + +# Phase 03 — RED contract tests + +Gate 3 of `docs/crew/DEVELOPMENT-WORKFLOW.md`. Every test in this phase must +**fail for the right reason** before phases 04-06 begin. The issue names this +explicitly: *"Contract tests RED first."* + +## Contracts to pin + +### C1 — CLI tag emission (Rust) + +Location: `crates/buzz-cli/src/commands/messages.rs` test module (or a Crew-owned +sibling module if the upstream file's test section is contested). + +- `--evidence test-run` on `messages send` produces exactly one + `["crew-evidence","test-run"]` tag on the built event. +- All four kinds accepted: `test-run`, `metrics`, `before-after-visual`, + `diff-stat`. +- An unknown value is rejected with the input-error exit code (1), and **no + event is published**. +- Omitting `--evidence` produces a tag array byte-identical to today's. +- `--evidence` composes with `--file` (imeta tags both present) and with + `--reply-to`. + +### C2 — Desktop card render (Playwright, mock bridge) + +Location: `desktop/tests/e2e/`, registered in `desktop/playwright.config.ts` +(`smoke` `testMatch` at `:30`). Inject tagged messages through +`__BUZZ_E2E_EMIT_MOCK_MESSAGE__` with `extraTags` +(`desktop/src/testing/e2eBridge.ts:1153`). + +- Each of the four kinds renders its card (`data-testid` per kind). +- `metrics`, `test-run` and `diff-stat` are **fully legible with no images + loaded** — assert the text content, not just card presence. This is the + phone-friendly requirement from the issue. +- An unrecognized `crew-evidence` value renders the ordinary message body, not a + broken card or an error boundary. +- A message with **no** `crew-evidence` tag renders exactly as today + (non-regression). +- **R-2 contract:** a `KIND_AGENT_RECEIPT` (46043) message that also carries a + `crew-evidence` tag keeps the receipt card and does **not** grow a second + evidence card. + +### C3 — Reaction round-trip on an evidence message + +- Owner Accept sends a kind-7 `✅` targeting the evidence event id. +- Owner Reject sends a kind-7 `❌` and opens the reply composer. +- The card reflects the owner's own reaction after it lands. +- A non-owner viewer sees the card **without** Accept/Reject controls + (owner resolution mirrors `AgentReceiptMessageBody`: + `profiles[message.pubkey].ownerPubkey === currentPubkey`). +- Reactions on non-evidence messages are unaffected. + +## RED validity criteria + +Each test must fail because the behavior is absent, not because of a typo, a +missing import, or an unbuilt fixture. Record the observed failure message for +each contract in the PR description. A test that fails with +`Cannot read properties of undefined (reading 'invoke')` is a **build mistake**, +not a RED — that means `pnpm build:e2e` was skipped (root `AGENTS.md`). + +## Steps + +1. Write C1 first — it is cheapest and pins the wire format the other two assume. +2. Write C2 with `locator.screenshot()` scoping already in place, so phase 09's + `shasum` distinct-state gate has a chance of passing (R-6). +3. Write C3 against the mock bridge's reaction path. +4. Run each suite, capture the failure output, confirm every failure is a + genuine absence. + +## Validation + +```bash +cargo test -p buzz-cli evidence +cd desktop && pnpm test:e2e:smoke +``` + +## Acceptance criteria + +- All C1/C2/C3 tests exist and are RED. +- Failure reasons recorded and each one is an absence, not an accident. +- No production behavior changed in this phase. + +## Risk + +Low, but this is the phase most likely to be skipped under time pressure. +Skipping it breaks the Crew workflow gate (AGENT-WORKING-AGREEMENT MUST NOT #8) +and removes the only proof that phases 04-06 actually delivered. diff --git a/plans/20260810-evidence-thread-log/phase-04-cli-evidence-flag.md b/plans/20260810-evidence-thread-log/phase-04-cli-evidence-flag.md new file mode 100644 index 00000000000..b142cf32794 --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-04-cli-evidence-flag.md @@ -0,0 +1,96 @@ +--- +phase: 04 +title: CLI --evidence flag on messages send +status: planned +priority: P0 +effort: S +dependencies: ["03"] +--- + +# Phase 04 — `buzz messages send --evidence ` + +Turns C1 green. Delivers DoD checkbox 2. + +## Seam + +`buzz messages send` today has `--channel`, `--content`, `--kind`, `--reply-to`, +`--broadcast`, `--file`, `--mention` — and **no generic `--tag`** (`--tag` exists +only on `notes`). The clap variant is `MessagesCmd::Send` at +`crates/buzz-cli/src/lib.rs:398`; the handler is `cmd_send_message` at +`crates/buzz-cli/src/commands/messages.rs:574`, which builds the event through +`buzz_sdk::build_message` at `:664` and signs at `:680`. + +**Key design choice — do not touch `buzz-sdk`.** Append the tag to the already +built `EventBuilder` before signing, using the precedent at +`crates/buzz-cli/src/client.rs:590` (`builder.tags([tag.clone()])`). That keeps +`crates/buzz-sdk/src/builders.rs` (+407 Crew lines today) at zero additional +delta and confines the change to the CLI. + +## Files + +| File | Change | Budget | +| --- | --- | --- | +| `crates/buzz-cli/src/lib.rs` | one clap arg on `MessagesCmd::Send` | ~6 lines | +| `crates/buzz-cli/src/commands/messages.rs` | field on `SendMessageParams` (`:564`), validate, append tag before sign | ~14 lines | +| new Crew-owned module (e.g. `crates/buzz-cli/src/commands/evidence.rs`) | kind enum + parse/validate + unit tests | new file | + +`messages.rs` currently has **zero Crew delta** (1375 lines, identical to +upstream). Keeping the validation logic in a new Crew-owned module is what holds +that first edit to ~14 lines. + +## Steps + +1. Define the kind enum in the new Crew module: `test-run`, `metrics`, + `before-after-visual`, `diff-stat`. One canonical string per variant; parse is + exact-match (no aliases, no case folding) so the wire format stays stable. +2. Add `--evidence ` to `MessagesCmd::Send`, following the existing + add-a-CLI-flag playbook in that file. +3. Thread it onto `SendMessageParams` and validate **before** any network call or + media upload — an invalid kind must exit 1 with a clear message and publish + nothing. +4. After the builder is constructed at `:664`, append + `["crew-evidence", ]`, then sign as today. +5. Emit exactly one such tag. If the value is somehow already present, do not + duplicate it. +6. Decide and document kind coverage: the tag is appended for whichever message + kind the user selected; **only kind 9 renders a card in this slice** (phase + 05). Say so in `--help` text rather than silently rejecting other kinds. +7. Update `crates/buzz-cli/TESTING.md` if it enumerates `messages send` flags. + +## What validation does and does not mean + +`--evidence` validates **the enum value only**. It cannot and does not verify +that the message body actually contains evidence (RT-4). Do not describe it as +"validated evidence" in help text or docs — say "validated evidence kind". + +## Acceptance criteria + +- All C1 contract tests green. +- No change to the tag array of messages sent without `--evidence`. +- `crates/buzz-sdk/src/builders.rs` and `crates/buzz-core/src/kind.rs` unchanged. +- Exit code 1 with a readable error on an unknown kind; nothing published. +- `--evidence` composes with `--file`, `--reply-to`, `--mention`, `--broadcast`. + +## Validation + +```bash +cargo test -p buzz-cli +cargo clippy --all-targets -- -D warnings +just ci +``` + +Manual smoke against a local relay: + +```bash +buzz messages send --channel --content "…" --evidence test-run +buzz --format compact messages thread --channel --event +``` + +## Anti-drift + +Update `docs/crew/STATE.md` in the same PR (#117). Add the CLI surface to the +Crew CLI documentation if one enumerates flags. + +## Risk + +Low. Additive flag, no behavior change when omitted, trivially revertible. diff --git a/plans/20260810-evidence-thread-log/phase-05-desktop-evidence-card.md b/plans/20260810-evidence-thread-log/phase-05-desktop-evidence-card.md new file mode 100644 index 00000000000..8fa53398e11 --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-05-desktop-evidence-card.md @@ -0,0 +1,109 @@ +--- +phase: 05 +title: Desktop evidence card (four kinds) +status: planned +priority: P0 +effort: L +dependencies: ["03"] +--- + +# Phase 05 — Desktop evidence card + +Turns C2 green. Delivers DoD checkbox 3. + +## Seams + +| Need | Seam | +| --- | --- | +| Tag reaches the renderer | `desktop/src/features/messages/lib/formatTimelineMessages.ts:520` already assigns `tags: applyEditTagOverlay(...)` onto `TimelineMessage`; `types.ts:50` types it as `string[][]`. **Zero plumbing work.** | +| Per-kind body dispatch | `desktop/src/features/messages/ui/MessageRow.tsx:368` `renderBody()` switch; evidence rides kind 9, so it lands in the `default:` branch at `:415` | +| Crew-owned body already in that branch | `MessageRowDefaultBody` (`:429-446`), a Crew-owned 127-line file that already receives `message` and `imetaByUrl` | +| Card structure precedent | `AgentReceiptCard.tsx` (Crew-owned, 173 lines) — testids, PR-reference link resolution defaulting to `Nuncio-hq/crew` | +| Tolerant parse precedent | `agentReceipt.mjs` `parseAgentReceipt()` returns `null` and the renderer falls back | + +## The ratchet constraint (R-1) — read before writing code + +`MessageRow.tsx` is **980 lines against a 1000-line cap** +(`desktop/scripts/check-file-sizes.mjs:8`, `MAX_LINES = 1000`). D-022 and +`docs/crew/UPSTREAM-SYNC.md:63-66` forbid raising the limit or granting an +exception — the required move is extracting Crew deltas into Crew-owned files. + +**Budget: ≤8 added lines in `MessageRow.tsx`.** + +Preferred route: pass the props the card needs down into the Crew-owned +`MessageRowDefaultBody` at `:429-446` and branch on the tag *there*. That is +roughly six added prop lines in the upstream-derived file and keeps all logic +Crew-side. + +If the budget cannot be met, **extract** existing Crew additions out of +`MessageRow.tsx` into a Crew-owned module until it fits. Do not raise +`MAX_LINES`. Do not add an allowlist entry. + +## Files + +| File | Owner | Change | +| --- | --- | --- | +| `desktop/src/features/messages/ui/MessageRow.tsx` | upstream-derived | **≤8 lines** — prop pass-through only | +| `desktop/src/features/messages/ui/MessageRowDefaultBody.tsx` | Crew | branch on evidence kind, delegate to the card | +| `desktop/src/features/messages/lib/evidenceTag.{mjs,ts}` | Crew (new) | `parseEvidenceKind(tags)` → kind or `null` | +| `desktop/src/features/messages/ui/EvidenceCard.tsx` | Crew (new) | shell + per-kind layouts | +| per-kind subcomponents if the file approaches ~200 lines | Crew (new) | keep files small | + +## Rendering rules + +| Kind | Layout | Image dependency | +| --- | --- | --- | +| `metrics` | compact number table (before / after / delta) | none | +| `test-run` | red→green block, failing excerpt above passing | none | +| `diff-stat` | summary line + PR reference link (reuse `AgentReceiptCard`'s href resolution) | none | +| `before-after-visual` | side-by-side images via the existing `imeta` media pipeline | required; must degrade to captions + links when images fail to load | + +Hard requirements: + +- The three text kinds must be **fully legible with no images loaded** — this is + the phone-friendly requirement and is asserted in C2. +- The card body is **agent-authored, untrusted**. Render through the existing + Markdown pipeline; no raw HTML, no new sanitizer bypass. +- Parse defensively: an unknown kind, a malformed body, or a missing image + renders the ordinary message body. Never an error boundary. +- **kind 46043 keeps the receipt card and ignores `crew-evidence`** (R-2). +- Text sizing: rem-based Tailwind tokens only. No `text-[13px]`, no arbitrary rem + literals — `pnpm check:px-text` fails the build otherwise (root `AGENTS.md`). + +## Steps + +1. Write `parseEvidenceKind` with the tolerant-parse contract. +2. Build `EvidenceCard` with the three text layouts first — they carry the + phone-friendly requirement and need no media plumbing. +3. Add `before-after-visual` last, reusing the `imetaByUrl` path + `MessageRowDefaultBody` already receives. +4. Wire the branch, measuring `MessageRow.tsx` line count before and after. +5. Run the ratchet and px-text guards explicitly, not just via `just ci`. + +## Acceptance criteria + +- All C2 contract tests green. +- `MessageRow.tsx` ≤ 1000 lines with `MAX_LINES` unchanged at 1000. +- `pnpm check:px-text` green. +- Non-evidence messages render byte-identically to today. +- No new upstream-file edits beyond the ≤8 lines in `MessageRow.tsx`. + +## Validation + +```bash +cd desktop && pnpm check:px-text && node scripts/check-file-sizes.mjs +cd desktop && pnpm test:e2e:smoke +just ci +``` + +## Anti-drift + +Update `docs/crew/STATE.md` in the same PR (#117). If harness capability facts +change, update `desktop/src/features/agents/AGENTS.md` in the same PR. + +## Risk + +Highest-risk phase in the plan. Two guards can trip (file-size ratchet, px-text) +and both have a tempting wrong fix (raise the limit, allowlist the line). Neither +is permitted. Rollback is removing the branch — published evidence messages then +render as ordinary messages, which is exactly the pre-slice behavior. diff --git a/plans/20260810-evidence-thread-log/phase-06-owner-accept-reject.md b/plans/20260810-evidence-thread-log/phase-06-owner-accept-reject.md new file mode 100644 index 00000000000..52bc0c5a20a --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-06-owner-accept-reject.md @@ -0,0 +1,91 @@ +--- +phase: 06 +title: Owner Accept/Reject via NIP-25 reactions +status: planned +priority: P0 +effort: M +dependencies: ["05"] +--- + +# Phase 06 — Owner Accept/Reject via existing reactions + +Turns C3 green. Delivers the send/persist/render half of DoD checkbox 4; the +agent-readable half is verified in phase 09 against already-shipped CLI. + +## Seams — this pattern already ships + +The agent-receipt card implements exactly this interaction today: + +| Behavior | Seam | +| --- | --- | +| Accept sends a kind-7 reaction | `desktop/src/features/messages/ui/MessageRow.tsx:408` — `onReviewed={() => handleReactionSelect("✅")}` | +| Reject carries a reason | `MessageRow.tsx:407` — `onRequestChanges={onReply ? () => onReply(message) : undefined}` opens the reply composer | +| Reaction state read back | `AgentReceiptMessageBody.tsx:45-50` — `reactions.some(r => r.emoji === "✅" && r.reactedByCurrentUser === true)` | +| Owner gating | `AgentReceiptMessageBody.tsx` — `profiles[normalizePubkey(message.pubkey)]?.ownerPubkey === currentPubkey` | +| Reaction transport | `useReactionHandler.ts` + `MessageReactions.tsx` — **upstream-owned, unchanged** | +| Agent reads the verdict | `buzz reactions get --event ` — `crates/buzz-cli/src/lib.rs:746-774`, **already exists** | + +**No new event kinds, no new semantics, no workflow machinery** — the issue is +explicit about this, and the code agrees. + +## Files + +| File | Owner | Change | +| --- | --- | --- | +| `desktop/src/features/messages/ui/EvidenceCard.tsx` | Crew | Accept/Reject controls + verdict state | +| `desktop/src/features/messages/ui/MessageRowDefaultBody.tsx` | Crew | accept and forward `reactions`, `canToggleReactions`, `currentPubkey`, `reactionPending`, `profiles`, `onReply` | +| `desktop/src/features/messages/ui/MessageRow.tsx` | upstream-derived | prop pass-through only — **counts against the same ≤8-line budget as phase 05** | +| `desktop/src/features/messages/ui/useReactionHandler.ts` | upstream | **no change** | + +## Behavior + +1. **Accept** → kind-7 `✅` on the evidence event, through the same handler the + receipt card uses. The card then shows an accepted state. +2. **Reject** → kind-7 `❌` **and** open the reply composer, so rejection carries + a reason (RT-7). A bare ❌ with no explanation leaves the agent nothing to act + on. +3. **Owner only.** Non-owners see the card and any existing reaction counts, but + no Accept/Reject controls. Owner resolution reuses the receipt-card rule + above; do not invent a second owner concept. +4. **Verdict state** is derived from the owner's own reaction on the event — it + is not stored anywhere new. Reactions are durable owner-signed room events; + that is the whole persistence story. +5. **No automation.** Nothing triggers on ❌. Any follow-up agent behavior is + prompt-level (phase 02), per the issue. + +## Open founder decision (D-2) + +✅ already means "reviewed" on `KIND_AGENT_RECEIPT`. This phase reuses ✅ as +"accept" on evidence messages, per the issue's "no new semantics". They never +collide on one event, but it is one glyph with two nearby meanings. Implement the +default; if the founder picks a distinct pair, only the emoji constants and the +C3 assertions change. + +## Acceptance criteria + +- All C3 contract tests green. +- No edits to `useReactionHandler.ts` or `MessageReactions.tsx`. +- No new event kind; `crates/buzz-core/src/kind.rs` unchanged. +- Non-owner view has no Accept/Reject affordance. +- Reject opens the composer with the evidence message as parent. +- Reactions on ordinary messages behave exactly as today. + +## Validation + +```bash +cd desktop && pnpm test:e2e:smoke +just ci +``` + +Round-trip against a local relay is covered in phase 09. + +## Anti-drift + +Update `docs/crew/STATE.md` in the same PR (#117). + +## Risk + +Medium. The interaction is precedented, so the risk is duplication rather than +novelty: resist writing a second owner-resolution helper or a second reaction +sender. Rollback removes the controls; already-published ✅/❌ reactions remain +valid room events and still render as ordinary reactions. diff --git a/plans/20260810-evidence-thread-log/phase-07-upstream-generic-half.md b/plans/20260810-evidence-thread-log/phase-07-upstream-generic-half.md new file mode 100644 index 00000000000..22ea0abae7c --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-07-upstream-generic-half.md @@ -0,0 +1,86 @@ +--- +phase: 07 +title: Upstream generic half — BLOCKED on founder decision +status: blocked +priority: P2 +effort: S +dependencies: ["02"] +--- + +# Phase 07 — Upstream generic half (BLOCKED) + +This phase exists so DoD checkbox 1's second clause is **visible and undropped**, +not silently executed and not silently deleted. + +## The conflict + +**Issue #121 item 1 asks:** + +> **Parallel, non-blocking:** open an upstream PR to `block/buzz` proposing the +> *generic* half (evidence-proportionate-to-change-type culture, no +> Crew-specific tooling). If accepted, the fork delta shrinks on a future sync. +> Do not wait on it for anything. + +**Repo law forbids it:** + +- `docs/crew/DECISIONS.md` **D-020** — *"no pull request will be opened against + `block/buzz` for this feature (or, by default, for any Crew work)."* +- Root `AGENTS.md` — *"Do not propose, draft, or open pull requests against + `block/buzz`; the upstream remote's push URL is disabled on purpose."* +- `docs/crew/UPSTREAM-SYNC.md:17` — *"The local upstream push URL is + deliberately disabled. Never push to `block/buzz`."* + +Per `docs/crew/IDENTITY.md:41` and AGENT-WORKING-AGREEMENT MUST #5, a conflict +between documents is surfaced, not silently resolved. + +**Precedent for the identical collision:** +`plans/20260805-1330-hermes-first-class-runtime/phase-01-upstream-tier1-pr.md` +was retargeted to `Nuncio-hq/crew` with the note that the upstream-targeted +version of that file is historical. + +## What this phase delivers while blocked + +A **Crew-owned draft artifact only** — no upstream remote interaction of any +kind. + +| Deliverable | Detail | +| --- | --- | +| `docs/crew/upstream-proposals/evidence-on-completion.md` (new, Crew-owned) | The generic, Crew-free section text: the evidence-by-change-type culture, the three token rules, the proportionality escape hatch. No `just desktop-screenshot`, no `--evidence`, no `crew-evidence` tag, no NuncioCrew naming | +| Note in the same file | That it is a proposal draft held under D-020, not a submitted PR, with the link to this phase | + +Writing the draft costs minutes and keeps the option open at zero risk. Phase 02 +should keep the Crew tooling pointers separable precisely so this text lifts out +cleanly. + +## Founder decision required (D-1) + +Choose one: + +- **A — Keep D-020 (default).** Phase 07 ends at the draft. DoD checkbox 1's + upstream clause is recorded as consciously deferred, not forgotten. No further + action. +- **B — Scoped exception.** Record a new entry in `docs/crew/DECISIONS.md` + authorizing this one upstream contribution, stating who submits it and under + what identity. Only then does anyone touch `block/buzz`. + +**Until the founder picks, this phase must not be executed beyond the draft.** +No agent may open, draft-in-GitHub, or push toward `block/buzz`. + +## Blocking status + +Blocked on D-1. Does **not** block phases 01-06 or 08-09 — the issue itself calls +this track "parallel, non-blocking". + +## Acceptance criteria + +- The generic draft exists and contains no Crew-specific vocabulary or tooling. +- The D-020 conflict is stated in the draft and in the PR description. +- **No PR, branch, or push exists against `block/buzz`.** +- If the founder chooses A, DoD checkbox 1's upstream clause is explicitly marked + deferred in the issue, with the reason. + +## Risk + +Zero technical risk. The real risk is process: an agent reading only the issue +and not the decision log would open an upstream PR and violate D-020. This file +is the guard against that. diff --git a/plans/20260810-evidence-thread-log/phase-08-decisions-and-state.md b/plans/20260810-evidence-thread-log/phase-08-decisions-and-state.md new file mode 100644 index 00000000000..eb5d38e0d24 --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-08-decisions-and-state.md @@ -0,0 +1,84 @@ +--- +phase: 08 +title: DECISIONS.md tag schema and known limit, STATE.md anti-drift +status: planned +priority: P1 +effort: S +dependencies: ["04", "05", "06"] +--- + +# Phase 08 — Decision record + state update + +Delivers DoD checkbox 5. + +## Why a decision entry + +The `crew-evidence` tag is a **sticky wire contract**. Once agents emit it and +the desktop renders it, changing the vocabulary breaks already-published room +history. D-025 requires recording any contract extension in `DECISIONS.md` with +the reason generic Buzz was insufficient. + +## `docs/crew/DECISIONS.md` — new entry (next free id, expected D-028) + +Must state: + +1. **The schema.** `["crew-evidence", ""]` on existing message kinds, with + the four values `test-run | metrics | before-after-visual | diff-stat`. First + occurrence wins. **No new event kind.** +2. **Why a tag and not a new kind.** Existing kinds carry it; other engines and + clients ignore an unknown tag safely (D-025 "prefer extensions that other ACP + engines can ignore safely"). Backed by the phase 01 spike verdict — cite it. +3. **Why the CLI appends post-build.** Keeps `buzz-sdk` and `buzz-core` at zero + additional Crew delta; precedent `crates/buzz-cli/src/client.rs:590`. +4. **The known limit, verbatim in substance from the issue.** Evidence is + self-reported; numbers and test excerpts *can* be fabricated. This slice + raises the cost of lying and the odds of getting caught — fabricated numbers + diverge from CI, fabricated screenshots diverge from the app — it does **not** + cryptographically verify work. Independent verification stays where it lives + today: CI and PR review. +5. **The scope limit.** Only the CLI can emit the tag this slice; the desktop + composer and mobile cannot. Only kind 9 renders a card. +6. **The ✅/❌ reuse** and the fact that `KIND_AGENT_RECEIPT` ignores the tag + (R-2). If the founder answers D-2 with a different glyph pair, record that + instead. +7. **Non-enforcement.** The ≤30-line evidence bound is a probe check and a prompt + rule, not a runtime guard (R-5). Do not write anything implying enforcement. + +## `docs/crew/STATE.md` — anti-drift + +Issue #117's rule: any PR changing shipped state updates `STATE.md` in the +**same** PR. Update: + +- **Current product slice** — evidence-on-thread-log shipped, with what works. +- **Verified evidence** — link the phase 09 probes and the Playwright evidence. +- **Open decisions** — add D-1 (upstream generic half under D-020) and, if still + unanswered, D-2 (glyph reuse). + +State only what is actually shipped and verified. Do not describe phase 07's +draft as an opened upstream PR. + +## Other docs + +| Doc | Update if | +| --- | --- | +| `docs/crew/UPSTREAM-SYNC.md` | phase 02 created the upstream-file-edit list — confirm it lists every file this slice touched, with final line counts | +| `crates/buzz-cli/TESTING.md` | it enumerates `messages send` flags | +| `desktop/src/features/agents/AGENTS.md` | any harness capability fact changed | + +## Acceptance criteria + +- The decision entry covers all seven points above and cites the spike. +- `STATE.md` reflects only shipped, verified behavior. +- The known limit appears in `DECISIONS.md`, not only in the issue. +- No doc claims independent verification, enforcement of the token bound, or an + opened upstream PR. + +## Validation + +Read each edited doc after writing and check every claim against source, tests +or live state. Verify every link resolves. + +## Risk + +Low technically, high in drift terms — a wrong or over-claiming decision entry +misleads every future agent that reads it as law. diff --git a/plans/20260810-evidence-thread-log/phase-09-live-probes-verification.md b/plans/20260810-evidence-thread-log/phase-09-live-probes-verification.md new file mode 100644 index 00000000000..4fc1171d353 --- /dev/null +++ b/plans/20260810-evidence-thread-log/phase-09-live-probes-verification.md @@ -0,0 +1,109 @@ +--- +phase: 09 +title: Live probes and Playwright evidence on the PR +status: planned +priority: P1 +effort: M +dependencies: ["04", "05", "06", "08"] +--- + +# Phase 09 — Live probes + PR evidence + +Delivers DoD checkbox 6 and the "agent-readable" half of checkbox 4. The +evidence feature's own PR must carry evidence. + +## Probe 1 — bugfix report end-to-end + +Against an isolated local relay: + +1. An agent completes a scripted bugfix task. +2. Its report message carries `crew-evidence: test-run` with a red→green excerpt. +3. The desktop renders the `test-run` card. +4. The owner clicks Accept. +5. A kind-7 `✅` lands on the relay against the evidence event id. +6. The card reflects the accepted state. +7. **The agent reads the verdict back** with the already-shipped command + (`crates/buzz-cli/src/lib.rs:746-774`): + + ```bash + buzz --format compact reactions get --event + ``` + + Step 7 is the agent-readable half of DoD checkbox 4. It needs **no new CLI + work** — only proof that it returns the reaction. + +Repeat the Reject path: `❌` lands and the reply composer opens with the evidence +message as parent. + +## Probe 2 — UI change, no computer-use + +1. An agent makes a small visual change. +2. It captures before/after with the **headless** harness — never "open the app + and capture": + + ```bash + just desktop-screenshot --name evidence-before + just desktop-screenshot --name evidence-after + ``` + +3. It sends the report with `--file` for both PNGs and + `--evidence before-after-visual`. +4. The desktop renders the images side by side. +5. Degrade check: with images blocked, the card still reads sensibly. + +## Probe 3 — token discipline spot-check + +- Every probe report stays within a small bound — the issue's example is + **≤ 30 lines of text per report**. +- **No work was re-executed solely to capture evidence.** Confirm from the + transcript: the artifact existed already. +- This is a spot-check, not an enforced guard (R-5). Report the observed line + counts; do not claim the bound is enforced. + +## Playwright evidence + +Specs from phase 03 cover all four card kinds plus reaction states. For the PR: + +- Scope each capture with `locator.screenshot()` — a full-page shot of a timeline + containing every card produces byte-identical PNGs (root `AGENTS.md`). +- Gate on distinctness **before** posting: + + ```bash + shasum -a 256 test-results//*.png # every hash must be unique + ``` + + Identical hashes mean two shots captured the same state. Fix the spec; do not + post. +- Post with `./scripts/post-screenshots.sh ` using + `{{filename}}` placeholders. **Never** `buzz upload` or a relay media URL — + those fail through GitHub's camo proxy. +- On repost, delete the superseded comment so reviewers do not see stale images. + +## CI + +- `just ci` green. +- The `base_prompt.md` edit carries its `UPSTREAM-SYNC.md` accounting entry in + the **same** PR. +- Crew's e2e smoke is flaky under load and Rust integration tests do not run in + the merge gate. Attribute every red shard individually before calling it a + regression, and never treat one scoped green run as merge authority (R-9). + +## Acceptance criteria + +- Probes 1-3 executed with recorded output, including the `reactions get` result. +- Four distinct card screenshots with unique `shasum` hashes, posted via + `post-screenshots.sh`. +- `just ci` green on the final head SHA. +- No computer-use anywhere in the flow. +- Probe results linked from `docs/crew/STATE.md` (phase 08). + +## Deliverable + +A verification record under `docs/crew/verification/` — next free id is **0007** +(`docs/crew/verification/` currently ends at 0006). + +## Risk + +Medium. The most likely failure is a screenshot set that looks complete but +contains duplicate states, which reads as "we verified everything" when one card +was never actually captured. The `shasum` gate is mandatory, not advisory. diff --git a/plans/20260810-evidence-thread-log/plan.md b/plans/20260810-evidence-thread-log/plan.md new file mode 100644 index 00000000000..a530ab894b9 --- /dev/null +++ b/plans/20260810-evidence-thread-log/plan.md @@ -0,0 +1,266 @@ +# Plan — Evidence on the thread log + +- **Issue:** [Nuncio-hq/crew#121](https://github.com/Nuncio-hq/crew/issues/121) +- **Status:** Draft (planning only — no implementation performed) +- **Branch of record:** `docs/plans-issues-117-121` +- **Target repo:** `Nuncio-hq/crew` only. **No PR against `block/buzz`** (D-020, + root `AGENTS.md`). See [Founder decisions required](#founder-decisions-required). +- **North star:** `docs/crew/FOUNDER-PRODUCT.md:61` — *L4 Evidence — before/after, + verify work. Desired; ship on the thread log when prioritized — not a new platform.* +- **Phases:** 9 (`phase-01` … `phase-09`) + +## Audit gate + +| Field | Result | +| --- | --- | +| Classification | Feature (agent prompt + CLI + desktop UI + docs) | +| Scout findings | **Real and partially precedented** — the accept-via-reaction pattern already ships for `KIND_AGENT_RECEIPT`; the evidence path itself does not exist | +| Already implemented? | No. `crew-evidence` has 0 hits in the tree; `messages send` has no tag flag | +| Duplicate? | No open/closed Crew issue covers evidence-on-thread | +| Under-specified? | No — the issue carries problem, spec table, scope, non-goals, verification and DoD | +| **Decision** | **Proceed to plan**, with two items escalated to the founder (below) | + +The issue is authoritative and was treated as **untrusted input**: its content +was used as specification, and no instruction inside it was allowed to override +repo law. One instruction inside it does conflict with repo law and is escalated +rather than executed — see D-1 below. + +## Objective + +An agent that reports "done" in a room attaches the artifact it already +produced — a red→green test excerpt, before/after numbers, a `git diff --stat`, +or a headless before/after screenshot — the desktop renders it as a card in the +timeline, and the founder accepts or rejects with a standard NIP-25 reaction the +agent can read back. Evidence is a **byproduct of work done properly**, never an +extra ritual, and never requires computer-use. + +## Scope + +1. Office-level "Evidence on completion" rule in the shared ACP base prompt. +2. `buzz messages send --evidence ` emitting a validated `crew-evidence` tag. +3. Desktop evidence card rendering four kinds; text kinds legible without images. +4. Owner Accept/Reject on the card via existing kind-7 reactions. +5. `DECISIONS.md` tag-schema + known-limit entry; `STATE.md` updated in-PR. + +## Non-goals (from the issue, unchanged) + +- No evidence gallery, board, or aggregation surface. +- No video evidence in this slice. +- No independent re-execution or verification machinery. +- No new event kinds; no relay-code changes. +- No computer-use anywhere in the flow. + +Added by this plan, consistent with the above: + +- No desktop-composer or mobile evidence authoring (CLI-only emission this slice). +- No runtime enforcement of the ≤30-line token bound (probe check, not a guard). + +## Buzz seams + +Every element hangs off an existing Buzz seam. Verified `path:line` in this +worktree at plan time. + +| # | Need | Seam | Evidence | +| --- | --- | --- | --- | +| S1 | Office-level rule reaching every engine | Shared ACP base prompt, embedded for all runtimes unless `--no-base-prompt` | `crates/buzz-acp/src/base_prompt.md:46` (`## Communication Patterns`); delivery `crates/buzz-acp/src/lib.rs:1942` `include_str!("base_prompt.md")` | +| S2 | Custom tag on an ordinary message kind | Upstream already ships a non-NIP tag on kind 9 | `crates/buzz-sdk/src/builders.rs:250` `FAILURE_NOTICE_TAG` | +| S3 | Append a tag without touching `buzz-sdk` | Post-build `EventBuilder::tags` append | `crates/buzz-cli/src/client.rs:590` `builder.tags([tag.clone()])` | +| S4 | CLI flag surface | `MessagesCmd::Send` clap variant | `crates/buzz-cli/src/lib.rs:398`; handler `crates/buzz-cli/src/commands/messages.rs:574` `cmd_send_message`, builder match `:664` | +| S5 | Image evidence transport | Existing Blossom upload → NIP-92 `imeta` | `crates/buzz-cli/src/commands/messages.rs:616-629` (`--file` → `build_imeta_tag`) | +| S6 | Tag reaching the desktop renderer | Timeline mapper already forwards raw tags | `desktop/src/features/messages/lib/formatTimelineMessages.ts:520` → `desktop/src/features/messages/types.ts:50` `tags?: string[][]` | +| S7 | Per-kind body rendering | `renderBody()` switch, Crew already owns a branch here | `desktop/src/features/messages/ui/MessageRow.tsx:368` (switch), `:400` (`KIND_AGENT_RECEIPT`), `:415` (`default:` — where kind 9 lands) | +| S8 | Accept via reaction, already shipping | Agent-receipt card sends ✅ through the normal reaction handler | `desktop/src/features/messages/ui/MessageRow.tsx:408` `onReviewed={() => handleReactionSelect("✅")}`; state read at `desktop/src/features/messages/ui/AgentReceiptMessageBody.tsx:45-50` | +| S9 | Reject that carries a reason | Same card wires "request changes" to the reply composer | `desktop/src/features/messages/ui/MessageRow.tsx:407` `onRequestChanges={onReply ? () => onReply(message) : undefined}` | +| S10 | Owner identity for gating actions | `profiles[pubkey].ownerPubkey === currentPubkey` | `desktop/src/features/messages/ui/AgentReceiptMessageBody.tsx` owner check | +| S11 | Agent reads the verdict back | **Already exists — no CLI work** | `buzz reactions get --event `, `crates/buzz-cli/src/lib.rs:746-774` | +| S12 | E2E injection of a tagged message | Mock bridge accepts arbitrary tags | `desktop/src/testing/e2eBridge.ts:1153` `extraTags?: string[][]` | + +**No seam is missing.** S11 means DoD checkbox 4's "agent-readable" half is +satisfied by shipped code and needs verification, not implementation. + +## Thin-fork accounting + +Line counts measured against `upstream/main` at plan time. + +| File | Owner | Crew delta today | This slice | Justification | +| --- | --- | --- | --- | --- | +| `crates/buzz-acp/src/base_prompt.md` | upstream | **0** (147 = 147) | **+ ≤18** (one section) | **First Crew edit to this file.** Office-level behavioral rule belongs in the office-level prompt; a per-agent Layer-3 path cannot cover the floor. Self-contained Markdown section — conflict cost is minutes, comparable to the accepted route/nav budget (`docs/crew/UPSTREAM-SYNC.md:26`) | +| `crates/buzz-acp/src/lib.rs` | upstream | +829 | **+ ~8** (one prompt-assertion test) | Follows the upstream test pattern at `upstream/main:crates/buzz-acp/src/lib.rs:3944` | +| `crates/buzz-cli/src/lib.rs` | upstream | +50 | **+ ~6** (one clap arg) | Established add-a-flag playbook; already a Crew-edited file | +| `crates/buzz-cli/src/commands/messages.rs` | upstream | **0** (1375 = 1375) | **+ ~14** | **First Crew edit.** Kept minimal by putting kind validation in a Crew-owned module and appending the tag post-build (S3) | +| `crates/buzz-sdk/src/builders.rs` | upstream | +407 | **0** | Avoided via S3 | +| `crates/buzz-core/src/kind.rs` | upstream | +21 | **0** | No new kinds (issue non-goal) | +| relay / `buzz-db` / `buzz-relay` | upstream | — | **0** | No relay changes (issue non-goal) | +| `desktop/…/MessageRow.tsx` | upstream-derived | +24 (**980 / 1000**) | **≤ 8** | See R-1: only ~20 lines of ratchet headroom; D-022 forbids raising `MAX_LINES` (`desktop/scripts/check-file-sizes.mjs:8`) | +| `docs/crew/UPSTREAM-SYNC.md` | Crew | — | new section | The issue says "record the edit in UPSTREAM-SYNC.md's upstream-file-edit list" — **that list does not exist yet** (see R-7); phase 02 creates it | +| New Crew-owned desktop + Rust files | Crew | — | new | Where all real logic lives | + +Total upstream-file footprint: **5 files, ~46 added lines**, of which two files +gain their first Crew delta. Everything else is additive Crew-owned code. + +## Generic-ACP check (D-025) + +| Element | Generic? | Reasoning | +| --- | --- | --- | +| Prompt rule | **Yes** | Lives in the shared base prompt embedded for every runtime (S1); no engine branch | +| `crew-evidence` tag | **Yes** | Emitted by `buzz messages send`, available to any engine that can run the CLI; other clients ignore an unknown tag | +| Desktop card | **Yes** | Keys off the tag value, never off the author's runtime or profile | +| Accept/Reject | **Yes** | Standard NIP-25 kind-7; readable by any engine via `buzz reactions get` | +| Image capture | **Engine-agnostic, repo-tool-dependent** | `just desktop-screenshot` needs a Crew checkout and shell access, not Hermes | + +**Hermes-only behavior in this slice: none.** Nothing in the design assumes +profile memory or profile-owned model config (FOUNDER-PRODUCT rule 4). + +Honest limit to state in docs: engines or surfaces that publish messages +*without* the CLI (desktop composer, mobile) cannot emit the tag this slice. + +## Proposed tag schema (becomes DECISIONS.md D-028) + +```text +["crew-evidence", ""] +kind ∈ { test-run | metrics | before-after-visual | diff-stat } +``` + +- Rides the existing message kinds; **no new event kind**. +- First occurrence wins; later duplicates ignored. +- CLI validates the value; **the renderer does not trust it** — an unrecognized + value falls back to the ordinary message body (forward-compatible, and safe + because tag content is agent-authored). +- `KIND_AGENT_RECEIPT` (46043) keeps its own card and **ignores** `crew-evidence` + (see R-2). + +## Phases + +| # | Title | Effort | Depends on | Gate | +| --- | --- | --- | --- | --- | +| 01 | Spike — unknown `crew-evidence` tag round-trip | S | — | Gate 1 | +| 02 | Office prompt rule + thin-fork accounting | M | 01 | Gate 4 | +| 03 | RED contract tests (CLI, desktop, reactions) | M | 01 | Gate 3 | +| 04 | CLI `--evidence ` | S | 03 | Gate 5 | +| 05 | Desktop evidence card (four kinds) | L | 03 | Gate 5 | +| 06 | Owner Accept/Reject via NIP-25 reactions | M | 05 | Gate 5 | +| 07 | Upstream generic half — **BLOCKED on founder decision** | S | 02 | — | +| 08 | DECISIONS.md schema + known limit, STATE.md anti-drift | S | 04, 05, 06 | Gate 6 | +| 09 | Live probes + Playwright evidence on the PR | M | 04, 05, 06, 08 | Gate 6 | + +Gate names refer to `docs/crew/DEVELOPMENT-WORKFLOW.md` +(Spike → RED tests → edge cases → approved plan → smallest implementation → +GREEN → review/docs). **Phases 04-06 must not start before phase 03 is RED and +this plan is approved** (AGENT-WORKING-AGREEMENT MUST NOT #8). + +## Definition of Done → phase map + +| # | DoD checkbox | Phase(s) | +| --- | --- | --- | +| 1 | base_prompt.md section + UPSTREAM-SYNC.md accounting in the same PR; upstream PR for the generic half (link recorded, not blocking) | **02** (section + accounting) · **07** (upstream half — blocked, see D-1) | +| 2 | `buzz messages send --evidence ` emits the validated tag | **03** (RED) · **04** (GREEN) | +| 3 | Desktop renders all four kinds; text kinds legible without images | **03** (RED) · **05** (GREEN) | +| 4 | Owner Accept/Reject end-to-end: send, persist, render, agent-readable | **03** (RED) · **06** (send/persist/render) · **09** (agent-readable via shipped `buzz reactions get`) | +| 5 | Known-limit statement and tag schema in DECISIONS.md; STATE.md updated in-PR | **08** | +| 6 | Live probes + Playwright evidence attached to the PR | **09** | + +Every checkbox maps to at least one phase. No issue requirement was dropped. + +## Founder decisions required + +### D-1 — The upstream PR in DoD checkbox 1 conflicts with repo law + +- **The issue asks (item 1, "Parallel, non-blocking"):** *"open an upstream PR to + `block/buzz` proposing the generic half."* +- **Repo law says:** D-020 — *"no pull request will be opened against + `block/buzz` for this feature (or, by default, for any Crew work)"*; root + `AGENTS.md` — *"Do not propose, draft, or open pull requests against + `block/buzz`; the upstream remote's push URL is disabled on purpose"*; + `docs/crew/UPSTREAM-SYNC.md:17` — *"The local upstream push URL is deliberately + disabled. Never push to `block/buzz`."* +- **This plan does not execute the upstream PR.** Phase 07 is written as + **BLOCKED** and delivers only a Crew-owned draft of the generic, Crew-free + section text, so the option stays open at zero cost. +- **Precedent for this exact collision:** + `plans/20260805-1330-hermes-first-class-runtime/phase-01-upstream-tier1-pr.md` + was retargeted to Crew with the note that the upstream-targeted version is + historical. +- **Ask:** keep D-020 (phase 07 stays a draft artifact), or record a scoped + exception in `DECISIONS.md` authorizing this one upstream contribution? + +### D-2 — ✅ now means two things on a Crew message + +`AgentReceiptMessageBody` already treats an owner ✅ as "reviewed" on kind 46043. +This plan reuses ✅ as "accept" on evidence-tagged kind 9. They never collide on +one event (different kinds, and 46043 ignores the tag — R-2), but the founder is +learning one glyph with two nearby meanings. + +- **Plan's default:** reuse ✅/❌ as the issue specifies ("no new semantics"). +- **Ask:** confirm, or pick a distinct pair for evidence? + +## Risks + +| # | Risk | Mitigation | Owner phase | +| --- | --- | --- | --- | +| R-1 | `MessageRow.tsx` is at 980/1000 lines; a naive card branch trips the ratchet and tempts someone to raise `MAX_LINES` (violating D-022) | Hard budget of ≤8 added lines; route through the Crew-owned `MessageRowDefaultBody`; if the budget cannot be met, extract Crew deltas out of `MessageRow.tsx` — never raise the limit | 05 | +| R-2 | Two competing review affordances if a receipt also carries the tag | kind 46043 keeps the receipt card and ignores `crew-evidence`; the evidence card is kind-9-only. Contract test asserts it | 03, 05 | +| R-3 | Fabricated evidence — self-report, not proof | Accepted and documented, per the issue's "Known limit"; recorded in `DECISIONS.md`, not only in the issue | 08 | +| R-4 | Prompt bloat: base_prompt is 147 lines and every turn of every agent pays it; a fat section contradicts the issue's own token-frugality principle | Hard cap ≤18 lines; compress the 7-row table to a compact list; assert the cap in the prompt test | 02 | +| R-5 | The ≤30-line token bound is unenforceable at runtime | Stated as a probe check, never as a guard. No claim of enforcement in docs | 02, 09 | +| R-6 | Playwright screenshots for four card kinds come out byte-identical | `locator.screenshot()` per card + `shasum -a 256` uniqueness gate before posting (root `AGENTS.md`) | 09 | +| R-7 | The issue assumes an UPSTREAM-SYNC.md "upstream-file-edit list" that **does not exist** | Phase 02 creates the section and seeds it with the files this slice touches | 02 | +| R-8 | Unknown tag could be stripped by the relay, silently breaking the whole design | Phase 01 spike proves round-trip before any production code | 01 | +| R-9 | Crew e2e smoke is flaky under load; a red shard could be misread as an evidence-card regression | Attribute each failure individually; never gate the merge on one scoped run | 09 | + +## Rollback / abort + +- **Phase 01 FAIL** (tag does not survive round-trip): stop. The wire design is + invalid; re-plan around a receipt-style body convention instead of a tag. Do + not start phases 02-09. +- **Post-merge revert:** the feature is additive. Reverting the CLI flag and the + desktop branch leaves already-published evidence messages rendering as ordinary + messages, and existing ✅/❌ reactions intact. No migration, no data loss. +- **Partial ship:** phases 02+04 (prompt + CLI) are shippable without 05/06 — + evidence lands in the log as plain text and stays readable. 05/06 are not + shippable without 04. + +## Validate pass + +Run against this plan on 2026-08-10. **Result: PASS.** + +| Check | Result | +| --- | --- | +| Every DoD checkbox maps to ≥1 phase | Pass — 6/6, table above | +| No issue requirement silently dropped or contradicted | Pass — the one contradiction (upstream PR) is escalated as D-1, not dropped | +| Every phase names a verified seam | Pass — S1-S12, all with `path:line` | +| Upstream-file edits justified with expected diff size | Pass — accounting table, ~46 lines across 5 files | +| Generic-ACP check performed (D-025) | Pass — no Hermes-only behavior; limits stated | +| Workflow gate order respected | Pass — spike → RED → implementation; 04-06 depend on 03 | +| Non-goals preserved | Pass — no new kinds, no relay edits, no gallery, no computer-use, no video | +| Anti-drift (#117): shipping PR updates STATE.md | Pass — phase 08, and restated in every implementation phase | +| PR target | Pass — `Nuncio-hq/crew` only; no upstream PR executed | +| Phase files carry required frontmatter | Pass — `phase`, `title`, `status`, `priority`, `effort`, `dependencies` | +| No implementation performed during planning | Pass — files on disk are plan artifacts only | + +## Red-team pass + +Adversarial review of this plan on 2026-08-10. **9 findings; 8 applied, 1 +escalated.** + +| # | Finding | Disposition | +| --- | --- | --- | +| RT-1 | "Just add a case to `MessageRow.tsx`" ships a ratchet violation; the tempting fix is raising `MAX_LINES`, which D-022 forbids outright | **Applied** — R-1 + explicit ≤8-line budget and extraction fallback in phase 05 | +| RT-2 | The plan originally reused ✅ without noticing `AgentReceiptMessageBody` already binds ✅ to "reviewed" | **Applied** — R-2 (kind isolation + contract test) and escalated as D-2 | +| RT-3 | A 7-row table plus 3 rules plus tooling pointers is ~30 prompt lines on *every* turn for *every* agent — the feature would violate its own token-frugality principle | **Applied** — R-4, hard ≤18-line cap asserted by test in phase 02 | +| RT-4 | "Validated tag" could be read as validating that evidence *exists*; it only validates the enum | **Applied** — stated in phase 04 and in the schema section | +| RT-5 | The issue's UPSTREAM-SYNC.md "upstream-file-edit list" does not exist — a phase written against it would fail on contact | **Applied** — R-7; phase 02 creates the section | +| RT-6 | `buzz reactions get` already exists, so a naive plan would add a redundant CLI command for DoD 4 | **Applied** — S11; phase 09 verifies rather than implements | +| RT-7 | Rejection without a reason is a dead end for the agent | **Applied** — phase 06 wires ❌ to the reply composer, mirroring `MessageRow.tsx:407` | +| RT-8 | Editing `base_prompt.md` creates a permanent conflict surface in a file with zero Crew delta today | **Applied** — accounted in the thin-fork table with a resolve hint; the section is self-contained Markdown | +| RT-9 | DoD checkbox 1 cannot be fully satisfied under D-020 | **Escalated, not applied** — D-1. The plan will not silently drop the checkbox nor silently open an upstream PR | + +**Blocking findings remaining: none.** D-1 and D-2 are founder decisions that +gate phase 07 and confirm phase 06's glyph choice respectively; phases 01-05 and +08-09 can proceed without them. + +## Constraints on execution + +- All PRs target `Nuncio-hq/crew`. Never `block/buzz` (D-020). +- Any PR that changes shipped state updates `docs/crew/STATE.md` in the same PR (#117). +- `git commit -s` on every commit (DCO); `just ci` green before PR. +- Desktop Tauri fmt must be run from the main checkout, not this worktree. diff --git a/plans/20260810-hermes-profile-editing/ISSUE-COMMENT.md b/plans/20260810-hermes-profile-editing/ISSUE-COMMENT.md new file mode 100644 index 00000000000..03b0265a1eb --- /dev/null +++ b/plans/20260810-hermes-profile-editing/ISSUE-COMMENT.md @@ -0,0 +1,59 @@ +## Issue-to-Plan Handoff — #118 + +**Decision:** proceed to plan. 10 phases in `plans/20260810-hermes-profile-editing/`. +Planning-only run — no branch, no PR, no code. All 5 DoD checkboxes map to ≥1 phase. + +### Phases + +| # | Title | Effort | Depends | +| - | ----- | ------ | ------- | +| 01 | Spike — Hermes config + SOUL.md read/write feasibility | S | — | +| 02 | RED contract tests (17 contracts) | M | 01 | +| 03 | Runtime capability descriptor | S | 01, 02 | +| 04 | Profile model read/write IPC | M | 01, 02 | +| 05 | SOUL.md read/write/reset IPC | M | 01, 02 | +| 06 | Model write-through UI | M | 03, 04 | +| 07 | SOUL.md editor UI + persona at birth | M | 03, 05 | +| 08 | Description optionality + layer copy | S | 02 | +| 09 | Docs truth + STATE anti-drift | M | 06, 07, 08 | +| 10 | Verification & evidence | S | 09 | + +### Key design decisions + +1. **Crew is a remote control, not a second store** — every write goes through + Hermes' own CLI/file with a read-back; Crew persists no model and no persona. +2. **Capability descriptor with zero upstream Rust edits (Option A)** — derive + `{modelSource, personaDoc, layer3}` at the Crew-owned catalog projection from + `profileArg`/`providerLocked`/`modelEnvVar`. Declaring it on `KnownAcpRuntime` + (Option B) costs 2 upstream files + `UPSTREAM-SYNC.md` approval — escalation only. +3. **D-019 item 2 superseded in its presentation half only.** `HERMES.md` rule 2, + feature 0001 C-04, and `desktop/src/features/agents/AGENTS.md` rules 3+8 all say + the opposite today and are updated in the same PR (new decision **D-028**). +4. **Layer-3 optionality is mostly already shipped** (`types.rs:119` maps empty → + `None`) — Phase 08 is contract-proof + copy + docs, not a new mechanism. +5. Upstream budget: `lib.rs` +4 (command registration), agents `AGENTS.md` ~15, + both agent dialogs ≤10 each — mount-only, since both are already over the + 1000-line ratchet, so all new markup lives in Crew-owned children. + +### Named Buzz seams + +`buzz-acp/src/acp.rs:2355` (`SystemPromptTransport`) · `pool.rs:255` +(`session_new_system_prompt`) · `lib.rs:1942` (L2 `base_prompt.md`) · +`managed_agents/runtime.rs:680` (`BUZZ_ACP_SYSTEM_PROMPT`) · `types.rs:119` +(empty→`None`) · `types/crew.rs:7` · `discovery/runtime_metadata.rs:41,43,70` · +`hermes_profile_lifecycle.rs:153,268,362` (CLI write-through precedent) · +`commands/hermes_profiles.rs:11` · `shared/api/fromRawAcpRuntimeCatalog.ts:71` · +`agents/lib/agentConfigCore.ts:217` · `ui/EditAgentModelAndProfileSection.tsx:71` + +### Open questions (none block Phase 01) + +1. **Reset-to-default source for `SOUL.md`** — Hermes template, throwaway-profile + copy, or none. A bundled Crew copy is rejected (drift); with no trustworthy + source the editor ships without the reset button. +2. **Model input shape** — recommend free-text + validation; `get_agent_models` + is keyed on adapter env/provider config, not on a Hermes profile. +3. **Does a `SOUL.md` edit apply without respawn?** UI copy depends on it. +4. **Option A vs B** for the descriptor — B needs founder approval. + +Validate **pass** (11 checks, 1 revision). Red-team **6 findings, 5 applied, 1 +recorded-not-applied**. Details in `plan.md`. diff --git a/plans/20260810-hermes-profile-editing/phase-01-spike-hermes-config-soul.md b/plans/20260810-hermes-profile-editing/phase-01-spike-hermes-config-soul.md new file mode 100644 index 00000000000..6175580c5a8 --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-01-spike-hermes-config-soul.md @@ -0,0 +1,75 @@ +--- +phase: 01 +title: Spike — Hermes config + SOUL.md read/write feasibility +status: planned +priority: critical +effort: S +dependencies: [] +--- + +# Phase 01 — Spike: Hermes config + SOUL.md read/write feasibility + +Issue #118 plan item 1. This is a **read-mostly investigation**, not +implementation. It exists because the whole plan rests on four facts nobody has +verified end to end. + +## Gate + +D-008 / Crew workflow: spike before RED tests, RED tests before implementation. +P02–P10 must not start until this phase's evidence note exists. + +## Questions to answer + +| # | Question | Why it blocks | Method | +| - | -------- | ------------- | ------ | +| Q1 | Is there a non-interactive read path for the profile's model? (`hermes -p config get model.default` / `model.provider` or equivalent) | P04 has nothing to read | Run against a real local profile; capture exit code + stdout shape | +| Q2 | Does `hermes -p config set model.provider …` / `model.default …` succeed non-interactively, and what does it do on a **bad** model id or unauthenticated provider? | P04 error classification + R-4 (do not brick the agent) | Set a valid value, then a deliberately invalid one; record exit code and stderr first line | +| Q3 | Where does a fresh profile's default `SOUL.md` come from — a template in the Hermes install, generated text, or neither? | "Reset to Hermes default" (unresolved question 1) has no source otherwise | Inspect the Hermes install; create a throwaway profile in a temp `HERMES_HOME` and diff its `SOUL.md` | +| Q4 | Does an edited `SOUL.md` take effect on the next fresh ACP session without a respawn (same semantics as C-07 for model)? | P07 UI copy is wrong if not | Edit, `!rotate`, run a turn, observe | +| Q5 | Which `SystemPromptTransport` variant does the Hermes adapter path take, and does `prompt: None` really yield a `session/new` payload with **no** system-prompt field? | P08 contract test asserts a payload shape | Read `crates/buzz-acp/src/pool.rs:255` + `acp.rs:2355`; confirm with a captured payload | +| Q6 | Are the profile's `provider_models_cache.json` / `models_dev_cache.json` reliable enough to seed a model list? | Unresolved question 2 (free-text vs list) | Inspect structure and staleness on ≥2 real profiles | +| Q7 | Can the capability descriptor be derived from already-projected catalog facts (`profileArg`, `providerLocked`, `modelEnvVar`) for hermes / claude-code / codex, without an upstream Rust edit? | Decides Option A vs Option B | Read the catalog projection; enumerate every runtime's fact triple | + +## Files to read (no writes) + +| Path | For | +| ---- | --- | +| `crates/buzz-acp/src/acp.rs:2355` | `SystemPromptTransport` (S-1) | +| `crates/buzz-acp/src/pool.rs:255` | `session_new_system_prompt` (S-2) | +| `crates/buzz-acp/src/lib.rs:1942` | Layer-2 `base_prompt.md` (S-3) | +| `desktop/src-tauri/src/managed_agents/runtime.rs:680` | `BUZZ_ACP_SYSTEM_PROMPT` set/remove (S-4) | +| `desktop/src-tauri/src/managed_agents/types.rs:119` | empty description → `None` (S-5) | +| `desktop/src-tauri/src/managed_agents/hermes_profile_lifecycle.rs:153,268,362` | CLI invocation + `run_hermes` + result enum (S-9) | +| `desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs:41,43,70` | catalog fact triple (S-7) | +| `desktop/src/shared/api/fromRawAcpRuntimeCatalog.ts:71` | projection boundary (S-8) | + +## Safety rules for this spike + +- Use a **throwaway profile under a temporary `HERMES_HOME`** for any `config set` + experiment. Do not mutate `builder`, `scout`, `crewmission`, or + `missionacceptance` beyond a value you restore immediately. +- **Never** print `auth.json`, credential values, or non-model config values. + Key *names* only, as during planning. +- Never bind or touch the manager's default profile (`~/.hermes`) — D-019 item 1. +- Read-only on the repo. No production code in this phase. + +## Deliverable + +A spike note (repo convention: `docs/crew/spikes/` numbered note, or +`plans/reports/`) containing: + +1. Verbatim command + exit code + redacted output for Q1, Q2, Q3. +2. A decision line for each of Q1–Q7: **confirmed / refuted / blocked**. +3. Option A vs Option B recommendation for the capability descriptor, with the + fact triple table for every runtime in the catalog. +4. Any Hermes-side ask that must be filed (e.g. no machine-readable config read). + +## Exit criteria + +- Q1, Q2, Q5, Q7 answered — these gate P02/P03/P04/P08. +- Q3, Q4, Q6 answered or explicitly marked blocked with the consequence named + (reset button dropped / UI copy adjusted / free-text model input). +- **If Q2 is refuted** (no non-interactive write with distinguishable failure): + P04 and P06 are dropped from scope, the read-only model row stays, and the + plan's abort criterion for P04 applies. Do not hand-edit `config.yaml` as a + substitute. diff --git a/plans/20260810-hermes-profile-editing/phase-02-red-contract-tests.md b/plans/20260810-hermes-profile-editing/phase-02-red-contract-tests.md new file mode 100644 index 00000000000..fbcc3ba2023 --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-02-red-contract-tests.md @@ -0,0 +1,73 @@ +--- +phase: 02 +title: RED contract tests +status: planned +priority: critical +effort: M +dependencies: [01] +--- + +# Phase 02 — RED contract tests + +Issue #118 plan item 6 ("Contract tests first (RED)"). Every contract below +must **fail** before any implementation phase starts, and each names the phase +that turns it green. + +## Contracts + +| ID | Scenario | Expected | Forbidden | Turns green in | +| -- | -------- | -------- | --------- | -------------- | +| E-01 | Read model for a bound profile | Crew reports the profile's current `model.provider` + `model.default` | Crew reading or writing `config.yaml` directly | 04 | +| E-02 | Write a valid model from Crew | Profile reflects the new value on re-read; Crew stores nothing locally | A Crew-side cached model treated as source of truth | 04 | +| E-03 | Write an invalid model id / unauthenticated provider | Classified failure surfaced with the previous value still intact and recoverable | Silent success, or an agent left unable to run a turn | 04 | +| E-04 | Read `SOUL.md` for an existing profile | Real current file content returned | Empty string standing in for "couldn't read" | 05 | +| E-05 | Write `SOUL.md` round-trip | Byte-exact round-trip; unchanged file when the editor is opened and closed without edits | Truncation, re-encoding, or blank-replace | 05 | +| E-06 | `SOUL.md` for a missing / orphaned profile | Named failure (reuse the `hermes_profile_lifecycle` result-enum shape, S-9) | Panic, `unwrap`, or a silently created profile directory | 05 | +| E-07 | Capability descriptor for hermes | `{ modelSource: "profileWriteThrough", personaDoc: "soulMd", layer3: "append" }` | Any `runtime.id === "hermes"` comparison in render code | 03 | +| E-08 | Capability descriptor for claude-code / codex / unknown runtime | `{ modelSource: "adapterSetting", personaDoc: "none", layer3: "append" }` | Hermes-only UI leaking into another runtime (C-15) | 03 | +| E-09 | Field model for a profile-bound runtime | Model field is **editable with write-through**, no longer an `ownedByProfile` omission (S-11, `agentConfigCore.ts:217`) | Silent removal of the omission concept for non-profile runtimes | 03, 06 | +| E-10 | Empty Crew description at spawn | `BUZZ_ACP_SYSTEM_PROMPT` is **removed**, not set empty (S-4, `runtime.rs:680`) | An empty-string env var | 08 | +| E-11 | Empty Crew description at `session/new` | Payload carries **no** system-prompt field at all (S-1/S-2/S-5) | `systemPrompt: ""` or `_meta.systemPrompt: {append: ""}` | 08 | +| E-12 | Non-empty description | Existing Layer-3 append behaviour unchanged for every engine | Any change to the shipped injection path | 08 | +| E-13 | `BUZZ_ACP_MODEL` strip guard for profile-locked runtimes | Still stripped at spawn (C-05/C-06) | Re-introducing the env var because Crew now edits models | 04, 06 | +| E-14 | Duplicate binding, keep/delete offboarding, orphan repair | C-10, C-13, C-14 unchanged | Regression from new IPC surface | 04, 05 | +| E-15 | Playwright: Hermes agent open in edit | Editable model control + the shared-everywhere note visible | The old read-only `profile-owned-model-row` copy | 06 | +| E-16 | Playwright: SOUL editor | Opens populated with real current content; save persists | A blank textarea presented as the persona | 07 | +| E-17 | Playwright: empty description | Create/save succeeds with an empty "Agent instructions" box, labelled optional | A required-field block | 08 | + +## Where the tests live + +| Layer | Location | +| ----- | -------- | +| Rust unit (model + soul) | colocated `#[cfg(test)]` in the new Crew-only `hermes_profile_config.rs` / `hermes_profile_soul.rs` | +| Rust contract (transport payload) | `crates/buzz-acp` test module next to the existing `base_prompt` tests (`lib.rs:4547` area) | +| Rust contract (spawn env) | `desktop/src-tauri/src/managed_agents` tests near the existing runtime tests | +| TS unit | `desktop/src/features/agents/lib/runtimeCapabilities.test.*`, `agentConfigCore` tests | +| Playwright | `desktop/tests/e2e/hermes-profile-binding.spec.ts`; split into a sibling spec if the file grows past the ratchet | + +## Mock bridge work + +`desktop/tests/helpers/bridge.ts` already exposes `hermesProfiles?: string[]` +(the `list_hermes_profiles` mock). E-15…E-17 need the same treatment for the new +commands: seeded model values and seeded `SOUL.md` content, plus a failure mode +so E-03's UI path is exercisable in mock mode. + +## Verification + +```bash +# Rust (desktop crate is NOT in the root workspace) +cargo test --manifest-path desktop/src-tauri/Cargo.toml hermes_profile +cargo test -p buzz-acp system_prompt + +# TS +cd desktop && pnpm test -- runtimeCapabilities agentConfigCore + +# Playwright — build:e2e is mandatory, a plain build strips the mock bridge +cd desktop && pnpm test:e2e:smoke +``` + +## Exit criteria + +All 17 contracts exist and **fail for the right reason** (missing behaviour, not +a typo or a missing mock). Record the RED run output; it is part of the DoD-5 +evidence attached to the PR. diff --git a/plans/20260810-hermes-profile-editing/phase-03-runtime-capability-descriptor.md b/plans/20260810-hermes-profile-editing/phase-03-runtime-capability-descriptor.md new file mode 100644 index 00000000000..0fd362882cf --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-03-runtime-capability-descriptor.md @@ -0,0 +1,92 @@ +--- +phase: 03 +title: Runtime capability descriptor +status: planned +priority: high +effort: S +dependencies: [01, 02] +--- + +# Phase 03 — Runtime capability descriptor + +Issue #118 § Design constraint. Gives every runtime a declared +`{ modelSource, personaDoc, layer3 }` so P06/P07 render from a capability, not +from a harness id. This is the D-025 obligation and the +`desktop/src/features/agents/AGENTS.md` rule-1 obligation in one place. + +## Deliverable + +```text +Hermes : { modelSource: "profileWriteThrough", personaDoc: "soulMd", layer3: "append" } +Claude Code / Codex : { modelSource: "adapterSetting", personaDoc: "none", layer3: "append" } +Unknown / custom : { modelSource: "adapterSetting", personaDoc: "none", layer3: "append" } +``` + +## Design — Option A (default, zero upstream edits) + +Derive the descriptor **once**, at the Crew-owned catalog projection boundary +`desktop/src/shared/api/fromRawAcpRuntimeCatalog.ts:71` (S-8), from facts the +Rust catalog already projects through `types/crew.rs:7` (S-6): + +| Fact | Source | `path:line` | +| ---- | ------ | ----------- | +| `profileArg` | `KnownAcpRuntime::profile_arg` | `discovery/runtime_metadata.rs:70` | +| `providerLocked` | `KnownAcpRuntime::provider_locked` | `discovery/runtime_metadata.rs:43` | +| `modelEnvVar` | `KnownAcpRuntime::model_env_var` | `discovery/runtime_metadata.rs:41` | + +`profileArg && providerLocked && !modelEnvVar` is exactly the predicate +`runtimeOwnsModelViaProfile` already uses +(`desktop/src/features/agents/lib/hermesProfileBinding.ts:24`). The descriptor +renames what that predicate means rather than inventing a parallel truth: the +same shape now yields `modelSource: "profileWriteThrough"` instead of "render a +read-only row". + +**The one fact that cannot be derived** is the persona filename. `SOUL.md` is +named once, as data, in the new Crew-only capability module — never as a branch +inside a component. That limitation is stated in `plan.md` and is the trigger +for Option B. + +### Option B — escalation only + +Declare `persona_doc` on `KnownAcpRuntime` and project it through +`discovery.rs:1273` + `types/crew.rs`. Cost: ~10 lines across **two upstream +Rust files**. `UPSTREAM-SYNC.md` requires a failed or insufficient non-Rust +spike **plus explicit approval** before taking it. Only P01/Q7 can authorise +this, and the founder must approve. + +## Files + +| Path | Owner | Change | +| ---- | ----- | ------ | +| `desktop/src/features/agents/lib/runtimeCapabilities.ts` | **new, Crew-only** | descriptor type + `deriveRuntimeCapabilities(entry)` + the persona-filename table | +| `desktop/src/shared/api/acpRuntimeCatalogTypes.ts` (67 lines) | Crew-only | add `capabilities` to the TS catalog type | +| `desktop/src/shared/api/fromRawAcpRuntimeCatalog.ts` (77 lines) | Crew-only | one line: `capabilities: deriveRuntimeCapabilities(entry)` | +| `desktop/src/features/agents/lib/hermesProfileBinding.ts` | Crew-only | re-express `runtimeOwnsModelViaProfile` in terms of the descriptor; keep the exported name so callers do not churn | +| `desktop/src/features/agents/lib/agentConfigCore.ts` (upstream, +90 Crew lines already) | upstream | replace the `ownedByProfile` model omission at `:217` with an editable-write-through field for `modelSource === "profileWriteThrough"`; **~12 lines**, no restructuring | + +**Upstream edits this phase:** `agentConfigCore.ts` only. Justified: the field +model is the single place that decides which controls exist, it already carries +an accepted Crew delta, and the alternative (a parallel Crew field model) is +exactly the "copied upstream implementation" anti-pattern in +`UPSTREAM-SYNC.md` § Fork-drift review. + +## Turns green + +E-07, E-08, E-09. + +## Watch out + +- `agentConfigCore.ts:317` also reads `omission.kind === "model" && omission.reason === "ownedByProfile"`. + Both sites must move together or the model control will render twice. +- The `ownedByProfile` omission concept must survive for any future runtime that + genuinely cannot expose a model — do not delete the reason, stop *using* it + for Hermes. +- C-15: a non-Hermes runtime must render byte-identically to today. + +## Verification + +```bash +cd desktop && pnpm test -- runtimeCapabilities agentConfigCore +just desktop-typecheck +cd desktop && pnpm check && pnpm check:file-sizes +``` diff --git a/plans/20260810-hermes-profile-editing/phase-04-profile-model-ipc.md b/plans/20260810-hermes-profile-editing/phase-04-profile-model-ipc.md new file mode 100644 index 00000000000..fa869a4410a --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-04-profile-model-ipc.md @@ -0,0 +1,102 @@ +--- +phase: 04 +title: Profile model read/write IPC +status: planned +priority: high +effort: M +dependencies: [01, 02] +--- + +# Phase 04 — Profile model read/write IPC + +Issue #118 thing-to-solve 1, backend half. Crew reads and writes the bound +profile's model **through Hermes' own CLI** and stores nothing. + +## Invariant (red-team R-3) + +> Crew persists no model for a profile-bound agent. The profile is the store. +> Every displayed value is a read; every write is followed by a read-back. + +A React Query cache is a cache, not a source of truth — it must be invalidated +on write and on dialog open. + +## Deliverable + +| Command | Shape | +| ------- | ----- | +| `read_hermes_profile_model(name)` | `{ provider, model }` or a named failure | +| `write_hermes_profile_model(name, provider, model)` | result enum; on success returns the **re-read** values | + +Exact CLI syntax comes from P01/Q1+Q2. Baseline documented in +`docs/crew/HERMES.md:71-72`: + +```bash +hermes -p config set model.provider +hermes -p config set model.default +``` + +## Design — reuse the shipped write-through precedent + +`hermes_profile_lifecycle.rs` (S-9) is the pattern to copy, not re-invent: + +| Element | `path:line` | Reuse | +| ------- | ----------- | ----- | +| `run_hermes(binary, args)` | `hermes_profile_lifecycle.rs:362` | same invocation helper | +| `first_error_line(combined)` | `hermes_profile_lifecycle.rs:378` | same error extraction | +| `create_profile_with(name, cmd)` injection seam | `hermes_profile_lifecycle.rs:153` | same testability seam — tests never shell out to a real `hermes` | +| `HermesProfileLifecycleResult` tagged enum | `hermes_profile_lifecycle.rs` | same result shape family (`Ok` / `DoesNotExist` / `BinaryMissing` / `Failed`) | +| `validate_hermes_profile_name` | `managed_agents/hermes_profile.rs` | reject `default` and malformed names before shelling out | + +## Failure classification (red-team R-4 — do not brick the agent) + +| Class | Cause | Surfaced as | +| ----- | ----- | ----------- | +| `BinaryMissing` | `hermes` not on the app's PATH | same copy as the existing MissingBinary path (`HERMES.md` § Failure classes) | +| `DoesNotExist` | orphaned binding | route to the existing recreate/rebind repair, do not create a profile | +| `Rejected` | invalid model id / unknown provider | first stderr line, previous value preserved | +| `Failed` | anything else | first stderr line, previous value preserved | + +On any non-`Ok`, the previously read value stays displayed and remains the +profile's value — a failed write must never leave the agent with an empty model +(`model: String should have at least 1 character`, `HERMES.md:166`). + +## Files + +| Path | Owner | Change | +| ---- | ----- | ------ | +| `desktop/src-tauri/src/managed_agents/hermes_profile_config.rs` | **new, Crew-only** | read/write + result enum + `#[cfg(test)]` unit tests | +| `desktop/src-tauri/src/commands/hermes_profiles.rs` (Crew-only, S-10) | Crew-only | two new `#[tauri::command]` wrappers beside `list/create/delete` at `:11,17,29` | +| `desktop/src-tauri/src/lib.rs` | **upstream** | **+2 lines** in `invoke_handler`, next to `:795-797`. Justified: Tauri has no out-of-tree command registration; identical to the already-accepted Hermes lifecycle delta | +| `desktop/src/shared/api/hermesProfiles.ts` (Crew-only, S-14) | Crew-only | TS wrappers + an auditable command-line helper matching `hermesProfileCreateCommandLine` at `:62` | +| `desktop/tests/helpers/bridge.ts` | Crew-only test helper | seed model values + a failure mode for mock mode | + +## Security + +- Touch `model.provider` and `model.default` only. Never read, log, or render + any other config key, and never `auth.json`. +- Never parse or rewrite `config.yaml` directly — the CLI is the only write path. +- Do not log full command output; use `first_error_line` as the lifecycle module + already does. + +## Turns green + +E-01, E-02, E-03; keeps E-13 and E-14 green. + +## Abort + +If P01/Q2 is refuted (no non-interactive write with a distinguishable failure), +drop P04 and P06, keep the read-only model row, ship P05/P07/P08, and file the +Hermes-side ask. Do not substitute direct `config.yaml` editing. + +## Verification + +```bash +cargo test --manifest-path desktop/src-tauri/Cargo.toml hermes_profile_config +just desktop-tauri-test +``` + +Manual (real profile, after the automated pass): + +1. Read model for `scout` in Crew → matches `hermes -p scout config get …`. +2. Write a valid model → re-read shows it; profile file reflects it. +3. Write garbage → classified error, previous value intact, agent still runs. diff --git a/plans/20260810-hermes-profile-editing/phase-05-profile-soul-ipc.md b/plans/20260810-hermes-profile-editing/phase-05-profile-soul-ipc.md new file mode 100644 index 00000000000..cffaebdadbc --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-05-profile-soul-ipc.md @@ -0,0 +1,99 @@ +--- +phase: 05 +title: SOUL.md read/write/reset IPC +status: planned +priority: high +effort: M +dependencies: [01, 02] +--- + +# Phase 05 — SOUL.md read/write/reset IPC + +Issue #118 thing-to-solve 2, backend half. Layer 1 (the profile's own persona +document) becomes readable and writable from Crew. + +## Why this matters (evidence) + +Every real profile on the manager's machine has a **one-line `SOUL.md`** — +`builder`, `scout`, and `crewmission` all still carry the generic default. The +issue's claim that "nobody edits it, so they all sound generic" is corroborated +on disk, not assumed. + +## Deliverable + +| Command | Shape | +| ------- | ----- | +| `read_hermes_profile_soul(name)` | current file content, or a named failure | +| `write_hermes_profile_soul(name, content)` | result enum; content written verbatim | +| `reset_hermes_profile_soul(name)` | restores the Hermes default — **only if P01/Q3 found a trustworthy source** | + +## Hard product rule (from the issue) + +> Edit-in-place on the **real current content**. **Never a blank-replace box.** + +The editor must open populated. An unreadable file is a named failure, not an +empty string — presenting empty content as "the persona" invites the founder to +save over real prose. + +## Design + +- Path: `hermes_profile_dir(name)` (`hermes_profile_lifecycle.rs:103`) joined + with `SOUL.md`. `hermes_home()` at `:87` already honours the `HERMES_HOME` + override, so tests can point at a temp dir. +- Existence check reuses `hermes_profile_directory_exists` + (`hermes_profile_lifecycle.rs:108`) — a missing profile is `DoesNotExist`, and + the command must **never create** a profile directory as a side effect. +- Name validation reuses `validate_hermes_profile_name` and the `default` + rejection (`managed_agents/hermes_profile.rs`) before any filesystem access. +- Writes are atomic (temp file in the same directory + rename) so an interrupted + save cannot leave a truncated persona. +- Read and write are byte-exact: no trailing-newline normalisation, no CRLF + rewriting, no re-encoding. + +### Reset source (unresolved question 1) + +P01/Q3 decides one of: + +| Source | Verdict | +| ------ | ------- | +| Template shipped inside the Hermes install | **Preferred** — Hermes stays the source of truth | +| Create a throwaway profile in a temp `HERMES_HOME` and copy its `SOUL.md` | Acceptable fallback; side-effect-free because it never touches `~/.hermes` | +| A copy bundled in Crew | **Rejected** — drifts from Hermes and would make Crew a second store | + +If none is trustworthy, **drop `reset_hermes_profile_soul` and the reset button** +and record it as a known gap in `HERMES.md`. Edit-in-place still ships. + +## Files + +| Path | Owner | Change | +| ---- | ----- | ------ | +| `desktop/src-tauri/src/managed_agents/hermes_profile_soul.rs` | **new, Crew-only** | read/write/reset + result enum + `#[cfg(test)]` tests against a temp `HERMES_HOME` | +| `desktop/src-tauri/src/commands/hermes_profiles.rs` | Crew-only | up to three `#[tauri::command]` wrappers | +| `desktop/src-tauri/src/lib.rs` | **upstream** | **+2 or +3 lines** in `invoke_handler` next to `:795-797`. Same justification as P04 | +| `desktop/src/shared/api/hermesProfiles.ts` | Crew-only | TS wrappers | +| `desktop/tests/helpers/bridge.ts` | Crew-only test helper | seeded `SOUL.md` content + an unreadable-file failure mode | + +## Security and privacy + +- `SOUL.md` is founder-authored prose and may contain business context. It stays + local: never published to a relay, never quoted in an issue or PR body, and + never included in a posted screenshot without explicit founder approval. +- No `unsafe`; no new `unwrap()`/`expect()` on production paths — `?` and typed + errors (root `AGENTS.md` quality gates). +- Never read any other file in the profile directory. `auth.json`, `memories/`, + `sessions/`, `skills/`, and `plans/` are out of scope (issue non-goals). + +## Turns green + +E-04, E-05, E-06. + +## Verification + +```bash +cargo test --manifest-path desktop/src-tauri/Cargo.toml hermes_profile_soul +just desktop-tauri-test +``` + +Manual: open `scout`'s persona in Crew, confirm it matches the file on disk +byte-for-byte, save an edit, confirm the file changed and nothing else in the +profile directory did. diff --git a/plans/20260810-hermes-profile-editing/phase-06-model-write-through-ui.md b/plans/20260810-hermes-profile-editing/phase-06-model-write-through-ui.md new file mode 100644 index 00000000000..8cca0a926bb --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-06-model-write-through-ui.md @@ -0,0 +1,83 @@ +--- +phase: 06 +title: Model write-through UI +status: planned +priority: high +effort: M +dependencies: [03, 04] +--- + +# Phase 06 — Model write-through UI + +Issue #118 thing-to-solve 1, UI half. The founder sees and changes the model +inside Crew, and is told plainly what that means. + +## Copy the issue requires + +> **This model belongs to profile `` — changing it here changes it +> everywhere the profile runs, not just in Crew.** + +This note is mandatory whenever `modelSource === "profileWriteThrough"`. It is +the honesty requirement from `FOUNDER-PRODUCT.md` rule 4 in UI form: the founder +must never think Crew is holding a private setting. + +## Behaviour + +| State | Render | +| ----- | ------ | +| Bound profile, model readable | Editable model + provider control seeded from the profile, plus the shared-everywhere note | +| Read fails (`BinaryMissing` / `DoesNotExist`) | Fall back to the existing informational row and the existing repair path — never an empty editable field implying "no model" | +| Write succeeds | Re-read from the profile and display the read-back value; invalidate the query | +| Write fails | Keep the previous value, show the classified message from P04, leave the control recoverable (red-team R-4) | +| Non-profile runtime | Exactly today's behaviour (C-15) | +| Discovery not yet run | Same deferral rule as today (`desktop/src/features/agents/AGENTS.md` rule 8) — do not flash a control that will disappear | + +**Effect timing:** a model change lands on the agent's **next fresh ACP +session** (C-07, `HERMES.md:116-120`). `!rotate` forces one. The UI must say so +rather than implying an immediate switch. + +## New-profile flow (DoD-1, second half) + +Create-in-place (`HermesProfileCreateAffordance.tsx`) currently runs +`hermes profile create --no-alias` and binds. After a successful create, +offer model selection through the same P04 write path — one explicit step, still +auditable, still Crew-owned. Bundled skills stay (D-023); no `--no-skills`. + +## Files + +| Path | Owner | Change | +| ---- | ----- | ------ | +| `desktop/src/features/agents/ui/HermesProfileModelField.tsx` | **new, Crew-only** | editable control + note + error/read-back states | +| `desktop/src/features/agents/ui/EditAgentModelAndProfileSection.tsx` (127 lines, Crew-only, S-12) | Crew-only | branch at `:71` renders the new field instead of `ProfileOwnedModelRow` when `modelSource === "profileWriteThrough"` | +| `desktop/src/features/agents/ui/HermesProfileBindingFields.tsx` (335 lines, Crew-only, S-13) | Crew-only | keep `ProfileOwnedModelRow` at `:40` as the read-failure fallback; retire it as the default | +| `desktop/src/features/agents/lib/hermesProfileBinding.ts` | Crew-only | replace `profileOwnedModelLabel` (`:170`) copy with the write-through note builder | +| `desktop/src/features/agents/ui/createHermesBindingFields.tsx` | Crew-only | post-create model step | +| `desktop/src/features/agents/ui/AgentDefinitionDialog.tsx` (1016 lines, **upstream, over ratchet**) | upstream | **≤10 lines** — mount the Crew-owned child only | +| `desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx` (1224 lines, **upstream, over ratchet**) | upstream | **≤10 lines** — same | + +**File-size rule:** both dialogs already exceed `MAX_LINES = 1000` +(`desktop/scripts/check-file-sizes.mjs`). All markup goes into Crew-owned +children. Never raise the limit; never add an override (D-022). + +## Rules that still bind this phase + +- No `runtime.id === "hermes"` in render code — read the P03 descriptor + (`desktop/src/features/agents/AGENTS.md` rule 1). +- Crew stores nothing (red-team R-3). The query cache is invalidated on write and + on dialog open; it is never the source of truth. +- `BUZZ_ACP_MODEL` stays stripped at spawn for profile-locked runtimes + (C-05/C-06, issue non-goal). Editing a model in Crew must not reintroduce it. +- Text sizing: rem-based Tailwind tokens only, no arbitrary px/rem literals + (`pnpm check:px-text`). + +## Turns green + +E-09 (UI half), E-15; keeps E-13 green. + +## Verification + +```bash +just desktop-typecheck +cd desktop && pnpm check && pnpm check:px-text && pnpm check:file-sizes +cd desktop && pnpm test:e2e:smoke +``` diff --git a/plans/20260810-hermes-profile-editing/phase-07-soul-editor-ui.md b/plans/20260810-hermes-profile-editing/phase-07-soul-editor-ui.md new file mode 100644 index 00000000000..2bfa4de7c69 --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-07-soul-editor-ui.md @@ -0,0 +1,81 @@ +--- +phase: 07 +title: SOUL.md editor UI + persona at birth +status: planned +priority: high +effort: M +dependencies: [03, 05] +--- + +# Phase 07 — SOUL.md editor UI + persona at birth + +Issue #118 thing-to-solve 2, UI half. The founder edits who the agent *is* from +inside Crew, and a profile Crew creates is born with a real persona. + +## Behaviour + +| State | Render | +| ----- | ------ | +| Bound profile, `SOUL.md` readable | Editor **populated with the real current content**; save writes it back | +| Read fails | Named error + the existing repair path. **Never** an empty box presented as the persona | +| Save fails | Keep the founder's unsaved text in the editor, show the classified message, do not close | +| Reset available (P01/Q3 found a source) | "Reset to Hermes default" with a confirm step, because it overwrites founder prose | +| Reset unavailable | Affordance absent; recorded as a known gap in `HERMES.md` | +| `personaDoc === "none"` (Claude Code, Codex, unknown) | Nothing renders — no empty section, no disabled control (C-15) | + +**Effect timing:** like the model (C-07), a persona edit lands on the **next +fresh ACP session**, not the current turn. P01/Q4 confirms; the UI says so. This +is the residual risk recorded in `plan.md`. + +## Persona at birth (DoD-2, second half) + +Create-in-place currently runs `hermes profile create --no-alias` and +binds, leaving the generic default `SOUL.md`. Add a **persona step** to that +flow: after a successful create, the same editor opens on the newly created +file so the founder writes a real persona before the agent ever runs. Bundled +skills are kept (D-023). + +The step is skippable — skipping leaves the Hermes default, which is exactly +today's behaviour, so nothing regresses if the founder is in a hurry. + +## Files + +| Path | Owner | Change | +| ---- | ----- | ------ | +| `desktop/src/features/agents/ui/HermesSoulEditor.tsx` | **new, Crew-only** | populated textarea, dirty tracking, save/cancel, reset-with-confirm, effect-timing note | +| `desktop/src/features/agents/ui/HermesProfileCreateAffordance.tsx` | Crew-only | persona step after successful create | +| `desktop/src/features/agents/ui/createHermesBindingFields.tsx` | Crew-only | mount point in the create flow | +| `desktop/src/features/agents/ui/EditAgentModelAndProfileSection.tsx` | Crew-only | mount the editor when `personaDoc === "soulMd"` | +| `desktop/src/shared/api/hermesProfiles.ts` | Crew-only | consume the P05 wrappers | +| `desktop/src/features/agents/ui/AgentDefinitionDialog.tsx` / `AgentInstanceEditDialog.tsx` | **upstream, over ratchet** | **≤10 lines each**, mount only | + +## Rules + +- Capability-gated on `personaDoc`, never on `runtime.id` + (`desktop/src/features/agents/AGENTS.md` rule 1). +- Byte-exact round-trip; opening and closing without editing must leave the file + untouched (E-05). +- Layer-1 vs Layer-3 must be visually distinct: this editor is the **profile's** + persona (shared everywhere the profile runs); the "Agent instructions" box in + P08 is Crew's per-agent Layer-3 append. Labelling them the same way is the + confusion the issue is trying to end. +- rem-based text tokens only (`pnpm check:px-text`); new markup lives in + Crew-owned files, not the two over-ratchet dialogs. +- Playwright: `waitForAnimations(page)` before any screenshot; scope shots with + `locator.screenshot()` and verify hashes are distinct before posting. + +## Turns green + +E-16. + +## Verification + +```bash +just desktop-typecheck +cd desktop && pnpm check && pnpm check:px-text && pnpm check:file-sizes +cd desktop && pnpm test:e2e:smoke +``` + +Manual: open `scout`'s persona, confirm it matches disk, edit and save, confirm +the file changed; create a throwaway profile from Crew and confirm the persona +step writes a real `SOUL.md` before first run. diff --git a/plans/20260810-hermes-profile-editing/phase-08-description-optionality.md b/plans/20260810-hermes-profile-editing/phase-08-description-optionality.md new file mode 100644 index 00000000000..515b71a6f7e --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-08-description-optionality.md @@ -0,0 +1,82 @@ +--- +phase: 08 +title: Description optionality + layer copy +status: planned +priority: medium +effort: S +dependencies: [02] +--- + +# Phase 08 — Description optionality + layer copy + +Issue #118 thing-to-solve 3. The Crew agent description becomes explicitly +**optional**: empty means no Layer-3 injection at all. + +## Honest starting point — most of this already works + +| Fact | Evidence | +| ---- | -------- | +| An empty description already normalises to `None` | `desktop/src-tauri/src/managed_agents/types.rs:119` (S-5) | +| `None` already removes the env var rather than setting it empty | `desktop/src-tauri/src/managed_agents/runtime.rs:680-682` (S-4) | +| `None` already yields no system-prompt field in `session/new` | `crates/buzz-acp/src/pool.rs:255` (S-2) → `acp.rs:2355` (S-1) | +| The definition dialog does not require a description to submit | `AgentDefinitionDialog.tsx` submit gate | + +So this phase is **contract-proof + copy + docs**, not a new mechanism. Saying +so is the point: the plan does not invent work to fill a DoD box. What is +genuinely unverified is which `SystemPromptTransport` variant the Hermes adapter +path takes — P01/Q5 answers it and E-11 pins it. + +## The three layers (the model the UI must teach) + +| Layer | Owner | Source | Applies to | +| ----- | ----- | ------ | ---------- | +| **L1 `SOUL.md`** | the Hermes profile | `~/.hermes/profiles//SOUL.md` | every place the profile runs | +| **L2 base prompt** | the harness | `crates/buzz-acp/src/base_prompt.md` (`lib.rs:1942`, S-3) | every Buzz-managed ACP agent | +| **L3 Crew description** | Crew | `system_prompt` → `BUZZ_ACP_SYSTEM_PROMPT` → `session/new` | this Crew agent only, when non-empty | + +`layer3: "append"` is generic — it holds for Claude Code and Codex too. Only L1 +is Hermes-specific. + +## Work + +1. **Prove the contracts** (E-10, E-11, E-12) rather than assume them. If any + fails, that is a real bug this phase fixes at the seam that broke. +2. **Label the field optional** at `AgentDefinitionDialog.tsx:826-839` and the + instance-edit equivalent (`AgentInstanceEditDialog.tsx:715`). Today it reads + "Agent instructions" with placeholder "Describe what this agent should do." — + nothing tells the founder it is optional, or that leaving it empty is the + right choice for a Hermes agent whose persona lives in `SOUL.md`. +3. **Explain the layering in one line of helper copy** next to the field, and + point Hermes users at the `SOUL.md` editor from P07. + +## Files + +| Path | Owner | Change | +| ---- | ----- | ------ | +| `desktop/src/features/agents/ui/AgentDefinitionDialog.tsx` (upstream, over ratchet) | upstream | **≤6 lines** — optional label + helper mount | +| `desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx` (upstream, over ratchet) | upstream | **≤6 lines** — same | +| `desktop/src/features/agents/ui/AgentInstructionsHelper.tsx` | **new, Crew-only** | the layer explanation, capability-aware (mentions `SOUL.md` only when `personaDoc === "soulMd"`) | +| `crates/buzz-acp` tests | upstream crate, tests only | E-11 payload contract | +| `desktop/src-tauri/src/managed_agents` tests | upstream, tests only | E-10 env contract | + +## Non-goals held + +- No change to the `BUZZ_ACP_MODEL` strip-at-spawn guard. +- No change to the Layer-3 injection path for non-empty descriptions (E-12). +- No new event kind, no new HTTP endpoint — this rides existing ACP contracts. + +## Turns green + +E-10, E-11, E-12, E-17. + +## Verification + +```bash +cargo test -p buzz-acp system_prompt +cargo test --manifest-path desktop/src-tauri/Cargo.toml system_prompt +cd desktop && pnpm test:e2e:smoke +``` + +Manual (issue § Verification): create a Hermes agent with an **empty** +description, inspect the `session/new` payload, and confirm no system-prompt +field is present. diff --git a/plans/20260810-hermes-profile-editing/phase-09-docs-and-state.md b/plans/20260810-hermes-profile-editing/phase-09-docs-and-state.md new file mode 100644 index 00000000000..bd4a89f6e41 --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-09-docs-and-state.md @@ -0,0 +1,108 @@ +--- +phase: 09 +title: Docs truth + STATE anti-drift +status: planned +priority: high +effort: M +dependencies: [06, 07, 08] +--- + +# Phase 09 — Docs truth + STATE anti-drift + +Issue #118 thing-to-solve 4 and DoD-4. Three shipped documents currently tell an +implementer the **opposite** of what this work does. Leaving any of them stale +would make the repo lie. + +## What must change + +### 1. New decision — D-028 (next free id; D-027 is the current highest) + +Records that Crew becomes a **remote control for the profile**, not a second +store: + +- Founder direction: *"Crew does not own the person, but Crew is where you edit + the person."* +- **Superseded:** the presentation half of D-019 item 2 — Crew may now offer an + editable model control for a profile-bound runtime. +- **Preserved:** the profile stays the single source of truth; Crew persists no + model and no persona; every write goes through Hermes' own CLI/file; + `BUZZ_ACP_MODEL` stays stripped at spawn. +- Capability-descriptor rule (D-025 continuity): behaviour is declared per + runtime, never branched on a harness id. +- If Option B was taken in P03, record the upstream Rust edit and its approval. + +### 2. `docs/crew/HERMES.md` + +| Location | Change | +| -------- | ------ | +| Rule 2 (`:28-33`) | "never in Crew" → model/provider are profile-owned **and editable from Crew via write-through**; keep the `BUZZ_ACP_MODEL` strip sentence | +| Hiring § step 2 (`:79-88`) | "Leave model blank — the UI replaces the model control with 'decided by profile scout'" is now wrong; describe picking a model in Crew and the persona step | +| Hiring § step 1 | Create-in-place now includes the persona step | +| Daily operations (`:116-124`) | Add "edit the persona" alongside "change the model"; keep the C-07 next-fresh-session semantics and `!rotate` | +| Known gaps (`:197-208`) | Mark model display/edit and persona editing done; add the reset-to-default gap if P01/Q3 came back empty | +| Security caveats | Unchanged (credential fallback, local custody, shared profile state) | + +### 3. `docs/crew/features/0001-hermes-first-class-runtime.md` — Slice 2 reconciliation + +C-04 currently reads: *"No model field | Render | Read-only 'provided by +profile'; no picker | **Forbidden:** editable model/provider control."* That +forbidden column is now the requirement. C-04 must be **superseded with a +pointer to D-028**, not silently rewritten and not merely ticked as shipped. + +Re-check the rest of the C-03…C-12 list against what is actually on main and +mark each shipped / superseded / still open. C-05, C-06, C-10, C-12, C-13, C-14, +C-15 are unchanged by this work and must stay green. + +### 4. `docs/crew/STATE.md` — anti-drift (issue #117) + +STATE.md is **currently stale**: its Hermes track still lists "Next gates: Slice +2 (binding/readiness/no-model UI + RED contracts), Slice 3 (upstream tier-1 PR +to block/buzz), Slice 4 (profile lifecycle UI)" — but Slices 2 and 3 shipped, +and D-020 cancelled the upstream PR entirely. Fix that alongside recording this +work. Per issue #117, any PR changing shipped state updates STATE.md **in the +same PR**. + +### 5. `desktop/src/features/agents/AGENTS.md` (upstream file) + +Rule 3 says a `profileArg + providerLocked + no modelEnvVar` runtime is +`ownedByProfile` and surfaces "**never an editable model control**". Rule 8 says +the model control is omitted before discovery runs. Both need updating to the +descriptor model. + +**Upstream-edit justification:** this file's own closing line requires updating +it in the same PR that changes how agent configuration is modelled, rendered, +persisted, applied, or cleared. Expected delta **~15 lines**, confined to rules +3 and 8 plus the enforcing-tests list. No restructuring, no renaming. + +## Docs NOT to change + +- Root `AGENTS.md`, `CONTRIBUTING.md`, `ARCHITECTURE.md` — no user-facing + workflow, command, or architecture boundary changes. +- `docs/crew/FOUNDER-PRODUCT.md` — rule 4 already requires labelling what is + Hermes-only; this work complies rather than amends. +- `docs/crew/UPSTREAM-SYNC.md`, `IDENTITY.md` — unchanged. + +## Rules + +- Read each document before editing it; verify every claim against source, tests, + or live state afterwards (`documentation-management` rule). +- No plan-phase numbers, finding codes, or audit labels in code comments, + filenames, or commit messages — plan references belong in these phase files and + the PR description. +- Branch name describes the product area (`agents/hermes-profile-editing`), not a + phase number. + +## Turns green + +DoD-4 in full. + +## Verification + +```bash +# links + lint +cd desktop && pnpm check +just ci +``` + +Manual: re-read `HERMES.md` end to end as a new contributor and confirm no +sentence still says the model cannot be set from Crew. diff --git a/plans/20260810-hermes-profile-editing/phase-10-verification-evidence.md b/plans/20260810-hermes-profile-editing/phase-10-verification-evidence.md new file mode 100644 index 00000000000..c93580b9401 --- /dev/null +++ b/plans/20260810-hermes-profile-editing/phase-10-verification-evidence.md @@ -0,0 +1,99 @@ +--- +phase: 10 +title: Verification & evidence +status: planned +priority: high +effort: S +dependencies: [09] +--- + +# Phase 10 — Verification & evidence + +Issue #118 § Verification and DoD-5. Turns the work into evidence a reviewer can +check without re-running it. + +## Automated gate + +```bash +. ./bin/activate-hermit + +just ci # fmt + clippy + desktop lint + unit tests + builds + +# desktop crate is excluded from the root workspace — run it explicitly +cargo test --manifest-path desktop/src-tauri/Cargo.toml +cargo test -p buzz-acp system_prompt + +cd desktop +pnpm check && pnpm check:px-text && pnpm check:file-sizes +pnpm test:e2e:smoke # builds with the mock bridge; never `pnpm run build` +``` + +`just test` (integration, needs Postgres + Redis) is **not** required — no +relay, db, or auth crate is touched. + +Every one of E-01…E-17 must be green, and C-05, C-06, C-07, C-10, C-12, C-13, +C-14, C-15 must still be green. + +## Live probe (real profile, this machine) + +The issue requires a live probe, not just mocks. + +1. Read the model for a bound profile in Crew; cross-check with the Hermes CLI. +2. Change it from Crew; confirm the profile reflects it and Crew stores nothing. +3. Send `!rotate`, run a turn, confirm the new model is in effect (C-07). +4. Open `SOUL.md` in Crew, confirm it matches disk, edit, save, confirm the file + changed and nothing else in the profile directory did. +5. Create an agent with an **empty** description; inspect the `session/new` + payload and confirm no system-prompt field is present. +6. Attempt an invalid model id; confirm the classified error and that the agent + still runs on its previous model (red-team R-4). +7. Confirm a non-Hermes runtime (Claude Code or Codex) renders exactly as before + (C-15). + +**Probe hygiene:** prefer a throwaway profile under a temporary `HERMES_HOME` +for destructive steps. Restore any value changed on a real profile. Never print +credential values or non-model config keys. + +## Playwright evidence + +Three states from the issue: + +| Shot | State | +| ---- | ----- | +| 1 | Hermes agent in edit — editable model control + the shared-everywhere note | +| 2 | `SOUL.md` editor open, populated with real current content | +| 3 | Create/edit with an empty "Agent instructions" box, labelled optional, save enabled | + +Rules that make this evidence trustworthy: + +- Build with `pnpm build:e2e` / `pnpm test:e2e:smoke`. A plain `pnpm run build` + strips the mock bridge and every spec fails looking like a product bug. +- Kill port 4173 before re-running after code changes (`reuseExistingServer` + serves stale code). +- `waitForAnimations(page)` before every capture. +- Scope each shot with `locator.screenshot()` and **verify hashes are distinct** + before posting — identical hashes mean two shots captured the same state: + ```bash + shasum -a 256 test-results/screenshots/*.png # every hash must be unique + ``` +- Post with `scripts/post-screenshots.sh [body.md]`. Never + `buzz upload`, never a relay media URL (they die behind GitHub's camo proxy). + Delete superseded screenshot comments after a repost. +- No `SOUL.md` content in a posted screenshot unless the founder approves that + specific text. + +## PR requirements + +- Target **`Nuncio-hq/crew`** only. Never `block/buzz` (D-020). +- Every commit signed off (`git commit -s`) — the DCO check fails otherwise. +- PR body lists each upstream-file edit with its justification and actual line + count, and states whether Option A or Option B was used for the capability + descriptor. +- `docs/crew/STATE.md` updated in this same PR (issue #117 anti-drift). +- RED-run output from P02 and the live-probe transcript attached. +- Merge through `NuncioCrew Gate`. + +## Exit criteria + +All five DoD checkboxes demonstrable from artifacts attached to the PR, with no +reviewer needing to re-run the live probe to believe it. diff --git a/plans/20260810-hermes-profile-editing/plan.md b/plans/20260810-hermes-profile-editing/plan.md new file mode 100644 index 00000000000..7164f2b47da --- /dev/null +++ b/plans/20260810-hermes-profile-editing/plan.md @@ -0,0 +1,308 @@ +# Hermes profile editing from Crew (model write-through, SOUL.md, optional description) + +- **Status:** Planned — not implemented, not approved for cook +- **Date:** 2026-08-10 +- **Issue:** [#118](https://github.com/Nuncio-hq/crew/issues/118) +- **Parent feature:** [`docs/crew/features/0001-hermes-first-class-runtime.md`](../../docs/crew/features/0001-hermes-first-class-runtime.md) +- **Related plans:** [`../20260805-1330-hermes-first-class-runtime/plan.md`](../20260805-1330-hermes-first-class-runtime/plan.md), + [`../20260806-1225-hermes-profile-picker/plan.md`](../20260806-1225-hermes-profile-picker/plan.md) +- **Decisions touched:** D-019 (item 2 partially superseded), D-020, D-023, + D-024, D-025 — **one new decision expected (D-028)** +- **Branch (suggested):** `agents/hermes-profile-editing` (area name, not a + phase number — `UPSTREAM-SYNC.md` branch rule) +- **Target repo:** `Nuncio-hq/crew` only. Never `block/buzz` (D-020). + +> This plan is **planning-only output**. No production code, no branch push, +> no PR, no issue comment was produced by the planning run. + +## Manager-visible outcome + +Oscar opens a Hermes agent in Crew and can **see and change the model**, and +**read and edit the profile's `SOUL.md`** — the words that decide who the +agent is — without leaving the app for a terminal. Crew tells him plainly +that both belong to the profile and therefore change everywhere that profile +runs. A profile he creates from Crew is born with a real persona instead of +the generic default. The Crew "Agent instructions" box becomes genuinely +optional: leave it empty and nothing extra is injected. + +Founder direction locked in the issue: **"Crew does not own the person, but +Crew is where you edit the person."** + +## What changes about a previously recorded rule + +D-019 item 2 and [`docs/crew/HERMES.md`](../../docs/crew/HERMES.md) rule 2 say +Crew shows `"Model: decided by profile "` and **never** offers an +editable model control. `desktop/src/features/agents/AGENTS.md` rule 3 says the +same in implementer language. Issue #118 supersedes **only the presentation +half** of that rule. + +| Still true (do not reopen) | Superseded by #118 | +| -------------------------- | ------------------ | +| The profile is the single source of truth for model/provider | Crew may only *display* the model | +| Crew stores no model for a profile-bound agent | The UI must not offer a model control | +| `BUZZ_ACP_MODEL` stays stripped at spawn for profile-locked runtimes | — | +| Model change takes effect on next fresh ACP session (C-07) | — | + +Crew becomes a **remote control for the profile**, not a second store. Every +write goes through Hermes' own CLI; Crew persists nothing. + +## Named Buzz seams (every feature hangs off one) + +| # | Seam | `path:line` | Used by | +| - | ---- | ----------- | ------- | +| S-1 | `SystemPromptTransport` enum (Layer-3 delivery shape) | `crates/buzz-acp/src/acp.rs:2355` | P01, P08 | +| S-2 | `session_new_system_prompt()` — picks `Field` / `ClaudeMeta` / `None` | `crates/buzz-acp/src/pool.rs:255` | P01, P08 | +| S-3 | Layer-2 harness prompt source (`base_prompt.md`) | `crates/buzz-acp/src/lib.rs:1942` | P08, P09 | +| S-4 | Spawn env: sets or removes `BUZZ_ACP_SYSTEM_PROMPT` | `desktop/src-tauri/src/managed_agents/runtime.rs:680` | P08 | +| S-5 | Empty description already normalises to `None` | `desktop/src-tauri/src/managed_agents/types.rs:119` | P02, P08 | +| S-6 | Crew-owned IPC catalog entry (capability projection) | `desktop/src-tauri/src/managed_agents/types/crew.rs:7` | P03 | +| S-7 | Rust catalog facts (`profile_arg`, `provider_locked`, `model_env_var`) | `desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs:41,43,70` | P03 | +| S-8 | Crew-owned raw→TS catalog projection | `desktop/src/shared/api/fromRawAcpRuntimeCatalog.ts:71` | P03 | +| S-9 | Hermes CLI invocation + result-enum precedent | `desktop/src-tauri/src/managed_agents/hermes_profile_lifecycle.rs:153,268,362` | P04, P05 | +| S-10 | Crew-owned Hermes IPC commands module | `desktop/src-tauri/src/commands/hermes_profiles.rs:11` | P04, P05 | +| S-11 | Field model: `ownedByProfile` model omission | `desktop/src/features/agents/lib/agentConfigCore.ts:217` | P03, P06 | +| S-12 | Model row vs editable picker branch | `desktop/src/features/agents/ui/EditAgentModelAndProfileSection.tsx:71` | P06 | +| S-13 | `ProfileOwnedModelRow` (copy #118 replaces) | `desktop/src/features/agents/ui/HermesProfileBindingFields.tsx:40` | P06 | +| S-14 | Crew-owned TS IPC wrappers for Hermes profiles | `desktop/src/shared/api/hermesProfiles.ts:41` | P04, P05 | +| S-15 | "Agent instructions" textarea (Layer-3 author surface) | `desktop/src/features/agents/ui/AgentDefinitionDialog.tsx:837` | P07, P08 | + +## Architecture decision — capability descriptor without an upstream Rust edit + +The issue requires a per-runtime capability descriptor so render code never +branches on `runtime.id === "hermes"` (D-025; `desktop/src/features/agents/AGENTS.md` +rule 1). Two ways to get one: + +| Option | Shape | Upstream cost | +| ------ | ----- | ------------- | +| **A (chosen)** | Derive `{ modelSource, personaDoc, layer3 }` in a **Crew-only** module from catalog facts already projected (`profileArg`, `providerLocked`, `modelEnvVar`), computed once at the `fromRawAcpRuntimeCatalog` boundary (S-8) | **zero** | +| B (escalation only) | Declare the descriptor on `KnownAcpRuntime` (S-7) and project it through `discovery.rs:1273` | 2 upstream Rust files, ~10 lines, requires the failed-non-Rust-spike + approval rule in `UPSTREAM-SYNC.md` | + +**Option A is the default.** `UPSTREAM-SYNC.md` states a Rust edit to an +upstream file "requires a failed or insufficient non-Rust spike plus explicit +approval" — so B may only be taken if P01 proves A insufficient, and then only +with founder approval recorded in the plan and in `DECISIONS.md`. + +Under A the capability facts still come from the Rust catalog; Crew only +*projects* them into a named descriptor at one boundary. Render code reads the +descriptor. The one fact A cannot read from the catalog is the persona +filename (`SOUL.md`); it is named once, as data, in the Crew-only capability +table — never as a branch inside a component. + +Descriptors (issue §Design constraint): + +```text +Hermes : { modelSource: "profileWriteThrough", personaDoc: "soulMd", layer3: "append" } +Claude Code / Codex : { modelSource: "adapterSetting", personaDoc: "none", layer3: "append" } +``` + +## Honest scoping note (do not inflate this work) + +**Layer-3 optionality is already mostly true in code.** `types.rs:119` (S-5) +already maps an empty description to `None`, and `AgentDefinitionDialog`'s +submit gate does not require a description. Phase 08 is therefore +**contract-proof + copy + docs**, not a new mechanism. The plan says so rather +than inventing work to fill a DoD box. What is genuinely unproven is *which* +`SystemPromptTransport` variant (S-1/S-2) the Hermes adapter path takes and +whether `None` really produces a payload with no system-prompt field — that is +a P01 spike item and a P02 RED test. + +## Scope + +| # | Phase | Ships | Depends | +| - | ----- | ----- | ------- | +| 01 | [Spike — Hermes config + SOUL read/write feasibility](phase-01-spike-hermes-config-soul.md) | Evidence note; go/no-go per mechanism | — | +| 02 | [RED contract tests](phase-02-red-contract-tests.md) | Failing Rust + TS + Playwright contracts | 01 | +| 03 | [Runtime capability descriptor](phase-03-runtime-capability-descriptor.md) | `{modelSource, personaDoc, layer3}` at the catalog boundary | 01, 02 | +| 04 | [Profile model read/write IPC](phase-04-profile-model-ipc.md) | `read/write_hermes_profile_model` over the Hermes CLI | 01, 02 | +| 05 | [SOUL.md read/write/reset IPC](phase-05-profile-soul-ipc.md) | `read/write/reset_hermes_profile_soul` | 01, 02 | +| 06 | [Model write-through UI](phase-06-model-write-through-ui.md) | Editable model + shared-everywhere note (edit + create) | 03, 04 | +| 07 | [SOUL.md editor UI + persona at birth](phase-07-soul-editor-ui.md) | Edit-in-place editor; persona step on create-in-place | 03, 05 | +| 08 | [Description optionality + layer copy](phase-08-description-optionality.md) | Empty = no Layer-3; layer semantics in the UI | 02 | +| 09 | [Docs truth + STATE anti-drift](phase-09-docs-and-state.md) | D-028, HERMES.md, feature 0001, STATE.md, agents AGENTS.md | 06, 07, 08 | +| 10 | [Verification & evidence](phase-10-verification-evidence.md) | Live probe, Playwright shots, `just ci` | 09 | + +## Definition of Done → phase map (issue #118) + +| DoD checkbox (issue) | Phases | +| -------------------- | ------ | +| 1. Model visible **and** editable from Crew for a Hermes agent, with the shared-everywhere note; new-profile flow can set a model | 01, 02, 03, 04, 06 | +| 2. `SOUL.md` readable/editable from Crew; Crew-created profiles get a real persona at birth (not the generic default) | 01, 02, 05, 07 | +| 3. Crew description optional; empty = no Layer-3 injection; layer semantics documented | 02, 08, 09 | +| 4. Decision recorded; `HERMES.md` hiring flow updated; feature 0001 Slice 2 (C-03…C-12) reconciled; `STATE.md` updated in the same PR | 09 | +| 5. Contract tests + live probe + Playwright evidence attached to the PR | 02, 10 | + +No DoD checkbox is unmapped. No phase exists without a DoD or issue-plan line +behind it. + +## Non-goals (from the issue — do not drift) + +- No Crew editor for profile **memory**, **skills**, or **credentials**. +- No change to the `BUZZ_ACP_MODEL` strip-at-spawn guard (C-05/C-06 stay green). +- No new engine implementations. Hermes is the only adapter; the UI seam must + still be capability-shaped so a future engine can declare its own descriptor. +- Not a rewrite of shipped binding / readiness / lifecycle UI. +- No auth badge (still blocked on the Hermes-side probe, spike 0010). +- Crew never stores model, provider, or persona text for a profile-bound agent. + +## Thin-fork budget (upstream files) + +| File | Owner | Expected delta | Justification | +| ---- | ----- | -------------- | ------------- | +| `desktop/src-tauri/src/lib.rs` | upstream | **+4 lines** — register new Hermes commands in `invoke_handler` next to `lib.rs:795-797` | Tauri has no out-of-tree command registration; identical to the already-accepted `list/create/delete_hermes_profile` delta | +| `desktop/src/features/agents/AGENTS.md` | upstream | **~15 lines** — rule 3 and rule 8 currently forbid an editable model control | Its own closing rule requires updating it in the same PR that changes how config is modelled/rendered; leaving it stale would make the file lie | +| `desktop/src/features/agents/ui/AgentDefinitionDialog.tsx` | upstream | **≤10 lines** — mount Crew-owned child components + optional-label copy | Already **1016 lines**, over the 1000-line ratchet: all new markup goes into Crew-owned children (D-022 direction), never into this file | +| `desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx` | upstream | **≤10 lines** — same | Already **1224 lines**; same rule | +| `desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs` | upstream | **0 (target)** / ~8 if Option B is approved | Only if P01 proves the Crew-side derivation insufficient | +| `desktop/src-tauri/src/managed_agents/discovery.rs` | upstream | **0 (target)** / ~2 if Option B is approved | Same | + +Everything else is **additive Crew-owned files**: `commands/hermes_profiles.rs`, +`managed_agents/hermes_profile_config.rs` (new), `managed_agents/hermes_profile_soul.rs` +(new), `shared/api/hermesProfiles.ts`, `features/agents/lib/runtimeCapabilities.ts` +(new), `features/agents/ui/HermesProfileModelField.tsx` (new), +`features/agents/ui/HermesSoulEditor.tsx` (new). + +**File-size guard:** `desktop/scripts/check-file-sizes.mjs` enforces +`MAX_LINES = 1000`. Both agent dialogs already exceed it. Never raise the limit +and never add an override — extract into Crew-owned children (D-022). + +## Generic-ACP check (D-025) + +| Mechanism | Generic across engines? | Hermes-only part | +| --------- | ----------------------- | ---------------- | +| Capability descriptor on the runtime catalog | **Yes** — every runtime gets one | Hermes' values | +| Layer-3 description → `SystemPromptTransport` (S-1/S-2) | **Yes** — unchanged Buzz contract for all engines | none | +| Empty description → no injection (S-5) | **Yes** | none | +| Model write-through via profile CLI | **No** | Labelled `modelSource: profileWriteThrough`; other engines keep `adapterSetting` | +| `SOUL.md` persona editor | **No** | Labelled `personaDoc: soulMd`; others render nothing | + +Per `FOUNDER-PRODUCT.md` rule 4, UI copy and docs must say which parts are +Hermes-only. Non-Hermes runtimes must render **exactly** what they render today +(C-15 stays green). + +## Security and privacy notes + +- **Never read or render `auth.json`.** Model write-through touches + `config.yaml` keys `model.provider` / `model.default` only, and only through + `hermes -p config set …` — Crew must not parse or rewrite the profile + YAML directly. +- **Never log or echo profile config values** that are not the model id/provider. + During planning, only key *names* were inspected on the live machine; the same + discipline applies to implementation and to any PR evidence. +- `SOUL.md` is founder-authored prose and may contain sensitive business + context. It stays local; it is never sent to a relay, never put in an issue, + never in a screenshot posted to a PR unless the founder approves that specific + text. +- D-024 still holds: profile-bound Hermes agents are owner-only and local. + Nothing in this plan opens a remote or allowlisted path to profile editing. +- The credential-fallback caveat (`HERMES.md` § Security caveats, spike 0010) is + unchanged: a fresh profile can spend the manager's provider credit. Changing a + model from Crew must not imply Crew is provisioning credentials. + +## Backward compatibility / migration + +None required. No stored schema changes: Crew persists no model and no persona +for profile-bound agents. Existing bindings, records, and legacy tier-3 custom +harness JSONs behave exactly as today. Agents whose Crew description is +non-empty keep injecting Layer 3 exactly as today. + +## Rollback / abort criteria + +- Revert the PR. The UI returns to `ProfileOwnedModelRow` (S-13), the new IPC + commands disappear, and no profile is left in a half-written state — every + write is a single idempotent Hermes CLI call or a single file write. +- **Abort P04 (model write-through)** if P01 finds no non-interactive + `hermes -p config set` path with a distinguishable failure exit — fall + back to shipping P05/P07/P08 and keeping the read-only model row, and record + the Hermes-side ask. Do not scrape or hand-edit `config.yaml` as a substitute. +- **Abort the "Reset to Hermes default" button only** (not the whole editor) if + P01 finds no trustworthy source for the default `SOUL.md`. Editing in place + still ships; the reset affordance is dropped and recorded as a known gap. +- If any of C-05, C-06, C-10, C-13, C-14, C-15 regress, stop and revert — those + are shipped guarantees, not negotiable. + +## Testing and validation plan + +| Level | What | Where | +| ----- | ---- | ----- | +| Rust unit | model read/write result enum, error classification, name validation reuse | `hermes_profile_config.rs`, `hermes_profile_soul.rs` (colocated `#[cfg(test)]`) | +| Rust unit | SOUL round-trip, missing-profile, unreadable-file, reset source | same | +| Rust contract | empty description → `session/new` payload with no system-prompt field (S-1/S-2/S-5) | `crates/buzz-acp` test module | +| TS unit | capability descriptor derivation for hermes / claude-code / codex / unknown runtime | `runtimeCapabilities.test` | +| TS unit | field-model change: `ownedByProfile` → editable-with-write-through (S-11) | `agentConfigCore` tests | +| Playwright (mock bridge) | three UI states from the issue: profile model editable + note, SOUL editor with real content, empty description accepted | `desktop/tests/e2e/hermes-profile-binding.spec.ts` (+ new spec if it outgrows the file) | +| Live probe | real profile on this machine: change model from Crew, verify with `hermes -p config get`, run a turn | P10 | +| Gate | `just ci`; `just desktop-tauri-test`; `pnpm check:file-sizes`; `pnpm check:px-text` | P10 | + +**Build rule for e2e:** always `pnpm test:e2e:smoke` / `pnpm build:e2e` — a plain +`pnpm run build` strips the mock bridge and every mock-mode spec fails in a way +that looks like a product bug. + +## Unresolved questions (for the founder — none block starting P01) + +1. **Reset-to-default source of truth for `SOUL.md`.** Three candidates: create + a throwaway profile and copy its `SOUL.md` (side-effecty), bundle a copy in + Crew (drifts from Hermes), or read a template from the Hermes install (best + if one exists). P01 must answer; if none is trustworthy, ship the editor + without the reset button. +2. **Model input shape.** Free-text model id with validation, or a list? + Recommendation: **free-text + validation**, optionally seeded from the + profile's own `provider_models_cache.json` / `models_dev_cache.json` if P01 + shows they are reliable. `get_agent_models` (`commands/agent_models.rs:36`) + is keyed on adapter env/provider config, not on a Hermes profile, so reusing + it would be a false promise. +3. **Does a `SOUL.md` edit apply without respawn?** C-07 says a model change is + picked up on the next fresh ACP session (`!rotate` forces one). P01 must + confirm the same is true for `SOUL.md`, because the UI copy depends on the + answer. +4. **Descriptor placement.** Option A (Crew-only derivation, zero upstream + edits) is the plan's default. Approving Option B is a founder call and needs + the `UPSTREAM-SYNC.md` Rust-edit approval. + +## Validation pass + +Run against `/ak:plan validate` criteria on 2026-08-10. **Result: pass.** + +| Check | Outcome | +| ----- | ------- | +| Objective, scope, non-goals present | Pass — non-goals copied from the issue verbatim in intent | +| Every DoD checkbox mapped to ≥1 phase | Pass — 5/5 mapped, table above | +| Phases ordered by real dependency | Pass — Spike → RED → mechanism → UI → docs → evidence | +| Every feature names a Buzz seam with `path:line` | Pass — 15 seams, all line numbers verified against the working tree | +| Upstream-file edits justified with expected diff size | Pass — 6 rows, 4 targeted at zero | +| Workflow gate order (D-008: spike → RED → implement) | Pass — P01, P02 precede all implementation phases | +| Testing/validation plan present | Pass | +| Security/privacy notes present | Pass — credential and `SOUL.md` handling called out | +| Rollback/abort criteria present | Pass — per-phase aborts, not just "revert" | +| Migration/back-compat addressed | Pass — none required, stated with reason | +| No implementation performed during planning | Pass — files on disk are plan documents only | + +**Revision made during validation:** the first draft mapped DoD-3 to a new +mechanism phase. Verifying `types.rs:119` showed empty descriptions already +normalise to `None`, so P08 was rewritten as contract-proof + copy + docs. The +plan now states this explicitly rather than claiming credit for shipped +behaviour. + +## Red-team pass + +Six findings, **five applied**, one recorded-not-applied. + +| # | Finding | Disposition | +| - | ------- | ----------- | +| R-1 | The plan silently reverses D-019 item 2, `HERMES.md` rule 2, and `AGENTS.md` rule 3 — three places tell an implementer the opposite | **Applied.** Added the "What changes about a previously recorded rule" table showing what is superseded vs still true, and made P09 update all three in the same PR. Nothing is reversed silently. | +| R-2 | "Capability descriptor" is a euphemism for adding a Hermes branch one layer down; `SOUL.md` cannot be derived from `profileArg` alone | **Applied.** Named the limit honestly: the persona filename is data in one Crew-owned table, never a render branch, and Option B is the recorded escalation if that is not good enough. | +| R-3 | Write-through invites Crew becoming a second store — a cached model value would silently diverge from the profile | **Applied.** Added the invariant "Crew persists nothing" to the outcome, the D-019 table, back-compat, and P04/P06 (read-after-write from the profile, no local cache as source of truth). | +| R-4 | The model editor could brick a working agent (bad model id ⇒ `model: String should have at least 1 character` class failure) with no undo | **Applied.** P04 must classify write failures and P06 must show the previous value and keep it recoverable; the abort criterion for P04 is explicit. | +| R-5 | "Reset to Hermes default" has no defined source; a bundled copy would drift and could overwrite founder-authored prose | **Applied.** Promoted to unresolved question 1 with a per-affordance abort criterion; the editor ships without reset rather than guessing. Never a blank-replace box (issue rule). | +| R-6 | Ten phases is heavy for one issue; P03/P04/P05 could be one Rust phase | **Not applied.** Recorded rationale: they carry different risk (pure projection vs CLI write-through vs file IO with an unresolved default source) and different abort criteria. Merging them would hide R-4 and R-5 behind one green checkbox. Splitting keeps each abort independently exercisable. | + +**Residual risk accepted:** if the founder edits `SOUL.md` in Crew while a +Hermes session is live, the change lands on the next fresh session, not the +current turn (same semantics as C-07 for model). P07 must say so in the UI. This +is a property of the profile lifecycle, not a defect introduced here. + +## Approval checkpoint + +No production implementation until this plan is approved. On approval the order +is fixed: **P01 spike → P02 RED → P03…P08 → P09 docs → P10 evidence**, one PR +series into `Nuncio-hq/crew` `main` through `NuncioCrew Gate`, commits signed +off (`git commit -s`, DCO). diff --git a/plans/20260810-profile-lifecycle-hardening/ISSUE-COMMENT.md b/plans/20260810-profile-lifecycle-hardening/ISSUE-COMMENT.md new file mode 100644 index 00000000000..38efa32ce59 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/ISSUE-COMMENT.md @@ -0,0 +1,70 @@ +# Plan ready — 7 phases + +Plan: `plans/20260810-profile-lifecycle-hardening/plan.md`. +Validate **pass**; red-team **9 findings, 8 applied, 1 rejected with rationale**. +All 6 DoD checkboxes map to ≥1 phase (mapping table in the plan). + +| # | Phase | Effort | Deps | +| - | ----- | ------ | ---- | +| 01 | Spike 0015 — readiness signals and archive mechanics | S (0.5-1 d) | — | +| 02 | Readiness model, evaluator, and projection | M (2-3 d) | 01 | +| 03 | Spawn preflight and attention routing | M (2 d) | 02 | +| 04 | Archive, restore, permanent delete, running-agent guard | L (3-4 d) | 01 | +| 05 | Readiness surfacing — card and dialog | M (2 d) | 02 | +| 06 | Offboarding archive, restore, permanent-delete UI | L (3 d) | 04, 05 | +| 07 | Verification, fault injection, and docs | M (2 d) | 03, 06 | + +02→05 ships the readiness half, 04→06 the archive half — independently +shippable, each leaving `main` green. + +## Key design decisions + +- **`auth-unknown` is not a `Requirement`.** A `Requirement` implies `NotReady`, + which makes `runtime.rs` spawn into setup-listener mode. With no headless auth + probe (spike 0010), *every* healthy profile is `auth-unknown` — modelling it as + blocking would take the runtime down. Split: `missing`/`broken-config`/ + `binary-missing` block; `ready`/`auth-unknown` are advisory. All five names + still reach the UI. +- **Carrier correction.** The issue names `AcpRuntimeCatalogEntry` / + `agentConfigCore.ts`; per `features/agents/AGENTS.md` rule 1 those hold + harness-scoped capability facts and field descriptors. Per-agent, time-varying + readiness rides the per-agent `Requirement` pipeline + runtime status + projection instead. Intent preserved (no frontend rival table, no `runtime.id` + checks, named reasons from Rust); mechanism corrected. +- **The graceful stop machinery the issue cites does not exist.** + `runtime/stop.rs:40,120,153` uses `Child::kill()` (immediate SIGKILL); the only + `SIGTERM` in `managed_agents` is `discovery.rs:927`, in the auth-probe timeout. + The guard is **refuse-while-running**, authoritative in Rust — not "archive + stops the agent for you". +- **The "existing backups area" does not exist either** — no `NuncioCrew Backups` + anywhere in the repo. Location/format is spike 0015's first output; no phase + hardcodes a path before then. +- **RED is a gate inside each phase**, not its own PR (a tests-only PR can't merge + green). **STATE.md is updated by every shipping phase** (#117), not just 07. +- **Thin fork:** 12 upstream files, ≈ **+153 / -24 lines**, every edit a hook or a + type mirror; substantive logic in Crew-owned files. Per-file justification in + the plan. **D-025:** transport, preflight, attention routing and the card stay + generic; YAML parse, `hermes --version`, profile-dir layout and archive/restore + are labelled Hermes-specific and confined to Crew files. + +## Named Buzz seams + +`readiness.rs:284/:333/:340` · `readiness/hermes.rs` · `runtime.rs:~575-630` +(`BUZZ_ACP_SETUP_PAYLOAD` setup-mode divert) · `configNudge.ts:69` + +`config-nudge-attachment.tsx` · `runtime_types.rs:90` → `types.ts:296` → +`managedAgentRuntimeStatus.ts:12` (the boolean being replaced) · +`AgentStatusBadge.tsx` / `ManagedAgentRow.tsx` · `agentAttention.ts` / +`needsYouStore.ts` (46010/46040) · `hermes_profile_lifecycle.rs` +(`HermesProfileLifecycleResult`) · `commands/hermes_profiles.rs` + +`lib.rs:795-797` · `hermes_profile.rs:13,19` (`default` reject) · +`usePersonaActions.ts:297-317` (the irreversible delete) · +`agent_discovery.rs:295-330,:425-455` (post-install re-evaluation model). + +## Open questions (phase 01 resolves 1-4) + +1. Archive location — none exists; app-data-dir proposed. +2. Archive format — `tar.gz` (new crate) vs plain copy (no dependency). +3. Which `config.yaml` conditions honestly mean `broken-config`; whether an + invalid model id is locally detectable at all. +4. Re-evaluation trigger — turn boundary (default), fs watch, or timer. +5. DECISIONS.md entry for archive semantics — recommended yes (D-028, phase 07). diff --git a/plans/20260810-profile-lifecycle-hardening/phase-01-spike-readiness-and-archive.md b/plans/20260810-profile-lifecycle-hardening/phase-01-spike-readiness-and-archive.md new file mode 100644 index 00000000000..edcd3a2b63d --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-01-spike-readiness-and-archive.md @@ -0,0 +1,148 @@ +--- +phase: 01 +title: Spike 0015 — readiness signals and archive mechanics +status: planned +priority: P0 +effort: S (0.5-1 d) +dependencies: [] +--- + +# Phase 01 — Spike 0015: readiness signals and archive mechanics + +## Why this phase exists + +D-008 requires a spike before implementation when the mechanism is +unknown. Four things are genuinely unknown and every one of them would +otherwise be guessed inside an implementation PR: + +1. Which `config.yaml` conditions honestly mean `broken-config` (OQ-3). +2. Where archives live — the "existing backups area" the issue cites + **does not exist** in this repo (OQ-1, plan DD-5). +3. What an archive costs and in what format (OQ-2). +4. What can cheaply trigger readiness re-evaluation after a healthy + spawn (OQ-4). + +This phase produces evidence and a decision record. **No production +code.** + +## Deliverable + +`docs/crew/spikes/0015-profile-readiness-and-archive.md` — next free +spike number (highest existing is `0014-agent-attention-recovery.md`). + +## Investigation items + +### I1 — `config.yaml` shape and failure modes (→ OQ-3) + +- Read a healthy Hermes v0.20.0 profile under + `$HERMES_HOME/profiles/` (or `~/.hermes/profiles/`). +- Enumerate the exact keys Hermes requires to start a session with a + model (`model.provider`, `model.default`, others). +- Fault-inject on a scratch profile and record actual Hermes behavior: + truncated YAML; valid YAML with no model block; valid YAML with a + nonexistent model id. +- **Decide** which of these Crew classifies as `broken-config`, biasing + conservative: a false `broken-config` blocks a working agent and is + strictly worse than the boolean it replaces (plan R7). Unparsable YAML + is the floor; anything above it needs evidence here. +- Record whether an invalid *model id* is detectable locally at all + (it may only be knowable to the provider — if so, it is **not** + `broken-config`, and the issue's mention of it must be recorded as + not-locally-detectable rather than silently implemented). + +### I2 — Binary probe cost and caching (→ readiness latency risk) + +- Time `hermes --version` cold and warm. +- Confirm the resolved command path the desktop app sees, reusing the + PATH augmentation already applied in + `desktop/src-tauri/src/managed_agents/discovery.rs`. +- Propose a TTL and the invalidation points; note that + `commands/agent_discovery.rs:295-330` and `:425-455` already + re-evaluate readiness post-install (seam S17). + +### I3 — Archive location (→ OQ-1) + +- Confirm by search that no `NuncioCrew Backups` area exists (expected: + it does not). +- Enumerate candidate locations: the app data dir + (`com.nuncio.crew`), a sibling `…/profile-archives/`, or a + user-visible path. +- Weigh: discoverability by the owner vs. accidental sync to iCloud vs. + permission model. `desktop/src-tauri/src/util.rs:86,242,275` shows the + existing restricted-permission backup helpers — reference for file + mode, not a reusable archive area. +- **Decide and record one location.** Later phases cite this record; + no later phase may hardcode a path first. + +### I4 — Archive format and size (→ OQ-2) + +- Measure a real profile: total size, and size after excluding + `audio_cache/`, `image_cache/`, `logs/` and any other transient + directories actually found on disk (do not assume the issue's list is + complete — enumerate what is there). +- Compare `tar.gz` (needs a crate) against a plain recursive copy (no + new dependency, larger on disk, trivial restore). +- **Decide** the format that meets "archives stay small" with the + smallest dependency delta, and record the definitive exclusion list. +- Record how a pre-action **size estimate** is computed cheaply, since + the issue requires showing it before archiving. + +### I5 — Re-evaluation trigger (→ OQ-4) + +- Determine what can cheaply detect post-spawn breakage: turn-boundary + check, filesystem watch on the profile dir, or a timer. +- Cost each; note that a filesystem watch adds a dependency and a + per-agent handle. +- **Recommend** one, with the fallback being turn-boundary (cheapest, + no new machinery). + +### I6 — Running-agent liveness read (→ phase 04 guard) + +- Identify the authoritative in-process read for "this agent has a live + runtime pair", starting from + `desktop/src-tauri/src/managed_agents/runtime/stop.rs` and + `runtime_commands.rs:313` (`stop_managed_agent_runtime`). +- **Confirm and record** that no SIGTERM→wait→SIGKILL graceful stop + exists: `stop.rs:40,120,153` use `Child::kill()`; the only `SIGTERM` + in `managed_agents` is `discovery.rs:927`, inside the auth-probe + timeout. The spike record is where this correction to the issue's + premise is captured for reviewers (plan DD-4). + +## Files + +- **Create:** `docs/crew/spikes/0015-profile-readiness-and-archive.md` +- **Read only:** `managed_agents/hermes_profile_lifecycle.rs`, + `managed_agents/readiness/hermes.rs`, `managed_agents/readiness.rs`, + `managed_agents/runtime/stop.rs`, `managed_agents/discovery.rs`, + `commands/agent_discovery.rs`, `src-tauri/src/util.rs` +- **Must not touch:** any `desktop/src/**` or `src-tauri/src/**` source + +## Validation + +- Every open question OQ-1 through OQ-4 has a recorded decision with + the evidence that produced it (measured numbers, observed CLI output, + or a cited `path:line`) — not a preference. +- I1 states explicitly whether an invalid model id is locally + detectable. +- I6 states explicitly that graceful stop does not exist today. +- Spike record links the spike-0010 auth-probe ask so `auth-unknown` + has a durable citation for DoD #5. +- `just ci` green (docs-only change, but the gate is not skipped). + +## Risk and rollback + +- **Risk:** the spike concludes that `broken-config` is not reliably + detectable beyond unparsable YAML. That is a legitimate outcome — + phase 02 then ships the narrower honest state and the plan records + the reduction rather than faking depth. +- **Risk:** no acceptable archive location exists without a new + user-facing surface. Escalate to the issue owner before phase 04 + rather than inventing one. +- **Rollback:** delete the spike record. No product surface changes. + +## PR + +Docs-only PR to `Nuncio-hq/crew` (D-020), branch +`agents/profile-lifecycle-spike`. `git commit -s`. Does not change +shipped state, so `docs/crew/STATE.md` is not required by #117 for this +phase — every later phase does require it. diff --git a/plans/20260810-profile-lifecycle-hardening/phase-02-readiness-model-backend.md b/plans/20260810-profile-lifecycle-hardening/phase-02-readiness-model-backend.md new file mode 100644 index 00000000000..ccf7f84d56c --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-02-readiness-model-backend.md @@ -0,0 +1,140 @@ +--- +phase: 02 +title: Readiness model, evaluator, and projection +status: planned +priority: P0 +effort: M (2-3 d) +dependencies: ["01"] +--- + +# Phase 02 — Readiness model, evaluator, and projection + +## Outcome + +Crew stops asking one question about a Hermes profile. The five named +states from the issue exist in Rust, are produced by a real evaluator, +and reach the frontend through the existing named-reason pipeline — +without any frontend rival table and without a `runtime.id` check +anywhere. + +DoD coverage: #1 (model + evaluator + projection half), #5 +(`auth-unknown` as an honest advisory field). + +## Design constraints inherited from the plan + +- **DD-1:** `missing` / `broken-config` / `binary-missing` are + **blocking** and become `Requirement` variants. `ready` / + `auth-unknown` are **non-blocking advisory** and ride a separate + field. Making `auth-unknown` a `Requirement` would push every healthy + Hermes agent into setup-listener mode via `runtime.rs` — a full + outage of the runtime. +- **DD-2:** the carrier is the per-agent `Requirement` pipeline plus the + runtime status projection, **not** `AcpRuntimeCatalogEntry` or + `lib/agentConfigCore.ts`. The catalog holds harness-scoped capability + facts and `agentConfigCore.ts` projects field descriptors + (`desktop/src/features/agents/AGENTS.md` rule 1); per-agent, + per-machine, time-varying readiness belongs in neither. +- **D-025:** the `Requirement` boundary is generic. Everything + Hermes-specific (YAML parse, `hermes --version`, profile dir layout) + stays inside `readiness/hermes.rs`, explicitly labelled as + Hermes-specific in doc comments. +- **DD-7:** RED tests first, in this PR, with observed failure output in + the PR body; then implement to green. + +## Seams + +| Seam | Use | +| ---- | --- | +| `managed_agents/readiness.rs:284` (`Requirement`), `:333` (`HermesProfileDirectoryMissing`) | Add sibling variants | +| `managed_agents/readiness.rs:340` (`AgentReadiness`) | Shape unchanged | +| `managed_agents/readiness/hermes.rs` (`hermes_requirements`) | Where the new checks land | +| `managed_agents/hermes_profile_lifecycle.rs:108` (`hermes_profile_directory_exists`), `hermes_home()`, `hermes_profile_dir()` | Existing path resolution — reuse, do not duplicate | +| `managed_agents/runtime_types.rs:90` (`local_setup`) | Additive sibling field for the named state | +| `shared/api/types.ts:296` (`localSetup`) | TS mirror | +| `shared/lib/configNudge.ts:69` | TS mirror of the new variants | + +## Work + +1. **RED contract tests** (write first, watch fail, record output): + - Each of the five states produced from a fixture: healthy profile → + `ready` + `auth-unknown` advisory; deleted dir → `missing`; + corrupt `config.yaml` → `broken-config`; binary off PATH → + `binary-missing`. + - `auth-unknown` on a healthy profile does **not** produce any + `Requirement` and does **not** yield `AgentReadiness::NotReady` + (the DD-1 regression guard — this is the single most important + test in the phase). + - Each blocking state carries human-readable, actionable copy. + - Tests use a temp `HERMES_HOME` and `lock_path_mutex()`, matching + the three existing tests in `readiness/hermes.rs`. + +2. **Extend `Requirement`** (upstream file, ~+25 lines): two additive + variants for broken config and unrunnable binary, alongside the + existing `HermesProfileDirectoryMissing` and `MissingBinary`. + Additive only — no restructuring, no reordering, no restyling + (`UPSTREAM-SYNC.md` § Thin-fork rules). Check whether the existing + `MissingBinary { command }` (`readiness.rs:328`) already covers + `binary-missing` for the ambient case; if it does, reuse it rather + than adding a variant, and record that in the PR. + +3. **Extend the evaluator** in `readiness/hermes.rs` (Crew-owned): + config parse per the phase-01 decision, cached binary probe per + phase-01 TTL, existing directory check. Doc-comment the file as + Hermes-specific per D-025. No `unwrap()`/`expect()`; no `unsafe`. + +4. **Advisory channel:** add the non-blocking readiness field to + `runtime_types.rs` (~+4 lines) and mirror in `types.ts` (~+3 lines). + It carries the named state including `ready` and `auth-unknown`, + plus the copy for honest degradation. + +5. **Mirror the new variants** in `configNudge.ts` (~+18 lines), one + arm each, following the `hermes_profile_directory_missing` pattern + at `:69`. + +6. **`docs/crew/STATE.md`** updated in this PR (#117 anti-drift, + plan DD-8). + +## Files + +- **Modify (upstream, justified):** `managed_agents/readiness.rs`, + `managed_agents/runtime_types.rs`, `shared/api/types.ts`, + `shared/lib/configNudge.ts` +- **Modify (Crew-owned):** `managed_agents/readiness/hermes.rs` +- **Read only:** `hermes_profile_lifecycle.rs`, `discovery.rs` (PATH + augmentation), phase-01 spike record +- **Must not touch:** `AcpRuntimeCatalogEntry` / runtime catalog, + `lib/agentConfigCore.ts` (DD-2), the create flow, occupancy checks, + owner-only/local invariants (issue non-goals) + +## Validation + +- All RED tests green; the DD-1 guard test explicitly asserted. +- `cargo test --manifest-path desktop/src-tauri/Cargo.toml` green (the + root workspace does not run desktop tests — `AGENTS.md` gotcha 5). +- `pnpm exec tsc --noEmit` clean in `desktop/`. +- `node desktop/scripts/check-file-sizes.mjs` passes — if a touched file + nears `MAX_LINES = 1000`, extract Crew deltas into Crew-owned files + (D-022). Never raise the limit, never add an override. +- `just ci` green. +- Manual: a healthy Hermes agent still spawns into a working session, + not setup mode. + +## Risk and rollback + +- **Risk:** an over-eager config check reports `broken-config` on a + working profile and blocks it. Mitigation: phase-01's conservative + decision, plus the requirement that every blocking state is + recoverable from the nudge without dropping to a terminal. +- **Risk:** binary probe latency on every readiness read. Mitigation: + phase-01 TTL; invalidate at the post-install re-evaluation points + (`commands/agent_discovery.rs:295-330`, `:425-455`). +- **Rollback:** the new variants are additive; reverting the evaluator + restores the directory-only check. No data migration, no persisted + state. + +## PR + +Branch `agents/profile-readiness` (product area, not phase number). +Target `Nuncio-hq/crew` (D-020). `git commit -s`. PR body records the +RED failure output, the upstream diff sizes actually produced against +the plan's estimates, and the D-025 generic/Hermes split. diff --git a/plans/20260810-profile-lifecycle-hardening/phase-03-preflight-attention-routing.md b/plans/20260810-profile-lifecycle-hardening/phase-03-preflight-attention-routing.md new file mode 100644 index 00000000000..367576ca889 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-03-preflight-attention-routing.md @@ -0,0 +1,123 @@ +--- +phase: 03 +title: Spawn preflight and attention routing +status: planned +priority: P0 +effort: M (2 d) +dependencies: ["02"] +--- + +# Phase 03 — Spawn preflight and attention routing + +## Outcome + +Detectable profile breakage never becomes a silent stall. When readiness +is blocking, the owner learns *why* through the surfaces they already +watch — attention and Needs-You — instead of watching an agent sit +quiet. + +DoD coverage: #2. + +## What already exists (do not rebuild) + +`desktop/src-tauri/src/managed_agents/runtime.rs:~575-630` **already** +evaluates readiness at spawn: on `AgentReadiness::NotReady` it builds +`BUZZ_ACP_SETUP_PAYLOAD` (`{agent_name, agent_pubkey, requirements}`), +sets `spawned_setup_mode`, and `buzz-acp` runs as a setup listener that +publishes a kind:9 `buzz:config-nudge` fenced sentinel, rendered by +`config-nudge-attachment.tsx`. + +Phase 02 makes that path fire for three states instead of one. This +phase closes the two real gaps (plan DD-3): + +- **Gap A:** the nudge lands in the channel, but nothing routes it into + the agent's attention state or Needs-You. An owner not reading that + channel still sees an agent that "just doesn't answer". +- **Gap B:** breakage that appears *after* a healthy spawn (profile + deleted mid-turn, binary removed) is never re-detected. + +## Seams + +| Seam | Use | +| ---- | --- | +| `managed_agents/runtime.rs` setup-payload branch (~:575-630) | The single hook point (~+15 lines); logic in Crew files | +| `commands/agent_discovery.rs:295-330`, `:425-455` (`should_restart_after_install`) | The existing model for re-evaluating readiness after the fact — copy the shape, not the code | +| `desktop/src/features/agents/agentAttention.ts` | Named attention states (`AGENTS.md` rule 3 — named reasons, not booleans) | +| `desktop/src/features/agents/needsYouStore.ts`, kinds 46010/46040 | Owner-visible item with TTL | +| `shared/lib/configNudge.ts` `extractConfigNudge()` | Already parses the payload; reuse as the routing input | + +## Work + +1. **RED contract tests** first (DD-7), failure output in the PR body: + - Spawn with a blocking readiness state produces an attention state + with the named reason, not a generic "not responding". + - The attention item's copy is actionable and names the repair + (recreate / rebind / install / fix config), not just the fault. + - A healthy spawn produces **no** attention item and **no** + Needs-You entry — the false-positive guard. + - `auth-unknown` alone never produces an attention item (DD-1 + regression guard at the routing layer). + - Re-evaluation: an agent healthy at spawn whose profile disappears + mid-turn transitions to the blocking state at the next trigger + rather than hanging. + +2. **Gap A — attention routing.** In a Crew-owned module, map the + existing config-nudge / requirement signal onto an attention state + and, where the owner must act, a Needs-You item. Keyed on the + *presence and kind of requirement*, never on `runtime.id` (D-025) — + a future engine emitting requirements routes identically with no + change here. + +3. **Gap B — re-evaluation.** Implement the trigger phase 01 recommended + (default: turn boundary — cheapest, no new machinery, no watcher + handles). Re-run the phase-02 evaluator; on a transition to blocking, + route as in Gap A. Respect the cached binary probe TTL so this does + not become a per-turn process spawn. + +4. **`runtime.rs` hook** (upstream, ~+15 lines): the smallest possible + call into the Crew-owned routing at the existing branch. Do not + restructure the surrounding function. + +5. **`docs/crew/STATE.md`** updated in this PR (#117). + +## Files + +- **Modify (upstream, justified):** `managed_agents/runtime.rs` +- **Create (Crew-owned):** readiness→attention routing module under + `desktop/src/features/agents/` (or its Rust-side counterpart if the + spike places the trigger in Rust) + its tests +- **Modify (Crew-owned):** attention/Needs-You wiring as required +- **Read only:** `commands/agent_discovery.rs`, `configNudge.ts`, + phase-02 output +- **Must not touch:** the setup-payload contract itself, `buzz-acp` + setup-listener behavior, the kind:9 sentinel format — all generic + Buzz contracts that other engines depend on + +## Validation + +- RED tests green, including both false-positive guards. +- Fault-injection by hand: delete a bound profile directory mid-turn → + the agent surfaces the named state at the next trigger instead of + going quiet; restore it → the state clears without a restart. +- `cargo test --manifest-path desktop/src-tauri/Cargo.toml`, desktop + unit tests, `pnpm exec tsc --noEmit`, `just ci` — all green. +- Confirm no component gained a `runtime.id` branch (D-025 / + `features/agents/AGENTS.md` rule 1). + +## Risk and rollback + +- **Risk:** attention noise. A profile that is briefly unreadable + (editor writing `config.yaml`) must not spam Needs-You. Mitigation: + route on a *stable* transition, not on every read; Needs-You entries + are deduplicated per agent per state. +- **Risk:** per-turn re-evaluation adds latency to every turn. + Mitigation: cached probe; directory check is a cheap stat. +- **Rollback:** revert the `runtime.rs` hook — the setup-mode path + returns to its current shipped behavior, which is functional, just + quieter. + +## PR + +Branch `agents/profile-preflight`. Target `Nuncio-hq/crew` (D-020). +`git commit -s`. PR body records the RED output and the fault-injection +transcript for the mid-turn case. diff --git a/plans/20260810-profile-lifecycle-hardening/phase-04-archive-restore-backend.md b/plans/20260810-profile-lifecycle-hardening/phase-04-archive-restore-backend.md new file mode 100644 index 00000000000..0ae1d7ce834 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-04-archive-restore-backend.md @@ -0,0 +1,145 @@ +--- +phase: 04 +title: Archive, restore, permanent delete, running-agent guard +status: planned +priority: P0 +effort: L (3-4 d) +dependencies: ["01"] +--- + +# Phase 04 — Archive, restore, permanent delete, running-agent guard + +## Outcome + +Offboarding stops destroying an employee record. The backend can file a +profile away with a manifest, bring it back, and — only for something +already filed away — destroy it deliberately. Every destructive path +refuses while the agent is running. + +DoD coverage: #3 (backend half), #4 (authoritative half). + +## Design constraints inherited from the plan + +- **DD-4:** the running-agent guard **refuses** while a runtime pair is + alive. It does not stop the agent for the owner. The issue's + "existing graceful stop machinery: SIGTERM → wait → SIGKILL fan-out" + does not exist — `managed_agents/runtime/stop.rs:40,120,153` uses + `Child::kill()` (immediate SIGKILL), and the only `SIGTERM` in + `managed_agents` is `discovery.rs:927`, inside the auth-probe + timeout. Building graceful stop is out of scope for #119. +- **DD-5 / OQ-1 / OQ-2:** archive location, format, and exclusion list + come from the phase-01 spike record. This phase **may not** invent + them. +- The guard is authoritative **in Rust**. UI disabling (phase 06) is + advisory only — a Playwright-invisible path must not be able to + corrupt a live profile (plan R8). +- **DD-7:** RED tests first, failure output in the PR body. + +## Seams + +| Seam | Use | +| ---- | --- | +| `managed_agents/hermes_profile_lifecycle.rs` — `HermesProfileLifecycleResult` | Named-result philosophy: every new op returns named states with human copy, never a bool | +| `hermes_profile_lifecycle.rs` — `hermes_home()`, `hermes_profiles_dir()`, `hermes_profile_dir()`, `list_profiles()` | Path resolution and listing — reuse, never re-derive | +| `managed_agents/hermes_profile.rs:13` (`HERMES_FORBIDDEN_PROFILE_NAME = "default"`), `:19` (`validate_hermes_profile_name`) | Every new destructive path routes through both | +| `commands/hermes_profiles.rs:11,17,29` | Where the new IPC commands live, beside `list`/`create`/`delete` | +| `lib.rs:795-797` | Three additive `invoke_handler` registration lines | +| `managed_agents/runtime/stop.rs`, `runtime_commands.rs:313` | Liveness read for the guard (per phase-01 I6) | +| `src-tauri/src/util.rs:86,242,275` | Reference for restricted file permissions on archive artifacts | + +## Work + +1. **RED contract tests** (write first, observe failure, record): + - Archive round-trip: archive → the live profile directory is gone, + the archive exists, the manifest parses. + - Manifest content: profile name, archive timestamp, bound agent name + + pubkey, optional free-text reason. + - Cache exclusion: caches present before archive are absent from the + archive; non-cache content survives byte-identical. + - Size estimate is produced before the action and is within a stated + tolerance of the real archive. + - Restore: unpacks to `~/.hermes/profiles/` with content + intact. + - Collision: restore onto a live profile of the same name is + **refused** with a named result, and the live profile is untouched. + - Permanent delete: succeeds on an archive; is **not exposed** for a + live profile; requires the type-name confirmation token. + - Guard: archive / restore-over / permanent-delete are refused while + a runtime pair is alive, with a named reason. Repeat after stop → + succeeds. + - `default` is hard-rejected on every new path; invalid names + rejected before any filesystem write. + - Path-traversal guard: a manifest or archive name containing + `..`/separators cannot escape the archive area or the profiles dir. + +2. **Archive service** (Crew-owned Rust module): pack per the phase-01 + format decision, applying the phase-01 exclusion list; write the + manifest; compute the size estimate; restricted permissions per S18. + Never touch `~/.hermes` root, never touch `default`. + +3. **Restore service**: read manifest, collision-check against + `list_profiles()`, unpack, return a named result that carries enough + for the UI to offer re-bind. + +4. **Permanent delete**: operates on an archive identifier only. The + signature takes the confirmation token so the backend, not the + dialog, is the gate. + +5. **Guard**: a shared precondition used by all three destructive ops, + reading liveness per phase-01 I6. Named refusal result carrying the + reason and the agent identity. + +6. **IPC commands** in `commands/hermes_profiles.rs` (Crew-owned) + + three registration lines in `lib.rs` (upstream, ~+3 lines) + + invoke wrappers in `shared/api/hermesProfiles.ts` (Crew-owned, + beside `:42,48,56`). + +7. **`docs/crew/STATE.md`** updated in this PR (#117). + +## Files + +- **Create (Crew-owned):** archive/restore/permanent-delete service + module(s) under `desktop/src-tauri/src/managed_agents/` + tests +- **Modify (Crew-owned):** `commands/hermes_profiles.rs`, + `shared/api/hermesProfiles.ts` +- **Modify (upstream, justified):** `lib.rs` (3 registration lines) +- **Read only:** `hermes_profile_lifecycle.rs`, `hermes_profile.rs`, + `runtime/stop.rs`, `util.rs`, phase-01 spike record +- **Must not touch:** `usePersonaActions.ts` and the dialogs (phase 06), + the create flow, occupancy checks, owner-only/local invariants + +## Validation + +- All RED tests green, run against a temp `HERMES_HOME` with + `lock_path_mutex()`, matching the existing lifecycle test pattern. +- Guard test proves refusal-while-running is enforced in Rust with no + UI involved. +- `cargo test --manifest-path desktop/src-tauri/Cargo.toml` green. +- `node desktop/scripts/check-file-sizes.mjs` passes; split by + responsibility (D-022) if a module grows — never raise `MAX_LINES`. +- No `unsafe`, no new `unwrap()`/`expect()` in production paths. +- `just ci` green. + +## Risk and rollback + +- **Risk (highest in the plan):** a bug here destroys real profile + state. Mitigation: archive is **copy-then-verify-then-remove**, never + move-then-hope; the live directory is removed only after the archive + is written and re-read successfully. Any failure leaves the live + profile intact and returns a named failure. +- **Risk:** the archive area fills the disk over time. Out of scope to + manage (scheduled backups are a stated non-goal), but the manifest + and size estimate make the cost visible; note retention as a + follow-up. +- **Risk:** exclusion list drifts as Hermes adds cache dirs. Mitigation: + the manifest records the exclusion list actually applied, so a stale + list is diagnosable from any archive. +- **Rollback:** the commands are additive and unreferenced until phase + 06 wires the UI. Reverting removes capability without changing any + shipped flow. + +## PR + +Branch `agents/profile-archive`. Target `Nuncio-hq/crew` (D-020). +`git commit -s`. PR body records the RED output and an archive → +restore transcript against a scratch profile. diff --git a/plans/20260810-profile-lifecycle-hardening/phase-05-readiness-surfacing-ui.md b/plans/20260810-profile-lifecycle-hardening/phase-05-readiness-surfacing-ui.md new file mode 100644 index 00000000000..8819db920a0 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-05-readiness-surfacing-ui.md @@ -0,0 +1,123 @@ +--- +phase: 05 +title: Readiness surfacing — agent card and edit dialog +status: planned +priority: P1 +effort: M (2 d) +dependencies: ["02"] +--- + +# Phase 05 — Readiness surfacing: agent card and edit dialog + +## Outcome + +The owner can see profile health at a glance in AgentsView, without +opening a dialog — and can see the detail and the repair when they do. +`auth-unknown` reads as a known limit, not a fault. + +DoD coverage: #1 (card + dialog half), #5 (display half). + +## Design constraints inherited from the plan + +- **DD-2:** consume the per-agent projection from phase 02. No frontend + rival table, no `runtime.id` check in any component + (`features/agents/AGENTS.md` rule 1). +- **AGENTS.md rule 3:** named reasons, never booleans. The specific + target is `managedAgentRuntimeStatus.ts:12`: + `if (!runtime.localSetup) return "Needs setup on this device"` — the + boolean the issue is asking to replace. +- **DD-1:** `auth-unknown` renders as neutral/informational and never as + an error state. Every healthy Hermes agent is `auth-unknown` today; a + warning treatment would train the owner to ignore the badge (plan + risk table). +- Text sizing: rem tokens only, never px (`AGENTS.md` § Text sizing). + Meta text uses `text-2xs` / `text-3xs`; no arbitrary literals — the CI + guard `pnpm check:px-text` fails on px *and* rem literals. + +## Seams + +| Seam | Use | +| ---- | --- | +| `features/agents/managedAgentRuntimeStatus.ts:12` | Replace the `localSetup` boolean read with the named state (~+20 / -4) | +| `features/agents/ui/AgentStatusBadge.tsx` | Badge variant per state (~+12) | +| `features/agents/ui/ManagedAgentRow.tsx` | Render the badge on the card (~+8) | +| `features/agents/ui/AgentsView.tsx:~218` (``), profiles collected `:398-416` | The card list — read-only if the badge composes into `ManagedAgentRow` | +| `shared/ui/config-nudge-attachment.tsx:43,139,162,421` | Two new render arms delegating to Crew-owned rows (~+20) | +| `shared/ui/HermesProfileOrphanRepairRow.tsx` | The pattern for the new repair rows; new rows are Crew-owned siblings | +| `features/agents/ui/EditAgentModelAndProfileSection.tsx` (Crew-owned) | Dialog detail lives here | + +## Work + +1. **RED tests** first (DD-7): component/unit tests asserting each of + the five states renders its own copy; `auth-unknown` renders + informational, not error; no component references `runtime.id`. + +2. **Named status projection**: replace the boolean read in + `managedAgentRuntimeStatus.ts` with the phase-02 state, mapping each + to copy that names the fault *and* the repair. + +3. **Card badge**: variant in `AgentStatusBadge.tsx`, rendered by + `ManagedAgentRow.tsx`. Glanceable — state distinguishable without + opening anything. Keep `AgentsView.tsx` untouched if the badge + composes into the row (preferred; smaller upstream delta). + +4. **Dialog detail** in the Crew-owned edit section: full state, copy, + and the repair affordance. + +5. **Nudge repair rows**: two new Crew-owned row components beside + `HermesProfileOrphanRepairRow.tsx` — one for broken config, one for + missing binary — wired by two additive arms in + `config-nudge-attachment.tsx`. Each states the fault and offers the + repair without requiring a terminal (plan R7). + +6. **`auth-unknown` copy**: "auth not verifiable", linking the + spike-0010 ask, per the issue's explicit wording — never green, never + red. + +7. **`docs/crew/STATE.md`** updated in this PR (#117). + +## Files + +- **Modify (upstream, justified):** `managedAgentRuntimeStatus.ts`, + `AgentStatusBadge.tsx`, `ManagedAgentRow.tsx`, + `shared/ui/config-nudge-attachment.tsx` +- **Create (Crew-owned):** two nudge repair row components + tests +- **Modify (Crew-owned):** `EditAgentModelAndProfileSection.tsx` +- **Must not touch:** `AcpRuntimeCatalogEntry` / catalog, + `lib/agentConfigCore.ts` (DD-2); `usePersonaActions.ts` and the delete + dialogs (phase 06) + +## Validation + +- RED tests green; the "no `runtime.id` in components" assertion holds. +- `pnpm exec tsc --noEmit` clean; biome clean. +- `pnpm check:px-text` passes — no new px or arbitrary rem text sizes. +- `node desktop/scripts/check-file-sizes.mjs` passes (D-022 if near). +- Playwright smoke for the card states, built with `pnpm build:e2e` + (never plain `pnpm run build` — the mock bridge is compiled in only + for `--mode e2e`; a plain build fails every mock spec with + `Cannot read properties of undefined (reading 'invoke')`). +- Screenshot states are **distinct**: scope each shot with + `locator.screenshot()` and gate on + `shasum -a 256 test-results//*.png` — every hash unique before + posting. Post via `scripts/post-screenshots.sh`; delete superseded + comments. +- `just ci` green. + +## Risk and rollback + +- **Risk:** badge noise — five states on every card is visual clutter. + Mitigation: `ready` renders no badge; only non-`ready` states draw + attention. `auth-unknown` is detail-level, not a card badge, unless + the phase-01/product read says otherwise. +- **Risk:** stale state on the card if the projection does not refresh. + Mitigation: the phase-03 re-evaluation trigger drives it; verify the + card clears after a repair without an app restart. +- **Rollback:** revert the four upstream edits; the boolean status + returns. New Crew-owned rows become unreferenced. + +## PR + +Branch `agents/profile-readiness-ui`. Target `Nuncio-hq/crew` (D-020). +`git commit -s`. PR body carries the distinct-hash screenshot set for +the card states. diff --git a/plans/20260810-profile-lifecycle-hardening/phase-06-offboarding-archive-ui.md b/plans/20260810-profile-lifecycle-hardening/phase-06-offboarding-archive-ui.md new file mode 100644 index 00000000000..5db232486d9 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-06-offboarding-archive-ui.md @@ -0,0 +1,136 @@ +--- +phase: 06 +title: Offboarding archive, restore, and permanent-delete UI +status: planned +priority: P0 +effort: L (3 d) +dependencies: ["04", "05"] +--- + +# Phase 06 — Offboarding archive, restore, and permanent-delete UI + +## Outcome + +The destructive offboard branch becomes archive. The owner can bring an +archived employee back and re-bind them. Permanent destruction exists, +but only for something already archived, and only after typing the +profile name. + +DoD coverage: #3 (UI half), #4 (UI half — advisory). + +## Design constraints inherited from the plan + +- **DD-4:** the running-agent guard is authoritative in Rust (phase 04). + The UI disable is **advisory** — it must state the reason and offer a + path to stop the agent, and it must handle a backend refusal + gracefully even when the UI believed the action was allowed. +- Permanent delete is **never** offered on a live profile. Only on an + archive. This is the issue's explicit requirement and the guardrail + that makes archive-by-default safe. +- Keep the existing keep-vs-destructive shape and its test ids + (`data-testid="hermes-profile-offboard-{keep,delete}"`); **keep** + remains the default (C-13). Archive replaces the *destructive* branch; + it does not become the new default. +- Every profile name entering a destructive path is validated and + `default`-rejected (`hermes_profile.rs:13,19`) — client-side is + explanatory, the server is the authority (HERMES.md rule 5 pattern). + +## Seams + +| Seam | Use | +| ---- | --- | +| `features/agents/ui/usePersonaActions.ts:297-317` | The irreversible `deleteHermesProfile` loop being replaced (~+15 / -20) | +| `features/agents/ui/PersonaDeleteDialog.tsx` | Dialog shell stays upstream; mount Crew-owned fields (~+10) | +| `features/agents/ui/HermesProfileOffboardFields.tsx` (Crew-owned) | The radio set gains the archive option and the size estimate | +| `shared/api/hermesProfiles.ts` | Phase-04 invoke wrappers | +| Existing binding UI / profile picker (`EditAgentModelAndProfileSection.tsx`) | Re-bind after restore — reuse, do not build a second binding surface | +| `hermesProfileDeleteCommandLine(name)` (shown in offboard fields) | The auditable-command-line pattern (P-6) to mirror for archive | + +## Work + +1. **RED tests** first (DD-7): + - Offboard dialog: keep is default; the destructive option is + archive; permanent delete is absent for a live profile. + - Size estimate renders before the action. + - Archive disabled with a stated reason while the agent runs; + enabled after stop. + - A backend refusal (guard, collision, invalid name) surfaces its + named message rather than a generic failure. + - Restore picker lists archives with manifest info; restore onto a + colliding live name is blocked with a clear message. + - Permanent delete requires the exact typed profile name; a + near-miss does not enable the action. + +2. **Offboard fields**: add the archive option to the Crew-owned + `HermesProfileOffboardFields.tsx`, with the size estimate from + phase 04, the optional free-text offboard reason (written to the + manifest), and the auditable command/operation line. + +3. **`usePersonaActions.ts`** (upstream): replace the + `deleteHermesProfile` branch at `:297-317` with a call into a + Crew-owned hook that invokes the archive command. Keep the edit + minimal — the logic lives in the Crew-owned hook. + +4. **Restore surface**: a Crew-owned view listing archives (name, + timestamp, bound agent, reason, size). Restore → on success, offer + re-bind through the *existing* binding UI. Collision message is the + backend's named result, surfaced verbatim in intent. + +5. **Permanent delete**: on an archive row only, behind + type-the-profile-name confirmation, passing the token to the backend + (which is the real gate). + +6. **`docs/crew/STATE.md`** updated in this PR (#117). + +## Files + +- **Modify (upstream, justified):** `usePersonaActions.ts`, + `PersonaDeleteDialog.tsx` +- **Modify (Crew-owned):** `HermesProfileOffboardFields.tsx` +- **Create (Crew-owned):** archive hook, restore/archive list view, + permanent-delete confirmation component + tests +- **Must not touch:** the create flow, occupancy checks, owner-only / + local invariants (issue non-goals); the phase-04 backend contracts + +## Validation + +- RED tests green, including the guard-refusal and near-miss-token + cases. +- `pnpm exec tsc --noEmit` clean; biome clean; `pnpm check:px-text` + passes. +- `node desktop/scripts/check-file-sizes.mjs` passes — if + `HermesProfileOffboardFields.tsx` or a dialog grows, split by + responsibility (D-022), never raise `MAX_LINES`. +- Playwright specs (mock bridge) for offboard-with-archive, restore + picker, collision block, and the type-name gate. Build with + `pnpm build:e2e`; kill port 4173 first if a stale preview server is + running (`reuseExistingServer: true` will otherwise serve old code). + `page.addInitScript` before `installMockBridge(page)`; call + `waitForAnimations(page)` before every screenshot. +- Distinct-state screenshots: `locator.screenshot()` per state, + `shasum -a 256` all-unique gate before posting via + `scripts/post-screenshots.sh`. +- `just ci` green. + +## Risk and rollback + +- **Risk:** the owner reads "archive" as "delete" and expects space + freed. Mitigation: copy states plainly that the profile is filed and + restorable, and shows the archive size. +- **Risk:** UI and backend disagree about liveness, so a disabled + button hides a working action or an enabled one gets refused. + Mitigation: the backend is authoritative (DD-4); the UI surfaces the + refusal rather than swallowing it. +- **Risk:** restore-then-rebind leaves an agent bound to a name that + does not exist if the rebind step is abandoned. Mitigation: restore + completes independently of rebind; the resulting unbound state is + exactly the existing `missing` readiness class from phase 02, already + repairable from the nudge. +- **Rollback:** revert the two upstream edits — offboarding returns to + keep-vs-delete. Phase-04 commands become unreferenced but harmless. + +## PR + +Branch `agents/profile-offboard-archive`. Target `Nuncio-hq/crew` +(D-020). `git commit -s`. PR body carries the distinct-hash screenshot +set for offboard / restore / permanent-delete states. diff --git a/plans/20260810-profile-lifecycle-hardening/phase-07-verification-and-docs.md b/plans/20260810-profile-lifecycle-hardening/phase-07-verification-and-docs.md new file mode 100644 index 00000000000..ec7f87d9685 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/phase-07-verification-and-docs.md @@ -0,0 +1,134 @@ +--- +phase: 07 +title: Verification, fault injection, and docs +status: planned +priority: P1 +effort: M (2 d) +dependencies: ["03", "06"] +--- + +# Phase 07 — Verification, fault injection, and docs + +## Outcome + +The issue's Verification section is executed against real state, the +evidence is on the PR, and the durable docs tell the truth about what +shipped. + +DoD coverage: #5 (documentation half), #6 (whole). + +## Scope note + +This phase owns the **end-to-end live verification** and the +**narrative docs consolidation**. It does not own per-PR STATE.md +updates — those are each shipping phase's obligation (#117 anti-drift, +plan DD-8). If phase 07 discovers STATE.md is stale, that is a defect in +the earlier phase, recorded as such. + +## Work + +### 1. Fault-injection probe (issue Verification bullet 1) + +On a scratch profile, for each state, record the observed card state, +the spawn behavior, and the attention/Needs-You item: + +| Injection | Expected | +| --------- | -------- | +| Corrupt `config.yaml` | card `broken-config`; spawn preflight fails fast with actionable copy | +| Remove `hermes` from the PATH the app sees | card `binary-missing` | +| Delete the profile directory | card `missing` | +| Healthy profile | card `ready`, with `auth-unknown` noted honestly (informational, not an error) | + +Also verify each state **clears** after repair without an app restart. + +### 2. Archive round-trip live (issue Verification bullet 2) + +Offboard a scratch agent with archive → inspect the archive: caches +excluded, manifest present and correct → restore → re-bind → the agent +answers a mention **with profile memory intact**. Memory intactness is +the real assertion; a byte-count check is not sufficient evidence. + +### 3. Guard live (issue Verification bullet 3) + +Attempt archive while the agent is running → blocked with the reason. +Stop the agent → succeeds. Note in the record that stop is the existing +immediate `Child::kill()` path, not a graceful SIGTERM sequence (plan +DD-4) — so reviewers do not read the guard copy as promising a graceful +drain. + +### 4. Playwright + screenshot evidence (issue Verification bullet 4) + +Specs from phases 05 and 06 registered in +`desktop/playwright.config.ts` (`smoke` project `testMatch`). Build with +`pnpm build:e2e`; prefer `pnpm test:e2e:smoke` over a manual build plus +`playwright test`. Distinct-state gate: +`shasum -a 256 test-results//*.png` — every hash unique. Post via +`scripts/post-screenshots.sh [body.md]` with `{{filename}}` +placeholders; delete superseded screenshot comments so reviewers see +only the current set. + +### 5. Docs + +- **`docs/crew/HERMES.md`**: + - § Offboarding (`:137`) — keep vs **archive**; restore + re-bind; + permanent delete only on archives behind type-name confirmation; + running-agent guard. Keep the CLI fallback and the spike-0011 `-y` + warning (`:154`) intact. + - § Failure classes (`:157`) — rows for `broken-config` and + `binary-missing` alongside the existing orphan rows. + - § Known gaps (`:197`) — `auth-unknown` is now *surfaced honestly*, + still blocked on the Hermes-side probe; keep the spike-0010 link + (DoD #5's durable citation). + - Archive location, format, and exclusion list, citing spike 0015. +- **`docs/crew/STATE.md`**: consolidate the shipped state for #119. + While here, reconcile two known staleness points found during + planning — Slice 3 still described as an upstream tier-1 PR to + `block/buzz` (superseded by D-020), and Slice 4 described as a future + gate though Phase 03/04 shipped. Fixing them is in scope for this + phase; do not let the file keep contradicting D-020. +- **`docs/crew/DECISIONS.md`**: D-028 (next free — D-027 is the highest + existing entry) — archive-on-offboard semantics + (archive replaces destructive delete; permanent delete only on + archives; guard refuses while running; archive location + format). + Brief, per the issue's recommendation (OQ-5). + +## Files + +- **Modify:** `docs/crew/HERMES.md`, `docs/crew/STATE.md`, + `docs/crew/DECISIONS.md` +- **Create:** `docs/crew/verification/0007-profile-lifecycle-hardening.md` + (next free — 0006 is the highest existing) — the fault-injection and + round-trip transcript +- **Modify:** `desktop/playwright.config.ts` if new specs need + registration +- **Must not touch:** product code, except spec registration and any + defect fix this phase's own verification exposes + +## Validation + +- All six issue DoD checkboxes demonstrably satisfied, each with named + evidence (test name, transcript section, or screenshot). +- Every fault-injection row observed and recorded — not inferred from + unit tests. +- Docs claims verified against source after editing + (`documentation-management` rule: read before updating, verify links + and claims after). +- `just ci` green; desktop suite green. +- If any earlier phase's STATE.md update was missed, that is recorded as + a defect rather than quietly patched here. + +## Risk and rollback + +- **Risk:** live verification surfaces a defect late. That is the + phase's purpose — fix in the owning phase's area and re-verify; do not + weaken the check to pass. +- **Risk:** `auth-unknown` copy reads as a Crew fault rather than a + known upstream limit. Mitigation: docs and UI both point at the + spike-0010 ask. +- **Rollback:** docs-only revert; verification record retained (a + stateful record, not evergreen authority). + +## PR + +Branch `agents/profile-lifecycle-docs`. Target `Nuncio-hq/crew` +(D-020). `git commit -s`. This is the PR that closes #119. diff --git a/plans/20260810-profile-lifecycle-hardening/plan.md b/plans/20260810-profile-lifecycle-hardening/plan.md new file mode 100644 index 00000000000..b08a8b38f46 --- /dev/null +++ b/plans/20260810-profile-lifecycle-hardening/plan.md @@ -0,0 +1,358 @@ +# Hermes profile lifecycle hardening — granular readiness + archive-on-offboard + +- **Status:** Planned (not started) — planning-only artifact, no branch pushed +- **Date:** 2026-08-10 +- **Issue:** [#119](https://github.com/Nuncio-hq/crew/issues/119) (authoritative spec) +- **Feature:** [`docs/crew/features/0001-hermes-first-class-runtime.md`](../../docs/crew/features/0001-hermes-first-class-runtime.md) +- **Builds on:** [`../20260805-1330-hermes-first-class-runtime/plan.md`](../20260805-1330-hermes-first-class-runtime/plan.md) + (Phase 03 profile lifecycle, Phase 04 picker) +- **Decisions in force:** D-008 (spike → RED → implement), D-019, D-020 + (PRs target `Nuncio-hq/crew` only), D-022 (extract, never raise the + file-size limit), D-024 (trusted one-manager boundary), D-025 (generic + ACP first, Hermes-specific labelled) +- **Phases:** 7 · **Validate:** pass · **Red-team:** 9 findings, 8 applied, + 1 rejected with rationale (see § Validation and red-team) + +## Goal + +Close the two gaps issue #119 found in the shipped Hermes profile +lifecycle: + +1. **Readiness is one question deep.** `hermes_profile_directory_exists()` + (`desktop/src-tauri/src/managed_agents/hermes_profile_lifecycle.rs:108`) + is the only health question Crew asks. A profile with a corrupt + `config.yaml`, a missing model id, or a `hermes` binary that vanished + from PATH reads as healthy until the spawn fails mid-flight — the exact + "agent stuck and I don't know why" class the attention line + (#105 → #114) exists to kill. +2. **Profile delete is irreversible.** `usePersonaActions.ts:297-317` + runs `hermes profile delete -y`; months of memory, skills and + credentials die on one misclick. No archive, no restore. + +The product frame is D-024's: a Hermes profile is an **employee record**. +Offboarding a person files their record; it does not shred it. This plan +turns that sentence into behavior. + +## Scope (from the issue — verbatim intent) + +**A. Readiness granularity (ambient + preflight)** + +- Five named states replacing the boolean: `ready` / `missing` / + `broken-config` / `binary-missing` / `auth-unknown`. +- State visible **on the agent card in AgentsView**, glanceable, plus + detail in the edit dialog. +- **Spawn preflight**: evaluate before a turn starts; on + `missing` / `broken-config` / `binary-missing`, fail fast with + actionable copy routed through the existing attention / Needs-You + surfaces — never a silent mid-turn stall. + +**B. Archive-on-offboard** + +- Offboarding's "delete profile" branch becomes **archive**: pack the + profile to a backups area, excluding caches, showing an estimated size + before the action. +- Archive carries a manifest: profile name, timestamp, bound agent name + + pubkey, optional free-text offboard reason. +- **Restore**: list archives with manifest info; restore unpacks to + `~/.hermes/profiles/` and offers re-bind. Collision with a live + profile blocks with a clear message. +- **Permanent delete** exists only on an *archive*, behind + type-the-profile-name confirmation. +- **Running-agent guard**: destructive profile actions require the + agent's runtime pairs stopped first; the UI disables the action and + states the reason while running. + +## Non-goals (from the issue, restated as plan boundaries) + +- Scheduled or automatic profile backups. +- Rename detection (indistinguishable from deletion; the orphan + + re-bind path already covers it). +- Any auth probe implementation inside Crew. +- Changes to the create flow, occupancy checks (C-10), or the + owner-only / local invariants — already correct, do not touch. + +## Named Buzz seams + +Every deliverable hangs off an existing Buzz seam. Named here so no phase +invents a parallel mechanism. + +| # | Seam | Location | What hangs off it | +| - | ---- | -------- | ----------------- | +| S1 | `Requirement` enum — named blocking reasons | `desktop/src-tauri/src/managed_agents/readiness.rs:284`, hermes orphan variant `:333` | The three *blocking* readiness states extend this enum instead of a new type | +| S2 | `AgentReadiness::{Ready,NotReady{requirements}}` | `readiness.rs:340` | Unchanged shape; the evaluator keeps returning it | +| S3 | Hermes requirements evaluator | `readiness/hermes.rs` — `hermes_requirements(effective)` | Crew-owned file where config-parse + binary-probe checks land | +| S4 | Spawn setup payload | `runtime.rs:~575-630` — builds `BUZZ_ACP_SETUP_PAYLOAD` when `NotReady`, sets `spawned_setup_mode` | The preflight *already exists*; phase 03 extends its reach, does not rebuild it | +| S5 | `buzz-acp` setup-listener → kind:9 `buzz:config-nudge` fenced sentinel | `crates/buzz-acp` setup mode | Unchanged transport for the actionable message | +| S6 | `extractConfigNudge()` + TS `Requirement` mirror | `desktop/src/shared/lib/configNudge.ts:69` (`hermes_profile_directory_missing`) | New variants mirrored here, one arm each | +| S7 | Config-nudge card renderer | `desktop/src/shared/ui/config-nudge-attachment.tsx:43,139,162,421` | New repair rows attach beside `HermesProfileOrphanRepairRow.tsx` | +| S8 | Runtime status projection | `runtime_types.rs:90` (`local_setup: bool`) → `types.ts:296` (`localSetup`) → `managedAgentRuntimeStatus.ts:12` | The boolean the issue wants replaced by named states | +| S9 | Agent card + badge | `AgentsView.tsx:~218`, `ManagedAgentRow.tsx`, `AgentStatusBadge.tsx` | Where the glanceable state renders | +| S10 | Agent attention + Needs-You | `desktop/src/features/agents/agentAttention.ts`, `needsYouStore.ts` (kinds 46010/46040) | Where preflight failure becomes an owner-visible item | +| S11 | Hermes lifecycle service + result enum | `hermes_profile_lifecycle.rs` — `HermesProfileLifecycleResult` | Archive/restore/permanent-delete reuse the named-result philosophy and the guarded CLI/dir paths | +| S12 | Lifecycle IPC commands | `desktop/src-tauri/src/commands/hermes_profiles.rs:11,17,29`, registered `lib.rs:795-797` | Three new commands register alongside | +| S13 | TS invoke wrappers | `desktop/src/shared/api/hermesProfiles.ts:42,48,56` | Archive/restore/permanent-delete wrappers | +| S14 | Offboard choice UI | `HermesProfileOffboardFields.tsx` (`data-testid="hermes-profile-offboard-{keep,delete}"`) | Radio set gains the archive option; delete branch re-homes | +| S15 | Persona delete action | `usePersonaActions.ts:297-317` | The irreversible call site being replaced | +| S16 | Name validation + `default` hard-reject | `hermes_profile.rs:13,19` | Every new destructive path routes through it | +| S17 | Post-install readiness re-evaluation + bounce | `commands/agent_discovery.rs:295-330`, `:425-455` (`should_restart_after_install`) | Closest existing machinery for "re-check readiness after the fact" — the model for mid-turn re-evaluation | +| S18 | Restricted-permission backup helpers | `desktop/src-tauri/src/util.rs:86,242,275` | Reference for archive file permissions; **not** a reusable archive area (see OQ-1) | + +## Design decisions + +### DD-1 — `auth-unknown` is NOT a `Requirement` + +The issue lists five states as a flat set. They are not flat in the +runtime. `Requirement` (S1) implies `AgentReadiness::NotReady` (S2), +which makes `runtime.rs` (S4) spawn the agent into **setup-listener +mode** instead of a working session. Modelling `auth-unknown` as a +`Requirement` would put every healthy Hermes agent into setup mode — +Hermes v0.20.0 has *no* headless auth probe (spike 0010), so +`auth-unknown` is the permanent state of every correctly configured +profile. + +Readiness therefore splits into two channels: + +- **Blocking** — `missing`, `broken-config`, `binary-missing` → new + `Requirement` variants → existing nudge + setup-mode path. +- **Non-blocking advisory** — `ready`, `auth-unknown` → a separate + advisory field on the projection, displayed but never gating spawn. + +The five names in the issue survive intact at the display layer; only +their transport differs. Recorded because it is a deviation of mechanism +from the issue's implied single pipe. + +### DD-2 — Carrier correction: per-agent snapshot, not `AcpRuntimeCatalogEntry` + +The issue's design constraint names +`AcpRuntimeCatalogEntry` / `lib/agentConfigCore.ts` as the pipeline. +Per `desktop/src/features/agents/AGENTS.md` rule 1, the runtime catalog +holds **harness-scoped capability facts** (what a runtime *can* do) and +`agentConfigCore.ts` projects **field descriptors**. Profile readiness is +neither — it is a per-agent, per-machine, time-varying fact about one +bound profile. Putting it in the catalog would make a harness-level table +carry agent-level state, which is what rule 1 forbids. + +The plan honors the constraint's *intent* — no frontend rival table, no +`runtime.id` checks in components, named reasons from Rust — by routing +readiness through the existing per-agent `Requirement` pipeline (S1→S8) +and the runtime status projection. The catalog stays untouched. +This is an explicit, justified deviation, not a dropped requirement. + +### DD-3 — Preflight is an extension, not a new mechanism + +S4 already evaluates readiness at spawn and diverts to setup mode. The +real gap in the issue's framing is **breakage that appears after a +healthy spawn** (profile deleted mid-turn, binary removed). Phase 03 +therefore adds (a) attention/Needs-You routing for the existing +divert, and (b) re-evaluation on turn boundaries modelled on S17. It +does not build a second preflight. + +### DD-4 — Running-agent guard is refuse-while-running, not stop-then-archive + +The issue asserts "existing graceful stop machinery: SIGTERM → wait → +SIGKILL fan-out". **That machinery does not exist.** +`managed_agents/runtime/stop.rs:40,120,153` uses `Child::kill()` +(SIGKILL, immediate). The only `SIGTERM` in `managed_agents` is +`discovery.rs:927`, inside the *auth-probe* 10s timeout — unrelated to +agent stop. + +The guard is therefore implemented as its intended contract — a +precondition that **refuses** a destructive action while any runtime +pair is alive, surfacing the reason and a Stop affordance — and never as +"archive stops the agent for you". Building graceful stop is out of +scope for #119; noted as a follow-up candidate. + +### DD-5 — Archive location is unknown; it is a spike, not an assumption + +The issue references "the existing backups area +(`~/Library/Application Support/NuncioCrew Backups/` pattern)". A +repo-wide search finds **no such area**. `util.rs` (S18) only writes +`.bak.*` siblings for keychain/store files. The path is aspirational. +Phase 01 resolves it; no later phase may hardcode it first. See OQ-1. + +### DD-6 — Generic-ACP check (D-025) + +| Mechanism | Generic or Hermes-specific | +| --------- | -------------------------- | +| `Requirement` variants + nudge transport (S1, S5, S6, S7) | **Generic** — any ACP engine emits requirements today | +| Spawn preflight + setup-mode divert (S4) | **Generic** — untouched contract | +| Attention / Needs-You routing (S10) | **Generic** — keyed on requirement presence, not engine | +| Card readiness badge (S9) | **Generic surface**, Hermes-populated for now | +| `config.yaml` parse, `hermes --version` probe, profile dir layout | **Hermes-specific — explicitly labelled.** Confined to `readiness/hermes.rs` (S3), behind the generic `Requirement` boundary | +| Archive / restore / permanent-delete of a profile directory | **Hermes-specific — explicitly labelled.** Crew-owned files only; the concept of "profile" has no generic ACP equivalent | + +No component branches on `runtime.id`. A future engine that ships its own +health facts emits `Requirement`s through the same seams with zero +frontend change. + +### DD-7 — RED is a gate step inside each phase, never its own PR + +D-008 requires RED contract tests before implementation. A phase whose +PR contains only failing tests cannot merge (`main` stays green, +`UPSTREAM-SYNC.md` § Thin-fork rules). Each implementation phase +therefore begins by writing the failing tests, records the observed +failure output in the PR body, then implements to green in the same PR. + +### DD-8 — STATE.md is updated by every shipping phase + +Issue #117's anti-drift rule: any PR changing shipped state updates +`docs/crew/STATE.md` in the same PR. That is phases 02-07, not only the +docs phase. Phase 07 owns the *narrative* consolidation, not the +per-phase obligation. + +## Thin-fork budget + +Upstream-file edits require explicit justification (`UPSTREAM-SYNC.md` +§ Thin-fork rules). Classification verified with +`git cat-file -e upstream/main:`. + +**Crew-owned (additive, no budget cost):** +`hermes_profile_lifecycle.rs`, `readiness/hermes.rs`, +`hermes_profile.rs`, `commands/hermes_profiles.rs`, +`hermesProfiles.ts`, `HermesProfileOffboardFields.tsx`, +`HermesProfileOrphanRepairRow.tsx`, plus every new file this plan adds. + +**Upstream files this plan edits:** + +| File | Phase | Justification | Expected diff | +| ---- | ----- | ------------- | ------------- | +| `managed_agents/readiness.rs` | 02 | New `Requirement` variants must live in the enum they extend; a Crew-side parallel enum is exactly the "copied upstream implementation" class UPSTREAM-SYNC forbids. Additive variants only — no restructuring. | ~+25 lines (2 variants + match arms) | +| `managed_agents/runtime_types.rs` | 02 | One additive field on the runtime status struct carrying the named state (replaces reading `local_setup` alone). | ~+4 lines | +| `managed_agents/runtime.rs` | 03 | Attention routing hook at the existing setup-payload branch (S4). Smallest possible hook; logic lives in Crew files. | ~+15 lines | +| `shared/api/types.ts` | 02 | TS mirror of the `runtime_types.rs` field. | ~+3 lines | +| `shared/lib/configNudge.ts` | 02 | TS mirror of the new `Requirement` variants — same file already mirrors the hermes orphan variant at `:69`. | ~+18 lines | +| `shared/ui/config-nudge-attachment.tsx` | 05 | Two new render arms delegating to Crew-owned row components. | ~+20 lines | +| `features/agents/managedAgentRuntimeStatus.ts` | 05 | Replace the `!runtime.localSetup` boolean read (`:12`) with the named state. | ~+20 / -4 lines | +| `features/agents/ui/AgentStatusBadge.tsx` | 05 | Badge variant for the readiness state. | ~+12 lines | +| `features/agents/ui/ManagedAgentRow.tsx` | 05 | Render the badge on the card. | ~+8 lines | +| `features/agents/ui/usePersonaActions.ts` | 06 | The irreversible delete call site (`:297-317`) is here; it must change. Replace the branch with a call into a Crew-owned hook. | ~+15 / -20 lines | +| `commands/hermes_profiles.rs` registration in `lib.rs` | 04 | Three `invoke_handler` lines beside `:795-797`. | ~+3 lines | +| `features/agents/ui/PersonaDeleteDialog.tsx` | 06 | Mount the Crew-owned archive fields; keep the dialog shell upstream. | ~+10 lines | + +Total upstream delta ≈ **+153 / -24 lines across 12 files**, every edit a +hook or a mirror, with substantive logic in Crew-owned files. No upstream +file is restyled, reorganized, or copied. + +**File-size ratchet:** `desktop/scripts/check-file-sizes.mjs` +(`MAX_LINES = 1000`). `readiness.rs` is at 1734 lines but is +`src-tauri/src` — already over and grandfathered by the script's scope; +confirm in phase 02 that the additive variants do not trip a new +failure. If any touched file approaches the limit, **extract Crew +deltas into Crew-owned files (D-022)** — never raise the limit, never +add an override. + +## DoD → phase mapping + +Every checkbox in the issue's Definition of Done maps to at least one +phase. + +| # | DoD checkbox | Phases | +| - | ------------ | ------ | +| 1 | Five named readiness states visible on agent card + edit dialog, flowing through the canonical catalog pipeline | 01 (signal validity), **02** (model + evaluator + projection), **05** (card + dialog). Carrier deviation per DD-2. | +| 2 | Spawn preflight fails fast into attention/Needs-You with actionable copy (no silent stalls) | 01 (re-evaluation trigger spike), **03** | +| 3 | Offboarding archives profiles (manifest, cache-excluded, size shown); restore + re-bind end-to-end; permanent delete only on archives behind type-name confirmation | 01 (archive mechanics + location), **04** (backend), **06** (UI) | +| 4 | Running-agent guard enforced for destructive profile actions | **04** (backend refusal, authoritative), **06** (UI disable + reason). Per DD-4. | +| 5 | Honest `auth-unknown` state documented with link to the Hermes probe ask (spike 0010) | **02** (advisory field), **05** (display), **07** (docs) | +| 6 | Contract tests + fault-injection + Playwright evidence on the PR; HERMES.md / STATE.md / DECISIONS.md updated in-PR | RED gate in **02-06** (DD-7); **07** consolidates fault-injection, Playwright, and docs. STATE.md per-PR in 02-07 (DD-8). | + +## Phases + +| # | Phase | Effort | Depends on | +| - | ----- | ------ | ---------- | +| 01 | [Spike 0015 — readiness signals and archive mechanics](phase-01-spike-readiness-and-archive.md) | S (0.5-1 d) | — | +| 02 | [Readiness model, evaluator, and projection](phase-02-readiness-model-backend.md) | M (2-3 d) | 01 | +| 03 | [Spawn preflight and attention routing](phase-03-preflight-attention-routing.md) | M (2 d) | 02 | +| 04 | [Archive, restore, permanent delete, running-agent guard](phase-04-archive-restore-backend.md) | L (3-4 d) | 01 | +| 05 | [Readiness surfacing — card and dialog](phase-05-readiness-surfacing-ui.md) | M (2 d) | 02 | +| 06 | [Offboarding archive, restore, and permanent-delete UI](phase-06-offboarding-archive-ui.md) | L (3 d) | 04, 05 | +| 07 | [Verification, fault injection, and docs](phase-07-verification-and-docs.md) | M (2 d) | 03, 06 | + +Phases 03/05 and 04 are independent after 01/02 and may run in parallel +if file ownership is respected (03 owns `runtime.rs`; 05 owns the UI +files; 04 owns the Rust command layer). + +Delivery order note: 02 → 05 ships the readiness half end-to-end before +04 → 06 ships the archive half; each half is independently shippable and +each PR leaves `main` green. + +## Open questions + +| ID | Question | Blocks | Default if unanswered | +| -- | -------- | ------ | --------------------- | +| OQ-1 | Where do archives live? No "NuncioCrew Backups" area exists (DD-5). Under the app data dir, or a user-visible `~/Documents`-adjacent path? | 04, 06 | Phase 01 proposes app-data-dir with restricted permissions per S18 and records the choice; not decided by fiat here | +| OQ-2 | Archive format: `tar.gz` (new dependency) vs plain directory copy (no dependency, larger, simpler restore)? | 04 | Phase 01 measures a real profile; default to whichever meets "archives stay small" without adding a crate | +| OQ-3 | Which `config.yaml` fields make a profile `broken-config`? Unparsable YAML is unambiguous; "missing required model fields" needs the exact key set Hermes v0.20.0 requires. | 02 | Phase 01 reads a healthy profile and enumerates; conservative default is unparsable-only, so a false `broken-config` never blocks a working agent | +| OQ-4 | Does the mid-turn re-evaluation trigger on turn boundaries, on a filesystem watch, or on a timer? | 03 | Phase 01 evaluates cost; turn-boundary is the cheap default | +| OQ-5 | Is archive semantics a DECISIONS.md entry? The issue recommends yes, brief. | 07 | Yes — draft **D-028** in phase 07 (D-027 is the highest existing entry) | + +None of these block starting phase 01; all are resolved by the end of +phase 01 except OQ-5. + +## Risks + +| Risk | Mitigation | +| ---- | ---------- | +| `auth-unknown` displayed as a warning trains the owner to ignore badges | Display as neutral/informational, never as an error; copy links the spike-0010 ask so it reads as a known limit, not a fault | +| Binary probe (`hermes --version`) on every readiness read costs latency | Cache with a short TTL and invalidate on install events (S17 already re-evaluates post-install) | +| Config parse produces false `broken-config`, blocking a working agent | OQ-3 conservative default; the state must be recoverable from the nudge without CLI | +| Archive silently excludes something the owner needed | Manifest records the exclusion list applied; phase 01 fixes the list from a real profile, not a guess | +| Restore clobbers a live profile | Collision check refuses; name validation + `default` reject via S16 on every path | +| Upstream sync conflicts on the 12 touched files | Every edit is a hook or mirror; conflict policy in UPSTREAM-SYNC.md § Conflict policy applies — reapply the smallest hook | + +## Validation and red-team + +### Validate — **pass** + +| Check | Result | +| ----- | ------ | +| Every DoD checkbox maps to ≥1 phase | Pass — 6/6, table above | +| Every phase has frontmatter (`phase`, `title`, `status`, `priority`, `effort`, `dependencies`) | Pass | +| Dependencies acyclic and satisfiable | Pass — 01 → {02, 04}; 02 → {03, 05}; {04, 05} → 06; {03, 06} → 07 | +| Every feature names an existing Buzz seam | Pass — S1-S18, all with `path:line` | +| Upstream edits justified with expected diff size | Pass — 12 files, § Thin-fork budget | +| D-020 honored (PRs to `Nuncio-hq/crew`) | Pass — stated in every phase's PR step | +| D-025 generic-ACP check performed | Pass — DD-6 table, Hermes-specific parts labelled | +| D-008 spike → RED → implement | Pass — phase 01 is the spike; RED gate in 02-06 per DD-7 | +| Non-goals not silently violated | Pass — no auth probe, no scheduled backups, no rename detection, no create/occupancy/owner-only changes | +| Issue requirements dropped | None. One mechanism deviation (DD-2) recorded with rationale, not dropped | +| Plan contains no implementation | Pass — planning artifacts only | + +### Red-team — 9 findings, 8 applied, 1 rejected + +| # | Finding | Disposition | +| - | ------- | ----------- | +| R1 | Modelling `auth-unknown` as a `Requirement` would put every healthy Hermes agent into setup-listener mode — a total outage of the Hermes runtime | **Applied** → DD-1 two-channel split | +| R2 | The issue's stated carrier (`AcpRuntimeCatalogEntry` / `agentConfigCore.ts`) contradicts `features/agents/AGENTS.md` rule 1; following it literally builds the rival table the same constraint forbids | **Applied** → DD-2 deviation with rationale and preserved intent | +| R3 | The issue asserts graceful stop machinery (SIGTERM → wait → SIGKILL) that does not exist; a plan assuming it would ship a guard that cannot honor its own copy | **Applied** → DD-4, verified at `runtime/stop.rs:40,120,153` and `discovery.rs:927` | +| R4 | The "existing backups area" does not exist anywhere in the repo; hardcoding the path would create a phantom dependency | **Applied** → DD-5 + OQ-1, resolved in phase 01 | +| R5 | A RED-tests-only phase cannot merge without breaking `main` green | **Applied** → DD-7, RED as in-phase gate | +| R6 | Treating STATE.md as phase 07's job violates #117 for phases 02-06 | **Applied** → DD-8, per-PR obligation | +| R7 | A too-eager `broken-config` heuristic is worse than the boolean it replaces — it blocks working agents on a guess | **Applied** → OQ-3 conservative default + recoverable-from-nudge requirement | +| R8 | Archive without a running-agent guard on the *backend* lets a Playwright-invisible path corrupt a live profile | **Applied** → guard is authoritative in phase 04 (backend), UI disable in 06 is advisory only | +| R9 | Phase 04 archive work should block on phase 02 so both halves share one readiness read | **Rejected.** Archive operates on directories and runtime-pair liveness, not on readiness state; coupling them serializes two independent halves for no shared contract and delays the first shippable slice. Recorded rather than applied. | + +### Deviations from the issue (explicit, none silent) + +1. **DD-2** — readiness rides the per-agent snapshot + `Requirement` + pipeline rather than `AcpRuntimeCatalogEntry` / `agentConfigCore.ts`. + Intent preserved; mechanism corrected against + `features/agents/AGENTS.md` rule 1. +2. **DD-1** — `auth-unknown` is advisory, not a `Requirement`. All five + names still reach the display layer. +3. **DD-4** — the running-agent guard refuses while running rather than + stopping the agent for the owner, because the graceful-stop machinery + the issue cites does not exist. + +## Constraints in force for every phase + +- **D-020:** every PR targets `Nuncio-hq/crew`. Never `block/buzz`, even + for upstream-owned files. Branch from Crew `main`, merge through + `NuncioCrew Gate`. +- Branch names describe product areas, not phase numbers + (`UPSTREAM-SYNC.md` § Feature branches) — e.g. + `agents/profile-readiness`, `agents/profile-archive`. +- `just ci` green before every PR; `git commit -s` on every commit (DCO). +- No `unsafe`; no new `unwrap()`/`expect()` in production paths. +- Desktop text sizing: rem tokens only, never px (`AGENTS.md`). +- Screenshots via `scripts/post-screenshots.sh`; distinct-state + `shasum -a 256` gate before posting.