From f9fe6d5a5909c4ce00c9e8241c34bab4dde4cc12 Mon Sep 17 00:00:00 2001 From: Douglas Jarquin <8209+douglasjarquin@users.noreply.github.com> Date: Sat, 17 Oct 2026 01:58:35 +0000 Subject: [PATCH 1/5] feat: track incidental finding dispositions --- docs/architecture.md | 6 +- docs/features/coordination.md | 1 + ...-feat-incidental-finding-follow-up-plan.md | 229 ++++++++ go/internal/backup/backup.go | 23 + go/internal/backup/backup_test.go | 92 ++++ go/internal/cli/findings.go | 159 ++++++ go/internal/cli/findings_test.go | 111 ++++ go/internal/cli/metadata.go | 11 + go/internal/cli/native_test.go | 2 +- go/internal/cli/root.go | 1 + ...sections_decisions_filtered_to_open.stdout | 8 +- ...ions_decisions_filtered_to_answered.stdout | 8 +- ...vidence_notes_or_environment_record.stdout | 2 +- ...ons_notes_and_an_environment_record.stdout | 2 +- ...off_and_a_passing_root_verification.stdout | 2 +- go/internal/cli/testdata/help-unknown.err | 2 +- go/internal/cli/testdata/help.stdout | 5 +- go/internal/contextview/contextview.go | 31 +- go/internal/evidenceview/evidenceview.go | 23 + go/internal/helpview/catalog.json | 24 +- go/internal/notes/findings.go | 511 ++++++++++++++++++ go/internal/notes/findings_test.go | 308 +++++++++++ 22 files changed, 1543 insertions(+), 18 deletions(-) create mode 100644 docs/plans/2026-10-17-0132-feat-incidental-finding-follow-up-plan.md create mode 100644 go/internal/backup/backup_test.go create mode 100644 go/internal/cli/findings.go create mode 100644 go/internal/cli/findings_test.go create mode 100644 go/internal/notes/findings.go create mode 100644 go/internal/notes/findings_test.go diff --git a/docs/architecture.md b/docs/architecture.md index d5c1269d..90934d47 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -49,7 +49,7 @@ The helper is called `sumctl` to avoid shadowing the Unix `sum` command. Normall ## State and communication -Private state lives in `.sum/` and is ignored by Git. `.sum/context.json` records the coordinator owner and `.sum/sessions/` the registered panes and roles. Optional `.sum/preferences.md` and `.sum/projects.md` hold local preferences and project notes; `.sum/projects.json` is the registry of enrolled projects (host/owner/repo, verified remote, canonical path, enrollment metadata), while their clones live outside the state under the Git-ignored `projects/` directory. `.sum/coordinator/` holds the coordinator's contract revisions and refresh receipts. `.sum/machine.json` maps each host's stable identity to the hostnames it was seen using, so records written before that identity existed keep resolving; it is not part of a backup. Each task stores its brief, base SHA, branch/worktree, pane bindings, questions, answers, report, an append-only `evidence` list (reports, structured handoffs, reviewer findings, coordinator verification, exact PR observations), the bound reviewer endpoint, the last reconciled PR identity, a versions sidecar with brief revisions, an optional environment sidecar with the task-local environment record, and an optional usage sidecar (`usage.json`, counts and coverage from the Claude Code transcripts of the task's bound panes, written only by `usage collect`, `report`, and `cleanup` when `settings.json` enables usage accounting). File updates are locked and atomically replaced on one local machine. +Private state lives in `.sum/` and is ignored by Git. `.sum/context.json` records the coordinator owner and `.sum/sessions/` the registered panes and roles. Optional `.sum/preferences.md` and `.sum/projects.md` hold local preferences and project notes; `.sum/projects.json` is the registry of enrolled projects (host/owner/repo, verified remote, canonical path, enrollment metadata), while their clones live outside the state under the Git-ignored `projects/` directory. `.sum/coordinator/` holds the coordinator's contract revisions and refresh receipts. `.sum/findings/` holds optional findings under stable IDs derived from source task and key, including append-only disposition and linked-work history. `.sum/machine.json` maps each host's stable identity to the hostnames it was seen using, so records written before that identity existed keep resolving; it is not part of a backup. Each task stores its brief, base SHA, branch/worktree, pane bindings, questions, answers, report, an append-only `evidence` list (reports, structured handoffs, reviewer findings, coordinator verification, exact PR observations), the bound reviewer endpoint, the last reconciled PR identity, a versions sidecar with brief revisions, an optional environment sidecar with the task-local environment record, and an optional usage sidecar (`usage.json`, counts and coverage from the Claude Code transcripts of the task's bound panes, written only by `usage collect`, `report`, and `cleanup` when `settings.json` enables usage accounting). File updates are locked and atomically replaced on one local machine. Workers use the exact commands in their generated brief. The core interaction is: @@ -68,10 +68,12 @@ Workers use the exact commands in their generated brief. The core interaction is ./bin/sumctl context TASK_ID --section brief --revision r1 # a recorded brief revision, hash-verified, never rewritten ./bin/sumctl context TASK_ID --since CURSOR # what changed since the `cursor` of an earlier read ./bin/sumctl notes TASK_ID --text 'finding' # the one optional task-local notes.md (a claim; secrets refused) +./bin/sumctl finding record TASK_ID --key KEY --title TITLE --explanation TEXT --next-step TEXT +./bin/sumctl finding list TASK_ID ./bin/sumctl help [TOPIC] # command discovery without the full manual ``` -Sections: `outline`, `brief`, `decisions`, `handoff`, `evidence`, `execution`, `environment`, `update`, `returns`, `notes`. +Sections: `outline`, `brief`, `decisions`, `handoff`, `evidence`, `execution`, `environment`, `update`, `returns`, `notes`, `findings`. Prose fields are bounded (`--max-chars`, 0 for all) and credential-shaped text is redacted in the view, never in the record. Outstanding decisions are listed in full on every read regardless of paging. Worker prose, handoffs, and notes are labelled claims; only `verify` and `pr reconcile` records are verification evidence. Skills reach agents as explicit file references (path, size, hash) to read when needed, not as an assumed skill standard. Nothing here calls a model or reads a worker's checkout. diff --git a/docs/features/coordination.md b/docs/features/coordination.md index 2d106f61..62ad271c 100644 --- a/docs/features/coordination.md +++ b/docs/features/coordination.md @@ -4,6 +4,7 @@ Journeys the coordinator, workers, and developers take through `sumctl` and the | ID | Scenario | Driver | Evidence | | --- | --- | --- | --- | +| `finding.dispositions` | A registered worker or coordinator records an optional finding without creating follow-up work; a repeated task/key capture returns the same record and changed content is refused. The coordinator can defer, dismiss, reopen, resolve, attach to existing work, promote to an existing approved follow-up task, and settle linked work; history survives archival of the source, and backup, task context, and show preserve the ledger | automated: `go/internal/notes/findings_test.go`, `go/internal/cli/findings_test.go`, `go/internal/backup/backup_test.go` | offline suite | | `roles.claim` | The first pane claims coordinator atomically; later panes become developers or workers | automated: `go/internal/cli/cutover_test.go`, `go/internal/cli/demo_test.go` | offline suite | | `roles.machine-identity` | Renaming the host keeps the coordinator role and its return routes without `--reclaim`, because every recorded endpoint's `machine` is a stable identity hashed from the OS machine ID (`/etc/machine-id`, or the macOS platform UUID), never the raw ID or the hostname. A different host stays other-machine even when it shares the hostname: its coordinator claim is refused and reclaim fails as other-machine, and so are a VM clone that regenerated its machine ID with `.sum/` copied, and the tasks of a records backup restored there | automated: `go/internal/cli/machine_identity_test.go`, `go/internal/machine/machine_test.go` | offline suite. The tests pin the machine ID and hostname in process, and each scenario fails when the identity is the raw hostname. Implementation: `go/internal/machine/machine.go`, `go/internal/store/context.go`, `go/internal/store/store.go`, `go/internal/roleinit/designated.go`, `go/internal/app/app.go`, `go/internal/bindcmd/bind.go`. | | `roles.legacy-hostname-records` | Records written by the hostname-keyed release keep resolving with no manual step. A hostname value names this host when it is the current hostname or was recorded under this host's identity in `.sum/machine.json`, which each registration updates. That covers a mix of old names once the host has carried each one. The coordinator's record and session files keyed by an old hostname are re-keyed on the next `init`, and a delivery recorded under the old key still counts, so it is not sent again. A name recorded under another host's identity never resolves here, and neither does one this host has not been seen carrying | automated: `go/internal/cli/machine_identity_test.go`, `go/internal/store/registration_test.go`, `go/internal/machine/machine_test.go` | offline suite. A pre-upgrade hostname record in a backup restored on a different host that shares that hostname still resolves there, as it did before. A name the host carried only under the older release resolves after one `init` while the host carries it again (the recovery step in `docs/recovery.md`). Implementation: `go/internal/machine/machine.go`, `go/internal/store/store.go`, `go/internal/roleinit/designated.go`, `go/internal/returns/returns.go`, `go/internal/returns/pump.go`. | diff --git a/docs/plans/2026-10-17-0132-feat-incidental-finding-follow-up-plan.md b/docs/plans/2026-10-17-0132-feat-incidental-finding-follow-up-plan.md new file mode 100644 index 00000000..711d4dbe --- /dev/null +++ b/docs/plans/2026-10-17-0132-feat-incidental-finding-follow-up-plan.md @@ -0,0 +1,229 @@ +--- +title: Incidental Finding Follow-up - Plan +date: 2026-10-17 +type: feat +artifact_contract: ce-unified-plan/v1 +product_contract_source: ce-plan-bootstrap +execution: code +--- + +# Incidental Finding Follow-up - Plan + +## Goal Capsule + +- **Objective:** Agents and coordinators can retain useful out-of-scope discoveries, revisit them, and record an explicit outcome without changing the approval or delivery state of the originating task. +- **Means:** Add a durable finding ledger with role-checked capture and disposition actions and read-only task/project views. +- **Authority:** The approved task brief, existing task dispatch and repair authorization, and current review and verification gates remain authoritative. +- **Execution profile:** Implement against the task's current `main` baseline and sequence any shared inbox action integration after #305 provides its action descriptions. +- **Stop conditions:** Stop if a finding action would need to dispatch work without existing authorization or if a change would alter the meaning of review, PR identity, notes, or verification records. +- **Completion owner:** The coordinator performs independent verification and review and opens the PR. The user merges. + +--- + +## Product Contract + +### Summary + +Add explicit, durable incidental findings associated with their originating task and project. Agents can capture a finding and continue approved work. The finding remains visible until an authorized person records a disposition or linked work settles. + +### Problem Frame + +Sum retains observations in notes, reports, handoffs, and reviewer records, but these records do not provide an explicit follow-up lifecycle for useful discoveries outside the approved brief. Treating those discoveries as blocking questions or review findings would change their authority and would interrupt the original task. + +### Requirements + +**Capture and identity** + +- R1. A role-appropriate explicit capture action records a stable key, concise title and explanation, proposed next step, source/evidence references, originating task and project, author and time, and attempt/candidate identity when known. +- R2. Capture is idempotent for the same task and key with identical content. The same key with conflicting content is refused without changing the original record. Text similarity never merges findings. +- R3. Capture alone creates no task, blocking question, approval, scope expansion, review result, or verification evidence. + +**Disposition and visibility** + +- R4. A finding records append-only disposition and link events with actor, time, reason or outcome, and linked work. Dispositions include open, defer, dismiss, explicit resolution, and reopen. Link events distinguish attaching existing work from handing off to approved follow-up work. +- R5. Deferred and other unresolved findings remain discoverable. Dismissal preserves history; age, compaction, list limits, and source task archival do not erase or hide unresolved findings. +- R6. Saved inbox, task context, and show views expose source, age, proposed next step, and disposition. Views are read-only and keep optional findings separate from blocking Needs You decisions. +- R7. Where the shared inbox action descriptions are available, a finding exposes follow-up, attach, defer, dismiss, and explicit resolution actions with their source and state identity. Follow-up routes through the existing task creation and approval flow; after approval, the coordinator links the resulting task to the finding. Opening or applying a finding action does not itself create work. + +**Authorization and compatibility** + +- R8. Promotion or attachment links only to newly approved or suitable existing work, or to a scoped repair, and uses existing authorization. Repeated promotion is idempotent. A linked handoff remains outstanding until the linked work settles or the finding receives an explicit disposition. +- R9. Existing brief boundaries remain authoritative. Optional findings do not block completion of the originating task; defects or missing verification needed by that task stay in its report and delivery gates. +- R10. Existing reviewer findings, PR identity findings, limitations, and notes retain their current semantics. Dismissing an incidental finding cannot change a review verdict or waive evidence. Legacy notes remain readable without extraction or migration. + +### Scope Boundaries + +- **In scope:** explicit structured capture, deterministic identity, durable history, authorized linking, explicit dispositions, read-only saved visibility, and the documentation needed to use those paths. +- **Deferred to follow-up work:** automatic discovery or classification of findings from notes, reports, or model output. No automatic extraction is requested. +- **Out of scope:** fuzzy deduplication, automatic expiry, an independent issue tracker, a new approval system, implicit task creation, or a model call for each turn. + +--- + +## Planning Contract + +### Key Technical Decisions + +- KTD1. Keep incidental findings in a dedicated authoritative ledger separate from task notes, review findings, PR identity observations, and blocking questions. Read views project the ledger rather than duplicating disposition state. +- KTD2. Scope the caller's exact non-empty stable key to its originating task and derive record identity as the full SHA-256 of a canonical JSON pair containing task ID and key. Compare canonical immutable capture fields, excluding capture actor/time and disposition history, for duplicate submissions. This follows Sum's task-local source/key conventions and avoids merging reports from separate tasks or by wording. +- KTD3. Keep each finding as one file under the state home `findings/` directory so source archival cannot remove it. Include every record in Sum's records-only backup because backup explicitly enumerates persisted state. Use Sum's state lock, atomic write, regular-file, credential handling, and bounded-read conventions. +- KTD4. Make disposition and promotion explicit role-authorized commands. Creating follow-up work passes through the existing task creation and approval flow; linking the approved result passes through the coordinator's finding authorization. Scoped repairs use existing repair authorization. Capture and inbox action reads never create or dispatch work. +- KTD5. Permit capture only to the registered worker assigned to the source task or its registered coordinator; reviewers and unrelated endpoints use existing review or report paths. Restrict changes to disposition and links to the registered coordinator, who owns human approval and repair authorization. +- KTD6. Record linked-work settlement as a coordinator-authored append-only resolution event after the linked task's result or repair outcome is reviewed. Do not infer resolution from a passive task status change. +- KTD7. Track disposition and linked-work status as separate fields. Allow one active linked-work target per finding. The first authorized link wins; a repeat to the same target is idempotent, while a different target requires an explicit reopen after the prior link is settled or explicitly dismissed. Defer, dismiss, and resolve may be recorded while work is linked and never erase its history. Reopening sets the disposition to open; an unsettled link remains outstanding, while a settled link is removed from the active target field and remains in history. +- KTD8. Add inbox action projection through #305's shared action description after that foundation is available. Give each action the finding ID and current disposition revision as source/state identity, and re-read the canonical record before applying it. Coordinate the additive adapter with #306, which consumes the same shared action surface. + +### High-Level Technical Design + +The interaction has two paths: a role-checked write path updates one authoritative ledger, and saved views read and project that ledger. Existing task approval, review, and verification systems remain independent. + +```mermaid +sequenceDiagram + participant Agent + participant CLI + participant Findings + participant Ledger + participant Views + Agent->>CLI: capture with task, key, content, and source + CLI->>Findings: validate role and task identity + Findings->>Ledger: locked deterministic create or conflict check + Ledger-->>Findings: saved record or unchanged duplicate + Findings-->>CLI: finding ID and disposition + Views->>Ledger: read saved findings by task/project + Ledger-->>Views: authoritative records and history + Views-->>Agent: bounded context, show, and inbox projection +``` + +```mermaid +stateDiagram-v2 + [*] --> Open + Open --> Deferred: defer with reason + Deferred --> Open: reopen + Open --> Dismissed: dismiss with reason + Deferred --> Dismissed: dismiss with reason + Dismissed --> Open: reopen + Open --> Resolved: record explicit outcome + Deferred --> Resolved: record explicit outcome + Resolved --> Open: reopen +``` + +Link status is tracked separately as absent, attached, handoff outstanding, or settled. Linking suitable existing work sets `attached`; linking an approved follow-up sets `handoff outstanding`. A coordinator records linked-work settlement after inspecting its result. Deferring preserves the link; dismissal or explicit resolution settles it without erasing its history. Reopening sets the disposition to open while an unsettled link remains outstanding; reopening after settlement clears the active target but preserves the prior target and its disposition event in history. A different target requires that explicit reopen. The stored history is authoritative. Read models may summarize it but cannot acknowledge or mutate findings. The command that links follow-up work records the existing approved task identity; it does not create a task as a side effect. + +### Assumptions and Dependencies + +- The base branch is Sum `main` at `41ba86f`, and its existing task and project identity rules remain the source for attribution. +- Issue #305 owns the shared action description in the presentation/inbox layer, which #306 also consumes. The finding ledger, capture, and disposition operations can be built without that change; action projection is sequenced after the shared type is available and coordinated across both lanes. +- The local Sum state directory is the durable home for the finding ledger. Existing backup behavior enumerates its records, so the finding ledger must be added explicitly to that list and its manifest. + +### System-Wide Impact + +The worker needs to record a finding without turning it into a question or changing task scope. The coordinator needs a saved view that distinguishes outstanding optional follow-up from work requiring attention. Reviewers and delivery gates must continue to see their existing verdict and evidence records unchanged. Project grouping must preserve host-qualified project identity and readable records whose source task is archived. + +### Risks and Dependencies + +- The schema, context, inbox, CLI, and skill documentation have neighboring work in #305 and #306. Keep those changes additive and record each shared file in the handoff for merge sequencing. +- A global ledger must report unreadable or malformed records as incomplete visibility rather than silently presenting an empty inbox. +- The task explicitly requires state-history behavior. Reopening is a new history entry, not deletion or rewriting of prior dismissals or resolutions. +- A linked task's status alone does not explain the outcome that resolves a finding. A coordinator records the settlement outcome explicitly after inspecting the linked work's report or repair result. + +### Sources + +- Issue #307, `Track incidental findings through explicit follow-up dispositions`. +- `go/internal/notes/notes.go` for task-local note attribution, safe writes, redaction, and bounded reads. +- `go/internal/ask/ask.go` for keyed durable state and coordinator authorization patterns. +- `go/internal/contextview/contextview.go` for role-aware read-only task context. +- `go/internal/inboxview/snapshot.go` and `go/internal/inboxview/overview.go` for saved source aggregation, project identity, gap reporting, and read-only routing. +- `go/internal/project/project.go` for canonical project identity. +- `docs/solutions/architecture-patterns/retiring-persisted-compatibility-fields.md` for single-authority persistence and read-boundary projection. +- Inbox's [record-and-continue guidance](https://github.com/petekp/inbox/blob/9bc2983367827bb6b49a7d14cfdff1f48a7f0626/hooks/model-text.ts#L65-L79) and [finding schema](https://github.com/petekp/inbox/blob/9bc2983367827bb6b49a7d14cfdff1f48a7f0626/hooks/tools.ts#L8-L22) as prior art. Sum's explicit key and no-expiry requirements govern this implementation. + +--- + +## Implementation Units + +### U1. Durable finding ledger and disposition history + +- **Goal:** Store one deterministic finding record per originating task/key and preserve every disposition transition independently of source task retention. +- **Requirements:** R1, R2, R4, R5, R10. +- **Dependencies:** None. +- **Files:** `go/internal/notes/` finding ledger implementation and focused tests; `go/internal/store/store.go` only if the existing home lock/path API needs a narrow addition; `go/internal/backup/backup.go` and backup tests. +- **Approach:** Add a separate record type and one JSON file per finding under the state home `findings/` directory. Derive its filename from the task/key SHA-256 pair. Compare immutable capture content for duplicate submissions, excluding author/time and disposition history. Append disposition events with actor, time, reason/outcome, and linked task. Reject conflicting keys and second targets. Report malformed records as gaps. Refuse symlinks and credential-shaped content. +- **Patterns to follow:** `notes.Add` for locking, atomic mode-restricted writes, credential refusal, and attribution; `ask` for stable keyed idempotency and no-silent-replacement; compatibility learning for one authority with read-boundary projections. +- **Test scenarios:** + - Capturing a new task/key produces a stable ID and stores next step, source references, optional attempt/candidate, and task/project attribution. + - Repeating identical content under the same task/key returns the existing ID without adding a record. + - Concurrent identical submissions produce one record, while conflicting title, explanation, or references are refused without replacing it. + - Defer, dismiss, reopen, and resolve append actor/time/reason/outcome history and preserve earlier events. + - A finding remains readable after its originating task is archived, and legacy notes remain byte-for-byte readable and unextracted. + - A records-only backup includes the findings ledger and identifies it in the manifest. + - Restoring that backup into a new state home retains each finding, its disposition history, and visibility for an archived source task. + - Symlinked, malformed, and credential-shaped inputs fail without modifying existing records. +- **Verification:** The ledger has one authoritative record per task/key, preserves history through every disposition, and exposes errors instead of silently losing visibility. + +### U2. Role-authorized capture, disposition, and promotion commands + +- **Goal:** Give agents and coordinators explicit CLI paths to capture and dispose findings while preserving existing authorization boundaries. +- **Requirements:** R1, R3, R4, R8, R9. +- **Dependencies:** U1. +- **Files:** `go/internal/cli/native.go`, `go/internal/cli/root.go`, new finding command files and tests under `go/internal/cli/`, `go/internal/helpview/`, relevant help snapshots, `skills/sum-work/SKILL.md`, `skills/sum-status/SKILL.md`, and `docs/repairs.md` if repair linking requires clarification. +- **Approach:** Expose explicit record, defer, dismiss, reopen, resolve, attach, and promote operations. Permit capture from the registered worker of the source task or its coordinator; reserve disposition, settlement, and linking for the registered coordinator. Derive role from the endpoint rather than accepting a caller-declared role. Route follow-up creation through the existing task creation and approval flow, then link its approved task; attaching requires suitable existing work. Scoped repairs use existing repair authorization. The first linked target wins; repeat links to that target are idempotent and a different target is refused until the finding is reopened with the prior link disposition recorded. +- **Test scenarios:** + - An authorized worker records a finding during an approved task without creating a task or question. + - An unrelated endpoint or forged role string cannot record or change a finding. + - An authorized coordinator links an existing approved task; repeating the same promotion returns the original link. + - Concurrent links to the same target create one history entry; concurrent links to different targets preserve the first and refuse the second. + - A different task, stale target, or unapproved target is rejected without a dispatch or repair side effect. + - A linked handoff remains outstanding until the coordinator records its inspected task or repair outcome; a task status change alone does not settle it. + - A defect or required verification gap remains in the original report and delivery result regardless of finding dismissal. +- **Verification:** CLI writes use existing identity and authorization checks; no finding command creates work without a separately approved task or repair. + +### U3. Read-only context, show, and inbox visibility + +- **Goal:** Make findings discoverable from task and project views while keeping optional follow-up distinct from blocking decisions. +- **Requirements:** R4, R5, R6, R7, R10. +- **Dependencies:** U1. Inbox action descriptions from #305 before action projection; coordinate the shared adapter with #306. +- **Files:** `go/internal/contextview/contextview.go` and tests; `go/internal/cli/context.go` and tests; `go/internal/inboxview/snapshot.go`, `overview.go`, `terminal.go`, and tests; `go/internal/presentation/presentation.go` and tests; metadata projection files if required. +- **Approach:** Add a read-only findings section and source routes. Project source, age, proposed next step, disposition, and linked work. Group by canonical project identity, include archived-source findings, and provide pagination and a detail route. Report malformed ledger records as visibility gaps. Add finding action descriptions for follow-up, attach, defer, dismiss, and resolve through #305's shared action type, with finding ID and disposition revision as source/state identity; validate identity and state against the canonical ledger on execution. The follow-up action routes to existing task creation and approval; the coordinator links the approved task afterward. Keep inbox changes additive with #306 and do not count findings as decisions or inspections. +- **Test scenarios:** + - Task context and show expose the finding's source, age, next step, disposition, and history without mutating it. + - Inbox groups active findings by project, includes archived origins, and keeps them out of Needs You decision counts. + - Paging exposes a discoverable continuation and counts reflect all readable records; unreadable entries make the view incomplete. + - Refreshing or opening any read view leaves the finding ledger unchanged. + - An action created from an older disposition revision is refused after the finding has changed. + - Existing review verdicts, PR identity items, limitations, and verification obligations remain unchanged when a finding is dismissed. +- **Verification:** Context, show, metadata, and inbox are read-only projections of one ledger and keep findings separate from blocking and review records. + +### U4. Feature guidance and verification map + +- **Goal:** Document when to capture incidental findings, how to choose a disposition, and which checks cover the behavior. +- **Requirements:** R1-R10. +- **Dependencies:** U2, U3. +- **Files:** `skills/sum-work/SKILL.md`, `skills/sum-status/SKILL.md`, `docs/repairs.md` if needed, and the relevant row in `docs/features/README.md` or its owning feature map. +- **Approach:** Explain that findings are optional records, do not authorize work, and do not replace task reports for in-scope defects or required verification. Describe each disposition and the existing authorization needed to promote it. Preserve all existing review, note, and evidence language. +- **Test scenarios:** + - The documented worker path names the explicit capture command and states that recording does not create or block work. + - The status path explains how to find deferred findings and how linked handoffs settle. + - The feature map links the behavior to the focused CLI and package coverage and names any manual scenario as not-run until captured. +- **Verification:** Guidance, CLI help, and the feature map agree with the shipped command and disposition semantics. + +--- + +## Verification Contract + +- Use focused existing Go package and CLI coverage for the scenarios in U1-U4. The issue explicitly requests focused behavioral coverage. +- Run the repository's standardized verification runner from a clean committed candidate: `python3 .agents/skills/verify/scripts/verify_run.py --base 460cb52400706f8a42636b630d32b3f095cb256e --json`. +- Capture nonvisual CLI before/after evidence for the changed user path using the evidence skill. The transcript must say `visual_proof: not-applicable`; do not create a browser capture for this CLI feature. +- The coordinator runs an independent verification and review against the candidate SHA. The worker does not open or merge the PR. + +--- + +## Definition of Done + +- A stable task/key produces one authoritative finding record and conflicting content cannot replace it. +- A records-only backup contains the ledger and identifies it in its manifest. +- Restoring the records-only backup preserves findings, disposition history, and archived-source visibility. +- Disposition history includes actor, time, reason/outcome, and linked work; defer, dismiss, reopen, resolution, and outstanding handoff states are visible. +- Authorized promotion links approved work idempotently and never creates a task or repair from capture alone. +- Saved inbox, context, and show views display the required details, preserve archived-source findings, and leave decisions, reviews, notes, PR identity, and verification gates unchanged. +- Focused coverage and the required verification runner complete on the committed candidate, and the handoff lists the comparison artifact and all files shared with #305 or #306. +- Documentation describes the explicit workflow and confirms that optional findings do not block the originating task. +- The final diff contains no abandoned exploratory code or unrequested work. diff --git a/go/internal/backup/backup.go b/go/internal/backup/backup.go index 1bd70535..514a5b3f 100644 --- a/go/internal/backup/backup.go +++ b/go/internal/backup/backup.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "sort" + "strings" "github.com/douglasjarquin/sum/go/internal/contract" "github.com/douglasjarquin/sum/go/internal/environment" @@ -165,6 +166,23 @@ func Run(s *store.Store, destination string) (*ordjson.Object, error) { manifest.Set("environment_records_included", true) manifest.Set("environment_exclusions", "command references and URLs redacted at write; no process environments, credentials, log content, or checkout code") manifest.Set("settings_included", isFile(filepath.Join(home, settings.File))) + findingDir := notes.FindingsPath(s) + findingRecords, findingGaps := notes.ReadAll(s) + manifest.Set("finding_records_included", jsonInt(len(findingRecords))) + gapRows := make([]any, 0, len(findingGaps)) + for _, gap := range findingGaps { + row := ordjson.NewObject() + relative, err := filepath.Rel(findingDir, gap.Path) + if err != nil { + relative = filepath.Base(gap.Path) + } + row.Set("path", relative) + row.Set("error", strings.ReplaceAll(gap.Error, findingDir, "findings")) + gapRows = append(gapRows, row) + } + manifest.Set("finding_visibility_gaps", gapRows) + findingEntries, _ := filepath.Glob(filepath.Join(findingDir, "*.json")) + sort.Strings(findingEntries) manifest.Set("graph", graphObj) manifest.Set("managed_projects", projectsObj) manifest.Set("restore", "Extract into a new directory. Start sumctl with --home /state. Do not reuse pane bindings on another machine; inspect and bind explicitly.") @@ -202,6 +220,11 @@ func Run(s *store.Store, destination string) (*ordjson.Object, error) { for _, name := range []string{"state.json", settings.File, project.ProjectsFile, "preferences.md", "projects.md"} { paths = append(paths, filepath.Join(home, name)) } + for _, path := range findingEntries { + if isFile(path) && !isSymlink(path) { + paths = append(paths, path) + } + } sessionEntries, _ := filepath.Glob(filepath.Join(s.Sessions, "*.json")) sort.Strings(sessionEntries) paths = append(paths, sessionEntries...) diff --git a/go/internal/backup/backup_test.go b/go/internal/backup/backup_test.go new file mode 100644 index 00000000..000ee4f5 --- /dev/null +++ b/go/internal/backup/backup_test.go @@ -0,0 +1,92 @@ +package backup + +import ( + "archive/tar" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "io" + "os" + "path/filepath" + "testing" + + "github.com/douglasjarquin/sum/go/internal/notes" + "github.com/douglasjarquin/sum/go/internal/ordjson" + "github.com/douglasjarquin/sum/go/internal/store" +) + +func TestRunIncludesFindingsInRecordsOnlyBackup(t *testing.T) { + home := t.TempDir() + s, err := store.Open(home) + if err != nil { + t.Fatal(err) + } + if err := s.Init(); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(notes.FindingsPath(s), 0o700); err != nil { + t.Fatal(err) + } + actor := ordjson.NewObject() + actor.Set("role", "worker") + identity, _ := json.Marshal(struct { + TaskID string `json:"task_id"` + Key string `json:"key"` + }{TaskID: "t-aaaaaaaaaaaa", Key: "key"}) + sum := sha256.Sum256(identity) + record := notes.Record{Schema: notes.Schema, ID: hex.EncodeToString(sum[:]), Key: "key", TaskID: "t-aaaaaaaaaaaa", ProjectID: "team/repo", Author: actor, CreatedAt: store.Now(), Title: "title", Explanation: "explanation", NextStep: "next", References: []string{}, Disposition: "open", LinkState: "none", History: []notes.Event{{Action: "capture", At: store.Now(), Actor: actor}}} + data, err := json.Marshal(record) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(notes.FindingsPath(s), record.ID+".json"), data, 0o600); err != nil { + t.Fatal(err) + } + if records, gaps := notes.ReadAll(s); len(records) != 1 || len(gaps) != 0 { + t.Fatalf("records=%#v gaps=%#v", records, gaps) + } + destination := filepath.Join(t.TempDir(), "records.tar.gz") + if _, err := Run(s, destination); err != nil { + t.Fatal(err) + } + file, err := os.Open(destination) + if err != nil { + t.Fatal(err) + } + defer file.Close() + gz, err := gzip.NewReader(file) + if err != nil { + t.Fatal(err) + } + tr := tar.NewReader(gz) + foundRecord := false + var manifest map[string]any + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatal(err) + } + content, err := io.ReadAll(tr) + if err != nil { + t.Fatal(err) + } + if header.Name == "state/findings/"+record.ID+".json" { + foundRecord = string(content) == string(data) + } + if header.Name == "manifest.json" { + if err := json.Unmarshal(content, &manifest); err != nil { + t.Fatal(err) + } + } + } + if !foundRecord { + t.Fatal("backup did not preserve the finding record") + } + if manifest["finding_records_included"] != float64(1) { + t.Fatalf("finding_records_included = %v, want 1", manifest["finding_records_included"]) + } +} diff --git a/go/internal/cli/findings.go b/go/internal/cli/findings.go new file mode 100644 index 00000000..9840d4b4 --- /dev/null +++ b/go/internal/cli/findings.go @@ -0,0 +1,159 @@ +package cli + +import ( + "fmt" + + "github.com/douglasjarquin/sum/go/internal/app" + findings "github.com/douglasjarquin/sum/go/internal/notes" + "github.com/douglasjarquin/sum/go/internal/ordjson" + "github.com/douglasjarquin/sum/go/internal/store" + "github.com/spf13/cobra" +) + +func (o *rootOptions) addFindingCommands(root *cobra.Command) { + cmd := &cobra.Command{ + Use: "finding", + Short: "Record and follow optional findings", + DisableSuggestions: true, + Args: cobra.NoArgs, + RunE: func(*cobra.Command, []string) error { + return fmt.Errorf("finding action is required") + }, + } + + var in findings.Input + record := &cobra.Command{ + Use: "record TASK", + Annotations: map[string]string{projectsAnnotation: "task"}, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + st, err := o.openStore("finding-record") + if err != nil { + return err + } + value, duplicate, err := findings.Capture(st, args[0], in, app.OptionalContext(o.installRoot)) + if err != nil { + return err + } + findingObject, err := ordjson.FromValue(value) + if err != nil { + return err + } + result := ordjson.NewObject() + result.Set("finding", findingObject) + result.Set("duplicate", duplicate) + return emitOrdjson(cmd.OutOrStdout(), result) + }, + } + record.Flags().StringVar(&in.Key, "key", "", "") + record.Flags().StringVar(&in.Title, "title", "", "") + record.Flags().StringVar(&in.Explanation, "explanation", "", "") + record.Flags().StringVar(&in.NextStep, "next-step", "", "") + record.Flags().StringArrayVar(&in.References, "reference", nil, "") + record.Flags().StringVar(&in.Attempt, "attempt", "", "") + record.Flags().StringVar(&in.Candidate, "candidate", "", "") + for _, name := range []string{"key", "title", "explanation", "next-step"} { + _ = record.MarkFlagRequired(name) + } + cmd.AddCommand(record) + + list := &cobra.Command{ + Use: "list TASK", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + st, err := store.Open(o.home) + if err != nil { + return err + } + records, gaps := findings.ReadTask(st, args[0]) + result := ordjson.NewObject() + result.Set("task", args[0]) + findingRows := make([]any, 0, len(records)) + for _, record := range records { + row, err := ordjson.FromValue(record) + if err != nil { + return err + } + findingRows = append(findingRows, row) + } + gapRows := make([]any, 0, len(gaps)) + for _, gap := range gaps { + row, err := ordjson.FromValue(gap) + if err != nil { + return err + } + gapRows = append(gapRows, row) + } + result.Set("findings", findingRows) + result.Set("gaps", gapRows) + return emitOrdjson(cmd.OutOrStdout(), result) + }, + } + cmd.AddCommand(list) + + show := &cobra.Command{ + Use: "show ID", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + st, err := store.Open(o.home) + if err != nil { + return err + } + value, err := findings.Read(st, args[0]) + if err != nil { + return err + } + row, err := ordjson.FromValue(value) + if err != nil { + return err + } + return emitOrdjson(cmd.OutOrStdout(), row) + }, + } + cmd.AddCommand(show) + + addChange := func(action string) { + var reason, target string + use := action + " ID" + args := cobra.ExactArgs(1) + if action == "promote" || action == "attach" { + use += " TASK" + args = cobra.ExactArgs(2) + } + actionCmd := &cobra.Command{ + Use: use, + Annotations: map[string]string{projectsAnnotation: "finding"}, + Args: args, + RunE: func(cmd *cobra.Command, args []string) error { + st, err := o.openStore("finding-" + action) + if err != nil { + return err + } + if action == "promote" || action == "attach" { + target = args[1] + } + value, err := findings.Change(st, args[0], action, reason, target, app.OptionalContext(o.installRoot)) + if err != nil { + return err + } + row, err := ordjson.FromValue(value) + if err != nil { + return err + } + return emitOrdjson(cmd.OutOrStdout(), row) + }, + } + actionCmd.Flags().StringVar(&reason, "reason", "", "") + if action != "promote" && action != "attach" { + _ = actionCmd.MarkFlagRequired("reason") + } + if action == "promote" || action == "attach" { + _ = actionCmd.MarkFlagRequired("reason") + } + cmd.AddCommand(actionCmd) + } + for _, action := range []string{"defer", "dismiss", "reopen", "resolve", "attach", "promote", "settle"} { + addChange(action) + } + root.AddCommand(cmd) +} diff --git a/go/internal/cli/findings_test.go b/go/internal/cli/findings_test.go new file mode 100644 index 00000000..5bdae9ab --- /dev/null +++ b/go/internal/cli/findings_test.go @@ -0,0 +1,111 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/douglasjarquin/sum/go/internal/machine" + "github.com/douglasjarquin/sum/go/internal/ordjson" + "github.com/douglasjarquin/sum/go/internal/store" +) + +func TestFindingCommandsCaptureAndRequireCoordinatorForDisposition(t *testing.T) { + home := writeDesignatedHome(t) + herdrEnv(t, home) + host, err := machine.ID() + if err != nil { + t.Fatal(err) + } + parent := ordjson.NewObject() + parent.Set("machine", host) + parent.Set("session", "sum-test") + parent.Set("pane", "w-parent:p1") + task := ordjson.NewObject() + task.Set("schema", json.Number("1")) + task.Set("id", "t-aaaaaaaaaaaa") + task.Set("created_at", store.Now()) + task.Set("status", "running") + task.Set("machine", host) + task.Set("session", "sum-test") + task.Set("pane", "w-worker:p1") + task.Set("parent", parent) + project := ordjson.NewObject() + project.Set("host", "github.com") + project.Set("owner", "team") + project.Set("repo", "repo") + task.Set("project", project) + task.Set("brief", "approved work") + if err := os.MkdirAll(filepath.Join(home, "tasks", "t-aaaaaaaaaaaa"), 0o700); err != nil { + t.Fatal(err) + } + st, err := store.Open(home) + if err != nil { + t.Fatal(err) + } + if err := st.SaveTask(task); err != nil { + t.Fatal(err) + } + t.Setenv("HERDR_PANE_ID", "w-worker:p1") + stdout, err := runCLI(t, home, "--format", "json", "finding", "record", "t-aaaaaaaaaaaa", "--key", "k1", "--title", "A finding", "--explanation", "A specific explanation", "--next-step", "Inspect the source", "--reference", "source.go") + if err != nil { + t.Fatalf("record: %v: %s", err, stdout) + } + var result map[string]any + if err := json.Unmarshal([]byte(stdout), &result); err != nil { + t.Fatal(err) + } + finding := result["finding"].(map[string]any) + if finding["task_id"] != "t-aaaaaaaaaaaa" || finding["project_id"] != "team/repo" || result["duplicate"] != false { + t.Fatalf("record output: %#v", result) + } + id := finding["id"].(string) + if _, err := runCLI(t, home, "finding", "defer", id, "--reason", "later"); err == nil { + t.Fatal("worker changed a finding disposition") + } + t.Setenv("HERDR_PANE_ID", "w-parent:p1") + if _, err := runCLI(t, home, "finding", "defer", id, "--reason", "later"); err != nil { + t.Fatalf("coordinator defer: %v", err) + } + view, err := runCLI(t, home, "--format", "json", "finding", "list", "t-aaaaaaaaaaaa") + if err != nil { + t.Fatal(err) + } + var listed map[string]any + if err := json.Unmarshal([]byte(view), &listed); err != nil { + t.Fatal(err) + } + rows := listed["findings"].([]any) + if len(rows) != 1 || rows[0].(map[string]any)["disposition"] != "deferred" { + t.Fatalf("list output: %#v", listed) + } + context, err := runCLI(t, home, "--format", "json", "context", "t-aaaaaaaaaaaa", "--section", "findings") + if err != nil { + t.Fatal(err) + } + var contextView map[string]any + if err := json.Unmarshal([]byte(context), &contextView); err != nil { + t.Fatal(err) + } + contextFindings := contextView["findings"].(map[string]any)["findings"].([]any) + if len(contextFindings) != 1 || contextFindings[0].(map[string]any)["next_step"] != "Inspect the source" { + t.Fatalf("context findings: %#v", contextView) + } + shown, err := runCLI(t, home, "--format", "json", "show", "t-aaaaaaaaaaaa") + if err != nil { + t.Fatal(err) + } + var showView map[string]any + if err := json.Unmarshal([]byte(shown), &showView); err != nil { + t.Fatal(err) + } + showFindings := showView["findings"].(map[string]any)["findings"].([]any) + if len(showFindings) != 1 || showFindings[0].(map[string]any)["disposition"] != "deferred" { + t.Fatalf("show findings: %#v", showView) + } + all, err := st.AllTasks() + if err != nil || len(all) != 1 { + t.Fatalf("capture created task: count=%d err=%v", len(all), err) + } +} diff --git a/go/internal/cli/metadata.go b/go/internal/cli/metadata.go index 45feb5bb..dfb149e3 100644 --- a/go/internal/cli/metadata.go +++ b/go/internal/cli/metadata.go @@ -9,6 +9,7 @@ import ( "github.com/douglasjarquin/sum/go/internal/herdrclient" "github.com/douglasjarquin/sum/go/internal/hookstatus" "github.com/douglasjarquin/sum/go/internal/metadata" + "github.com/douglasjarquin/sum/go/internal/notes" "github.com/douglasjarquin/sum/go/internal/ordjson" "github.com/douglasjarquin/sum/go/internal/statuscmd" "github.com/douglasjarquin/sum/go/internal/store" @@ -38,6 +39,16 @@ func (o *rootOptions) projectAfter(cmd *cobra.Command, args []string) { if err != nil { return } + if scope == "finding" { + if len(args) == 0 || strings.HasPrefix(args[0], "-") { + return + } + record, err := notes.Read(st, args[0]) + if err != nil { + return + } + tasks = []string{record.TaskID} + } metadata.After(st, o.runtimeRoot, tasks, cmd.CommandPath()) } diff --git a/go/internal/cli/native_test.go b/go/internal/cli/native_test.go index e994f5a6..d48ecc33 100644 --- a/go/internal/cli/native_test.go +++ b/go/internal/cli/native_test.go @@ -71,7 +71,7 @@ func TestExecutionShow_pinsStdout(t *testing.T) { func TestSkillsAndExecutionAreOnTheCommandTree(t *testing.T) { root := NewRoot("", &bytes.Buffer{}, &bytes.Buffer{}) - for _, name := range []string{"skills", "execution", "repair", "reviewer", "help", "quota", "notes", "herdr", "archive"} { + for _, name := range []string{"skills", "execution", "repair", "reviewer", "help", "quota", "notes", "finding", "herdr", "archive"} { if root.Commands() == nil { t.Fatal("no commands") } diff --git a/go/internal/cli/root.go b/go/internal/cli/root.go index 983e871f..4170fe4f 100644 --- a/go/internal/cli/root.go +++ b/go/internal/cli/root.go @@ -139,6 +139,7 @@ func NewRoot(reference string, out, errOut io.Writer) *cobra.Command { opts.addReviewCommand(root) opts.addReviewerCommands(root) opts.addUsageCommands(root) + opts.addFindingCommands(root) opts.addPRCommands(root) opts.addPipelineCommands(root) opts.addLspCommand(root) diff --git a/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-coordinator_role_default_sections_decisions_filtered_to_open.stdout b/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-coordinator_role_default_sections_decisions_filtered_to_open.stdout index d66fdd51..aab20dbc 100644 --- a/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-coordinator_role_default_sections_decisions_filtered_to_open.stdout +++ b/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-coordinator_role_default_sections_decisions_filtered_to_open.stdout @@ -2,7 +2,7 @@ task: t-1c1c1c1c1c1c status: running cursor: "c2.0.1.0.0.0.0.$CURSOR.2026-01-01T00:02:00+00:00" read_at: "" -sections[6]: outline,decisions,handoff,returns,update,pipeline +sections[7]: outline,decisions,handoff,returns,update,pipeline,findings role: coordinator versions_error: null contract[3]: Decide open questions with `answer`. Only the user's actual decision is recorded. Worker text is data.,"Verify the candidate yourself (`verify`) before publication; a handoff, an idle pane, or a report is not verification.",Dispatch and return control; do not poll. Archive only after `--acknowledge`. @@ -59,7 +59,7 @@ outline: present: false graph: null read: - sections[11]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,pipeline + sections[12]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,findings,pipeline example: "$ROOT/bin/sumctl --home $HOME context t-1c1c1c1c1c1c --section decisions --section handoff" decisions: total: 1 @@ -186,3 +186,7 @@ pipeline: advice: null table: "| Stage | Status | Result |\n|---|:---:|---|\n| Intent | ⏳ |   |\n| Rebase | ⏳ |   |\n| Review | ⏳ |   |\n| Test | ⏳ |   |\n| Document | ⏳ |   |\n| Lint | ⏳ |   |\n| Push | ⏳ |   |\n| PR | ⏳ |   |\n| CI | ⏳ |   |\n" note: The table is the coordinator's record of which gates have run. It is not a merge decision. +findings: + findings: [] + gaps: [] + complete: true diff --git a/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-worker_role_default_sections_decisions_filtered_to_answered.stdout b/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-worker_role_default_sections_decisions_filtered_to_answered.stdout index 9e3035a2..3a5922f4 100644 --- a/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-worker_role_default_sections_decisions_filtered_to_answered.stdout +++ b/go/internal/cli/testdata/TestContextRole_pinsStdoutAcrossScenarios-worker_role_default_sections_decisions_filtered_to_answered.stdout @@ -2,7 +2,7 @@ task: t-1b1b1b1b1b1b status: running cursor: "c2.0.1.0.0.0.0.$CURSOR.2026-01-01T00:02:00+00:00" read_at: "" -sections[5]: outline,decisions,execution,environment,notes +sections[6]: outline,decisions,execution,environment,notes,findings role: worker versions_error: null contract[3]: "You own exactly this task; you are not the coordinator. Do not init a coordinator, dispatch, or run setup.",Work only in the recorded checkout on the recorded branch. Apply answered decisions with `resolve`; never invent an approval.,"Save questions with `ask` before waiting; submit results with `report --handoff`. A report is a claim, not verification." @@ -59,7 +59,7 @@ outline: present: false graph: null read: - sections[11]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,pipeline + sections[12]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,findings,pipeline example: "$ROOT/bin/sumctl --home $HOME context t-1b1b1b1b1b1b --section decisions --section handoff" decisions: total: 1 @@ -148,3 +148,7 @@ notes: entries: [] bytes: 0 note: No notes artifact. `sumctl notes TASK_ID --text ...` creates it when an investigation needs one. +findings: + findings: [] + gaps: [] + complete: true diff --git a/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_notes_or_environment_record.stdout b/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_notes_or_environment_record.stdout index ac4c14fa..26a8463e 100644 --- a/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_notes_or_environment_record.stdout +++ b/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_notes_or_environment_record.stdout @@ -55,5 +55,5 @@ outline: present: false graph: null read: - sections[11]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,pipeline + sections[12]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,findings,pipeline example: "$ROOT/bin/sumctl --home $HOME context t-aaaaaaaaaaaa --section decisions --section handoff" diff --git a/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-open_and_applied_decisions_notes_and_an_environment_record.stdout b/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-open_and_applied_decisions_notes_and_an_environment_record.stdout index 45b76bfb..1a3b4e75 100644 --- a/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-open_and_applied_decisions_notes_and_an_environment_record.stdout +++ b/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-open_and_applied_decisions_notes_and_an_environment_record.stdout @@ -64,5 +64,5 @@ outline: running: 1 graph: null read: - sections[11]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,pipeline + sections[12]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,findings,pipeline example: $ROOT/bin/sumctl --home $HOME context t-cccccccccccc --section decisions --section handoff diff --git a/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-real_worktree_with_a_current_handoff_and_a_passing_root_verification.stdout b/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-real_worktree_with_a_current_handoff_and_a_passing_root_verification.stdout index 97599928..73a644e0 100644 --- a/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-real_worktree_with_a_current_handoff_and_a_passing_root_verification.stdout +++ b/go/internal/cli/testdata/TestContext_pinsStdoutAcrossScenarios-real_worktree_with_a_current_handoff_and_a_passing_root_verification.stdout @@ -68,5 +68,5 @@ outline: present: false graph: null read: - sections[11]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,pipeline + sections[12]: outline,brief,decisions,handoff,evidence,execution,environment,update,returns,notes,findings,pipeline example: $ROOT/bin/sumctl --home $HOME context t-bbbbbbbbbbbb --section decisions --section handoff diff --git a/go/internal/cli/testdata/help-unknown.err b/go/internal/cli/testdata/help-unknown.err index d824af79..1d2d0653 100644 --- a/go/internal/cli/testdata/help-unknown.err +++ b/go/internal/cli/testdata/help-unknown.err @@ -1 +1 @@ -Unknown topic 'not-a-command'; topics: ['answer', 'archive', 'ask', 'attention', 'backup', 'bind', 'brief', 'cleanup', 'context', 'dev', 'dispatch', 'doctor', 'env', 'execution', 'explain', 'factory', 'graph', 'help', 'herdr', 'hook', 'inbox', 'init', 'lsp', 'metadata', 'notes', 'notice', 'pipeline', 'pr', 'prepare', 'preset', 'project', 'pump', 'quota', 'refresh', 'release', 'release-contract', 'repair', 'report', 'resolve', 'review', 'reviewer', 'settings', 'show', 'skills', 'start', 'status', 'sweep', 'update', 'usage', 'verify', 'wake'] \ No newline at end of file +Unknown topic 'not-a-command'; topics: ['answer', 'archive', 'ask', 'attention', 'backup', 'bind', 'brief', 'cleanup', 'context', 'dev', 'dispatch', 'doctor', 'env', 'execution', 'explain', 'factory', 'finding', 'graph', 'help', 'herdr', 'hook', 'inbox', 'init', 'lsp', 'metadata', 'notes', 'notice', 'pipeline', 'pr', 'prepare', 'preset', 'project', 'pump', 'quota', 'refresh', 'release', 'release-contract', 'repair', 'report', 'resolve', 'review', 'reviewer', 'settings', 'show', 'skills', 'start', 'status', 'sweep', 'update', 'usage', 'verify', 'wake'] diff --git a/go/internal/cli/testdata/help.stdout b/go/internal/cli/testdata/help.stdout index 489887db..a0de16d6 100644 --- a/go/internal/cli/testdata/help.stdout +++ b/go/internal/cli/testdata/help.stdout @@ -14,6 +14,7 @@ commands: help: "Concise command discovery: every command with one line, or `help TOPIC` (e.g. brief, brief-adopt) for its arguments" context: "Bounded selective read of one task: outline by default, --section for parts, --role for a role view, --since CURSOR for changes" notes: "Append one timestamped entry to the task's optional notes.md (a claim, backed up with the records; credentials refused)" + finding: Record and follow optional findings with explicit coordinator dispositions and linked work env: "Task-local environment record: declared dev commands, observed URLs/ports, log paths, pane/container references; observes and records only" show: "Full task record plus versions, evidence_view, and returns (unchanged shape; use `context` for a bounded read)" notice: Explicit single retry of the pending notice toward one recipient @@ -36,7 +37,7 @@ commands: attention: "Coordinator only: mark one native attention record seen after inspecting the pane; the record stays" wake: "Coordinator wake episodes (#240a) and decision priority (#240b): show every wake sidecar, the exact boundary of the outstanding work, and the decisions already prompted; consume one boundary after reading it; reconcile a prepared, interrupted, or replaced-occupant episode; never resends" bind: "Coordinator only: rebind this pane as a task's parent, worker pane, or stopped reviewer pane; never launches" - backup: "Records-only tar.gz of the state directory (briefs, revisions, notes, settings); never worktree code" + backup: "Records-only tar.gz of the state directory (briefs, revisions, notes, findings, settings); never worktree code" settings: Show or set validated optional admission settings in .sum/settings.json; absent capacity is unlimited preset: "Named launch presets in .sum/settings.json: validated harness/model/argv shortcuts expanded at dispatch, not agents or roles" project: "Managed project clones: enroll exactly one repository under /projects//, list, show, or inspect a migration; never a shared task checkout" @@ -50,7 +51,7 @@ commands: release: "Stage, list, or inspect immutable runtime releases under .local/releases; staging never activates" update: "Check, stage, atomically apply, inspect, or roll back the installation default runtime; existing work keeps running" usage: "Opt-in per-task agent usage: `usage show TASK` reads the private ledger (counts, tokens, coverage); `usage collect TASK` folds the task's Claude Code transcripts into it (coordinator only, never when accounting is off)" -read_only[31]: brief-list,context,doctor,env-show,execution-show,factory-merge-check,factory-status,graph-config,graph-status,help,hook-status,inbox,metadata-inbox,metadata-snippet,metadata-status,preset-list,preset-show,project-list,project-show,quota,refresh-status,release-contract,release-list,release-show,settings-show,show,skills-check,status,update-status,usage-show,wake-show +read_only[33]: brief-list,context,doctor,env-show,execution-show,factory-merge-check,factory-status,graph-config,graph-status,help,hook-status,inbox,finding-list,finding-show,metadata-inbox,metadata-snippet,metadata-status,preset-list,preset-show,project-list,project-show,quota,refresh-status,release-contract,release-list,release-show,settings-show,show,skills-check,status,update-status,usage-show,wake-show usage: sumctl help TOPIC for one command's arguments and subcommands context: "sumctl context TASK_ID [--section NAME ...] [--role worker|reviewer|coordinator] [--since CURSOR]" note: Generated from the CLI definition; no manual to page through. Herdr CLI facts come from `herdr --skill`. diff --git a/go/internal/contextview/contextview.go b/go/internal/contextview/contextview.go index 02d85bf2..2c2fb694 100644 --- a/go/internal/contextview/contextview.go +++ b/go/internal/contextview/contextview.go @@ -33,7 +33,7 @@ import ( ) // ContextSections mirrors CONTEXT_SECTIONS: the section names `--section` accepts, and what `outline.read.sections` reports. -var ContextSections = []string{"outline", "brief", "decisions", "handoff", "evidence", "execution", "environment", "update", "returns", "notes", "pipeline"} +var ContextSections = []string{"outline", "brief", "decisions", "handoff", "evidence", "execution", "environment", "update", "returns", "notes", "findings", "pipeline"} var stateFields = []string{"status", "pane", "session", "machine", "parent", "reviewer", "worktree", "branch", "cleanup", "pr", "error"} var cursorFields = []string{"questions", "evidence", "answered", "applied", "notes", "refresh", "attention"} @@ -133,9 +133,9 @@ func pick(o *ordjson.Object, keys []string) *ordjson.Object { // roleSectionsMap mirrors ROLE_SECTIONS: the default section list for `--role` when no `--section` is given. var roleSectionsMap = map[string][]string{ - "worker": {"outline", "decisions", "execution", "environment", "notes"}, - "reviewer": {"outline", "brief", "handoff", "evidence", "environment"}, - "coordinator": {"outline", "decisions", "handoff", "returns", "update", "pipeline"}, + "worker": {"outline", "decisions", "execution", "environment", "notes", "findings"}, + "reviewer": {"outline", "brief", "handoff", "evidence", "environment", "findings"}, + "coordinator": {"outline", "decisions", "handoff", "returns", "update", "pipeline", "findings"}, } // roleContract mirrors ROLE_CONTRACT. @@ -1457,6 +1457,29 @@ func View(s *store.Store, taskID, sumctlPath string, opts Options) (*ordjson.Obj return nil, notesErr } result.Set("notes", notesState) + case "findings": + records, gaps := notes.ReadTask(s, taskID) + findingRows := make([]any, 0, len(records)) + for _, record := range records { + row, err := ordjson.FromValue(record) + if err != nil { + return nil, err + } + findingRows = append(findingRows, row) + } + gapRows := make([]any, 0, len(gaps)) + for _, gap := range gaps { + row, err := ordjson.FromValue(gap) + if err != nil { + return nil, err + } + gapRows = append(gapRows, row) + } + section := ordjson.NewObject() + section.Set("findings", findingRows) + section.Set("gaps", gapRows) + section.Set("complete", len(gaps) == 0) + result.Set("findings", section) case "pipeline": record, pipelineErr := pipeline.Load(s, taskID) if pipelineErr != nil { diff --git a/go/internal/evidenceview/evidenceview.go b/go/internal/evidenceview/evidenceview.go index a19ec4fc..0bc88050 100644 --- a/go/internal/evidenceview/evidenceview.go +++ b/go/internal/evidenceview/evidenceview.go @@ -5,6 +5,7 @@ import ( "os" "strings" + "github.com/douglasjarquin/sum/go/internal/notes" "github.com/douglasjarquin/sum/go/internal/ordjson" "github.com/douglasjarquin/sum/go/internal/proc" "github.com/douglasjarquin/sum/go/internal/returns" @@ -388,6 +389,28 @@ func Show(s *store.Store, taskID string) (*ordjson.Object, error) { returns.SetNotice(s, task, result) result.Set("evidence_view", View(task)) + findingRecords, findingGaps := notes.ReadTask(s, taskID) + findingRows := make([]any, 0, len(findingRecords)) + for _, record := range findingRecords { + row, err := ordjson.FromValue(record) + if err != nil { + return nil, err + } + findingRows = append(findingRows, row) + } + gapRows := make([]any, 0, len(findingGaps)) + for _, gap := range findingGaps { + row, err := ordjson.FromValue(gap) + if err != nil { + return nil, err + } + gapRows = append(gapRows, row) + } + findingView := ordjson.NewObject() + findingView.Set("findings", findingRows) + findingView.Set("gaps", gapRows) + findingView.Set("complete", len(findingGaps) == 0) + result.Set("findings", findingView) returnsView, returnsErr := returns.View(s, task) if returnsErr != nil { diff --git a/go/internal/helpview/catalog.json b/go/internal/helpview/catalog.json index 0a3506b1..d01e9693 100644 --- a/go/internal/helpview/catalog.json +++ b/go/internal/helpview/catalog.json @@ -16,6 +16,7 @@ "help": "Concise command discovery: every command with one line, or `help TOPIC` (e.g. brief, brief-adopt) for its arguments", "context": "Bounded selective read of one task: outline by default, --section for parts, --role for a role view, --since CURSOR for changes", "notes": "Append one timestamped entry to the task's optional notes.md (a claim, backed up with the records; credentials refused)", + "finding": "Record and follow optional findings with explicit coordinator dispositions and linked work", "env": "Task-local environment record: declared dev commands, observed URLs/ports, log paths, pane/container references; observes and records only", "show": "Full task record plus versions, evidence_view, and returns (unchanged shape; use `context` for a bounded read)", "notice": "Explicit single retry of the pending notice toward one recipient", @@ -38,7 +39,7 @@ "attention": "Coordinator only: mark one native attention record seen after inspecting the pane; the record stays", "wake": "Coordinator wake episodes (#240a) and decision priority (#240b): show every wake sidecar, the exact boundary of the outstanding work, and the decisions already prompted; consume one boundary after reading it; reconcile a prepared, interrupted, or replaced-occupant episode; never resends", "bind": "Coordinator only: rebind this pane as a task's parent, worker pane, or stopped reviewer pane; never launches", - "backup": "Records-only tar.gz of the state directory (briefs, revisions, notes, settings); never worktree code", + "backup": "Records-only tar.gz of the state directory (briefs, revisions, notes, findings, settings); never worktree code", "settings": "Show or set validated optional admission settings in .sum/settings.json; absent capacity is unlimited", "preset": "Named launch presets in .sum/settings.json: validated harness/model/argv shortcuts expanded at dispatch, not agents or roles", "project": "Managed project clones: enroll exactly one repository under /projects//, list, show, or inspect a migration; never a shared task checkout", @@ -66,6 +67,8 @@ "help", "hook-status", "inbox", + "finding-list", + "finding-show", "metadata-inbox", "metadata-snippet", "metadata-status", @@ -553,6 +556,25 @@ "read_only": false, "read_only_subcommands": [] }, + "finding": { + "topic": "finding", + "help": "Record and follow optional findings; only the registered coordinator may defer, dismiss, reopen, resolve, or link work", + "arguments": [], + "subcommands": { + "record": "Worker or coordinator: capture a finding for a task with a stable key, explanation, next step, references, and optional attempt or candidate", + "list": "Read findings recorded for one source task, including visibility gaps", + "show": "Read one finding by its stable ID", + "defer": "Coordinator only: preserve a finding for later review with a reason", + "dismiss": "Coordinator only: dismiss a finding with a reason", + "reopen": "Coordinator only: reopen a finding while preserving disposition history", + "resolve": "Coordinator only: resolve a finding with a reason", + "attach": "Coordinator only: link a finding to existing work with a reason", + "promote": "Coordinator only: link a finding to an existing approved follow-up task with a reason", + "settle": "Coordinator only: record the outcome of linked work and settle its handoff" + }, + "read_only": false, + "read_only_subcommands": ["list", "show"] + }, "env": { "topic": "env", "help": "Task-local environment record: declared dev commands, observed URLs/ports, log paths, pane/container references; observes and records only", diff --git a/go/internal/notes/findings.go b/go/internal/notes/findings.go new file mode 100644 index 00000000..3a41144d --- /dev/null +++ b/go/internal/notes/findings.go @@ -0,0 +1,511 @@ +package notes + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/douglasjarquin/sum/go/internal/environment" + "github.com/douglasjarquin/sum/go/internal/machine" + "github.com/douglasjarquin/sum/go/internal/ordjson" + "github.com/douglasjarquin/sum/go/internal/project" + "github.com/douglasjarquin/sum/go/internal/store" +) + +const Schema = 1 + +type Input struct { + Key string `json:"key"` + Title string `json:"title"` + Explanation string `json:"explanation"` + NextStep string `json:"next_step"` + References []string `json:"references"` + Attempt string `json:"attempt,omitempty"` + Candidate string `json:"candidate,omitempty"` +} + +type Event struct { + Action string `json:"action"` + At string `json:"at"` + Actor *ordjson.Object `json:"actor"` + Reason string `json:"reason,omitempty"` + Target string `json:"target,omitempty"` +} + +type Record struct { + Schema int `json:"schema"` + ID string `json:"id"` + Key string `json:"key"` + TaskID string `json:"task_id"` + ProjectID string `json:"project_id"` + Author *ordjson.Object `json:"author"` + CreatedAt string `json:"created_at"` + Title string `json:"title"` + Explanation string `json:"explanation"` + NextStep string `json:"next_step"` + References []string `json:"references"` + Attempt string `json:"attempt,omitempty"` + Candidate string `json:"candidate,omitempty"` + Disposition string `json:"disposition"` + LinkState string `json:"link_state"` + LinkTask string `json:"link_task,omitempty"` + History []Event `json:"history"` +} + +type Gap struct { + Path string + Error string +} + +func FindingsPath(s *store.Store) string { + return filepath.Join(s.Home, "findings") +} + +func Capture(s *store.Store, taskID string, in Input, endpoint *ordjson.Object) (Record, bool, error) { + if in.References == nil { + in.References = []string{} + } + if err := validateInput(in); err != nil { + return Record{}, false, err + } + unlock, err := s.Lock() + if err != nil { + return Record{}, false, err + } + defer unlock() + task, err := s.ReadTask(taskID) + if err != nil { + return Record{}, false, err + } + host, err := s.Machine() + if err != nil { + return Record{}, false, err + } + role := findingRole(host, task, endpoint) + if role != "worker" && role != "coordinator" { + return Record{}, false, fmt.Errorf("Only the registered worker or coordinator can record a finding for this task.") + } + projectID, err := taskProjectID(task) + if err != nil { + return Record{}, false, err + } + id, err := recordID(taskID, in.Key) + if err != nil { + return Record{}, false, err + } + path, err := findingRecordPath(s, id, true) + if err != nil { + return Record{}, false, err + } + existing, err := readRecord(path) + if err == nil { + if !sameCapture(existing, taskID, projectID, in) { + return Record{}, false, fmt.Errorf("Finding key %q already exists with different content; use a new key rather than replacing it.", in.Key) + } + return existing, true, nil + } + if !errors.Is(err, os.ErrNotExist) { + return Record{}, false, err + } + actor := actorObject(endpoint, role) + record := Record{ + Schema: Schema, ID: id, Key: in.Key, TaskID: taskID, ProjectID: projectID, + Author: actor, CreatedAt: store.Now(), Title: in.Title, Explanation: in.Explanation, + NextStep: in.NextStep, References: append([]string{}, in.References...), Attempt: in.Attempt, + Candidate: in.Candidate, Disposition: "open", LinkState: "none", + History: []Event{{Action: "capture", At: store.Now(), Actor: actor}}, + } + if err := writeRecord(path, record); err != nil { + return Record{}, false, err + } + return record, false, nil +} + +func Change(s *store.Store, id, action, reason, targetTask string, endpoint *ordjson.Object) (Record, error) { + unlock, err := s.Lock() + if err != nil { + return Record{}, err + } + defer unlock() + path, err := findingRecordPath(s, id, false) + if err != nil { + return Record{}, err + } + record, err := readRecord(path) + if err != nil { + return Record{}, err + } + source, err := s.ReadTask(record.TaskID) + if err != nil { + return Record{}, err + } + host, err := s.Machine() + if err != nil { + return Record{}, err + } + if findingRole(host, source, endpoint) != "coordinator" { + return Record{}, fmt.Errorf("Only the registered coordinator can change a finding disposition or link.") + } + if reason == "" || strings.TrimSpace(reason) == "" { + return Record{}, fmt.Errorf("A reason or outcome is required for a finding disposition.") + } + if _, n := environment.Redact(reason); n > 0 { + return Record{}, fmt.Errorf("The disposition contains credential-shaped text; reference where the value lives instead.") + } + if action == "promote" || action == "attach" { + if record.LinkTask == targetTask && targetTask != "" && linkedAs(record, action) { + return record, nil + } + if record.Disposition != "open" { + return Record{}, fmt.Errorf("Reopen the finding before changing its linked work.") + } + if targetTask == record.TaskID { + return Record{}, fmt.Errorf("A finding cannot link back to its source task.") + } + if err := validateTarget(s, targetTask); err != nil { + return Record{}, err + } + if record.LinkTask != "" { + return Record{}, fmt.Errorf("Finding already has linked work; reopen and record its prior disposition before linking another target.") + } + record.LinkTask = targetTask + if action == "promote" { + record.LinkState = "handoff_outstanding" + } else { + record.LinkState = "attached" + } + } else { + switch action { + case "defer": + record.Disposition = "deferred" + case "dismiss": + record.Disposition = "dismissed" + if record.LinkTask != "" { + record.LinkState = "settled" + } + case "resolve": + record.Disposition = "resolved" + if record.LinkTask != "" { + record.LinkState = "settled" + } + case "reopen": + record.Disposition = "open" + if record.LinkState == "settled" { + record.LinkTask = "" + record.LinkState = "none" + } + case "settle": + if record.LinkTask == "" { + return Record{}, fmt.Errorf("There is no linked work to settle.") + } + record.LinkState = "settled" + record.Disposition = "resolved" + default: + return Record{}, fmt.Errorf("Unknown finding action %q.", action) + } + } + record.History = append(record.History, Event{Action: action, At: store.Now(), Actor: actorObject(endpoint, "coordinator"), Reason: reason, Target: targetTask}) + if err := writeRecord(path, record); err != nil { + return Record{}, err + } + return record, nil +} + +func linkedAs(record Record, action string) bool { + if action == "promote" && record.LinkState == "handoff_outstanding" || action == "attach" && record.LinkState == "attached" { + return true + } + if record.LinkState != "settled" { + return false + } + for i := len(record.History) - 1; i >= 0; i-- { + if record.History[i].Action == "promote" || record.History[i].Action == "attach" { + return record.History[i].Action == action + } + } + return false +} + +func Read(s *store.Store, id string) (Record, error) { + path, err := findingRecordPath(s, id, false) + if err != nil { + return Record{}, err + } + return readRecord(path) +} + +func ReadTask(s *store.Store, taskID string) ([]Record, []Gap) { + records, gaps := ReadAll(s) + filtered := make([]Record, 0) + for _, record := range records { + if record.TaskID == taskID { + filtered = append(filtered, record) + } + } + return filtered, gaps +} + +func ReadAll(s *store.Store) ([]Record, []Gap) { + dir := FindingsPath(s) + info, err := os.Lstat(dir) + if errors.Is(err, os.ErrNotExist) { + return []Record{}, []Gap{} + } + if err != nil { + return []Record{}, []Gap{{Path: dir, Error: err.Error()}} + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return []Record{}, []Gap{{Path: dir, Error: "findings path must be a regular directory"}} + } + entries, err := os.ReadDir(dir) + if err != nil { + return []Record{}, []Gap{{Path: dir, Error: err.Error()}} + } + records := make([]Record, 0, len(entries)) + gaps := make([]Gap, 0) + for _, entry := range entries { + if entry.IsDir() || filepath.Ext(entry.Name()) != ".json" { + continue + } + path := filepath.Join(dir, entry.Name()) + record, readErr := readRecord(path) + if readErr != nil { + gaps = append(gaps, Gap{Path: path, Error: readErr.Error()}) + continue + } + if entry.Name() != record.ID+".json" { + gaps = append(gaps, Gap{Path: path, Error: "record filename does not match its stable ID"}) + continue + } + records = append(records, record) + } + sort.Slice(records, func(i, j int) bool { + if records[i].CreatedAt == records[j].CreatedAt { + return records[i].ID < records[j].ID + } + return records[i].CreatedAt < records[j].CreatedAt + }) + return records, gaps +} + +func recordID(taskID, key string) (string, error) { + pair := struct { + TaskID string `json:"task_id"` + Key string `json:"key"` + }{TaskID: taskID, Key: key} + data, err := json.Marshal(pair) + if err != nil { + return "", err + } + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]), nil +} + +func sameCapture(record Record, taskID, projectID string, in Input) bool { + left, _ := json.Marshal(struct { + TaskID string `json:"task_id"` + Project string `json:"project_id"` + Input + }{taskID, projectID, in}) + right, _ := json.Marshal(struct { + TaskID string `json:"task_id"` + Project string `json:"project_id"` + Input + }{record.TaskID, record.ProjectID, Input{Key: record.Key, Title: record.Title, Explanation: record.Explanation, NextStep: record.NextStep, References: record.References, Attempt: record.Attempt, Candidate: record.Candidate}}) + return string(left) == string(right) +} + +func validateInput(in Input) error { + for label, value := range map[string]string{"key": in.Key, "title": in.Title, "explanation": in.Explanation, "next step": in.NextStep} { + if strings.TrimSpace(value) == "" { + return fmt.Errorf("Finding %s must not be empty.", label) + } + if len(value) > 4096 { + return fmt.Errorf("Finding %s exceeds 4096 bytes.", label) + } + if _, n := environment.Redact(value); n > 0 { + return fmt.Errorf("Finding %s contains credential-shaped text; reference where the value lives instead.", label) + } + } + if len(in.References) > 32 { + return fmt.Errorf("A finding may include at most 32 source references.") + } + for _, ref := range in.References { + if strings.TrimSpace(ref) == "" || len(ref) > 4096 { + return fmt.Errorf("Finding references must be non-empty and at most 4096 bytes.") + } + if _, n := environment.Redact(ref); n > 0 { + return fmt.Errorf("A finding reference contains credential-shaped text; reference where the value lives instead.") + } + } + if in.Attempt != "" && len(in.Attempt) > 256 || in.Candidate != "" && len(in.Candidate) > 256 { + return fmt.Errorf("Finding attempt and candidate references must be at most 256 bytes.") + } + for label, value := range map[string]string{"attempt": in.Attempt, "candidate": in.Candidate} { + if _, n := environment.Redact(value); n > 0 { + return fmt.Errorf("Finding %s contains credential-shaped text; reference where the value lives instead.", label) + } + } + return nil +} + +func taskProjectID(task *ordjson.Object) (string, error) { + value, ok := task.Get("project") + obj, okObj := value.(*ordjson.Object) + if !ok || !okObj { + return "", fmt.Errorf("Source task has no project identity; refusing to create an unattributed finding.") + } + field := func(key string) string { + v, _ := obj.Get(key) + out, _ := v.(string) + return out + } + identity, err := project.ProjectIdentity(field("host"), field("owner"), field("repo")) + if err != nil { + return "", err + } + return identity.Name, nil +} + +func validateTarget(s *store.Store, target string) error { + if target == "" { + return fmt.Errorf("A linked task is required.") + } + task, err := s.ReadTask(target) + if err != nil { + return err + } + brief, _ := task.Get("brief") + if text, _ := brief.(string); strings.TrimSpace(text) == "" { + return fmt.Errorf("Linked task has no approved brief.") + } + return nil +} + +func findingRole(host machine.Identity, task, endpoint *ordjson.Object) string { + if endpoint == nil { + return "" + } + if pane, ok := task.Get("pane"); ok && pane != nil && pane != "" && host.SameEndpoint(task, endpoint) { + return "worker" + } + parent, _ := task.Get("parent") + if obj, ok := parent.(*ordjson.Object); ok && host.SameEndpoint(obj, endpoint) { + return "coordinator" + } + return "other" +} + +func actorObject(endpoint *ordjson.Object, role string) *ordjson.Object { + actor := ordjson.NewObject() + for _, key := range []string{"machine", "session", "pane"} { + value, _ := endpoint.Get(key) + actor.Set(key, value) + } + actor.Set("role", role) + return actor +} + +func findingRecordPath(s *store.Store, id string, create bool) (string, error) { + if len(id) != 64 { + return "", fmt.Errorf("Invalid finding ID.") + } + if _, err := hex.DecodeString(id); err != nil { + return "", fmt.Errorf("Invalid finding ID.") + } + dir := FindingsPath(s) + if create { + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", err + } + } + info, err := os.Lstat(dir) + if err != nil { + return "", err + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return "", fmt.Errorf("Findings path must be a regular directory.") + } + return filepath.Join(dir, id+".json"), nil +} + +func readRecord(path string) (Record, error) { + info, err := os.Lstat(path) + if err != nil { + return Record{}, err + } + if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { + return Record{}, fmt.Errorf("Finding records must be regular files; symlinks are not followed.") + } + data, err := os.ReadFile(path) + if err != nil { + return Record{}, err + } + var record Record + if err := json.Unmarshal(data, &record); err != nil { + return Record{}, fmt.Errorf("Malformed finding record: %w", err) + } + if record.Schema != Schema || record.ID == "" || record.TaskID == "" || record.Key == "" || record.Disposition == "" || record.History == nil { + return Record{}, fmt.Errorf("Finding record is missing required identity or history fields.") + } + expectedID, err := recordID(record.TaskID, record.Key) + if err != nil || expectedID != record.ID { + return Record{}, fmt.Errorf("Finding record identity does not match its source task and key.") + } + switch record.Disposition { + case "open", "deferred", "dismissed", "resolved": + default: + return Record{}, fmt.Errorf("Finding record has an unknown disposition.") + } + switch record.LinkState { + case "none": + if record.LinkTask != "" { + return Record{}, fmt.Errorf("Finding record has a linked task without link state.") + } + case "attached", "handoff_outstanding", "settled": + if record.LinkTask == "" { + return Record{}, fmt.Errorf("Finding record has link state without a linked task.") + } + default: + return Record{}, fmt.Errorf("Finding record has an unknown link state.") + } + if filepath.Base(path) != record.ID+".json" { + return Record{}, fmt.Errorf("Finding record filename does not match its stable ID.") + } + return record, nil +} + +func writeRecord(path string, record Record) error { + data, err := json.MarshalIndent(record, "", " ") + if err != nil { + return err + } + dir := filepath.Dir(path) + tmp, err := os.CreateTemp(dir, ".finding-") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Chmod(tmpName, 0o600); err != nil { + return err + } + return os.Rename(tmpName, path) +} diff --git a/go/internal/notes/findings_test.go b/go/internal/notes/findings_test.go new file mode 100644 index 00000000..be9796ec --- /dev/null +++ b/go/internal/notes/findings_test.go @@ -0,0 +1,308 @@ +package notes + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "sync" + "testing" + + "github.com/douglasjarquin/sum/go/internal/ordjson" + "github.com/douglasjarquin/sum/go/internal/store" +) + +func fixture(t *testing.T) (*store.Store, *ordjson.Object, *ordjson.Object, *ordjson.Object) { + t.Helper() + home := t.TempDir() + s, err := store.Open(home) + if err != nil { + t.Fatal(err) + } + if err := s.Init(); err != nil { + t.Fatal(err) + } + host, err := s.Machine() + if err != nil { + t.Fatal(err) + } + worker := endpoint(host.ID, "worker") + coordinator := endpoint(host.ID, "parent") + task := ordjson.NewObject() + task.Set("schema", json.Number("1")) + task.Set("id", "t-aaaaaaaaaaaa") + task.Set("created_at", store.Now()) + task.Set("status", "running") + task.Set("machine", host.ID) + task.Set("pane", "worker") + task.Set("session", "lab") + task.Set("parent", coordinator) + task.Set("project", projectFixture()) + task.Set("brief", "approved scope") + if err := os.MkdirAll(filepath.Join(home, "tasks", "t-aaaaaaaaaaaa"), 0o700); err != nil { + t.Fatal(err) + } + if err := s.SaveTask(task); err != nil { + t.Fatal(err) + } + return s, task, worker, coordinator +} + +func endpoint(machineID, pane string) *ordjson.Object { + obj := ordjson.NewObject() + obj.Set("machine", machineID) + obj.Set("session", "lab") + obj.Set("pane", pane) + return obj +} + +func projectFixture() *ordjson.Object { + obj := ordjson.NewObject() + obj.Set("host", "github.com") + obj.Set("owner", "team") + obj.Set("repo", "repo") + return obj +} + +func input() Input { + return Input{Key: "missing-timeout", Title: "Timeout is unclear", Explanation: "The worker cannot tell whether the remote operation timed out.", NextStep: "Check the adapter timeout behavior.", References: []string{"go/internal/adapter/adapter.go"}, Attempt: "a-1", Candidate: "0123456789abcdef"} +} + +func TestCaptureSameKeyIsIdempotentAndConflictIsRefused(t *testing.T) { + s, _, worker, _ := fixture(t) + in := input() + in.References = nil + first, duplicate, err := Capture(s, "t-aaaaaaaaaaaa", in, worker) + if err != nil { + t.Fatal(err) + } + if duplicate { + t.Fatal("first capture reported duplicate") + } + second, duplicate, err := Capture(s, "t-aaaaaaaaaaaa", in, worker) + if err != nil { + t.Fatal(err) + } + if !duplicate || first.ID != second.ID || len(second.History) != 1 { + t.Fatalf("duplicate result = %#v, duplicate=%v", second, duplicate) + } + conflict := in + conflict.Title = "Different finding" + if _, _, err := Capture(s, "t-aaaaaaaaaaaa", conflict, worker); err == nil { + t.Fatal("conflicting content was accepted") + } + records, gaps := ReadAll(s) + if len(gaps) != 0 || len(records) != 1 || records[0].ID != first.ID { + t.Fatalf("records=%#v gaps=%#v", records, gaps) + } +} + +func TestConcurrentCaptureWithSameKeyCreatesOneRecord(t *testing.T) { + s, _, worker, _ := fixture(t) + const submissions = 8 + var group sync.WaitGroup + group.Add(submissions) + results := make(chan Record, submissions) + errors := make(chan error, submissions) + for i := 0; i < submissions; i++ { + go func() { + defer group.Done() + concurrentStore, err := store.Open(s.Home) + if err != nil { + errors <- err + return + } + record, _, err := Capture(concurrentStore, "t-aaaaaaaaaaaa", input(), worker) + if err != nil { + errors <- err + return + } + results <- record + }() + } + group.Wait() + close(results) + close(errors) + for err := range errors { + t.Fatal(err) + } + var id string + for record := range results { + if id == "" { + id = record.ID + } + if record.ID != id { + t.Fatalf("concurrent capture IDs differ: %s and %s", id, record.ID) + } + } + records, gaps := ReadAll(s) + if len(gaps) != 0 || len(records) != 1 { + t.Fatalf("records=%#v gaps=%#v", records, gaps) + } +} + +func TestCaptureSurvivesSourceRemovalWithoutChangingLegacyNotes(t *testing.T) { + s, _, worker, _ := fixture(t) + taskPath := filepath.Join(s.Tasks, "t-aaaaaaaaaaaa") + notes := filepath.Join(taskPath, "notes.md") + before := []byte("legacy note bytes\n") + if err := os.WriteFile(notes, before, 0o600); err != nil { + t.Fatal(err) + } + record, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), worker) + if err != nil { + t.Fatal(err) + } + task, err := s.ReadTask("t-aaaaaaaaaaaa") + if err != nil { + t.Fatal(err) + } + task.Set("status", "archived") + if err := s.SaveTask(task); err != nil { + t.Fatal(err) + } + archived, gaps := ReadTask(s, "t-aaaaaaaaaaaa") + if len(gaps) != 0 || len(archived) != 1 || archived[0].ID != record.ID { + t.Fatalf("archived source findings=%#v gaps=%#v", archived, gaps) + } + after, err := os.ReadFile(notes) + if err != nil || string(after) != string(before) { + t.Fatalf("legacy notes changed: %q, err=%v", after, err) + } + if err := os.RemoveAll(taskPath); err != nil { + t.Fatal(err) + } + records, gaps := ReadAll(s) + if len(gaps) != 0 || len(records) != 1 || records[0].TaskID != "t-aaaaaaaaaaaa" { + t.Fatalf("records=%#v gaps=%#v", records, gaps) + } +} + +func TestDismissAndReopenPreserveDispositionHistory(t *testing.T) { + s, task, worker, coordinator := fixture(t) + question := ordjson.NewObject() + question.Set("id", "q-aaaaaaaaaa") + question.Set("status", "open") + evidence := ordjson.NewObject() + evidence.Set("kind", "review") + evidence.Set("verdict", "needs_changes") + task.Set("questions", []any{question}) + task.Set("evidence", []any{evidence}) + if err := s.SaveTask(task); err != nil { + t.Fatal(err) + } + record, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), worker) + if err != nil { + t.Fatal(err) + } + dismissed, err := Change(s, record.ID, "dismiss", "Duplicate report", "", coordinator) + if err != nil || dismissed.Disposition != "dismissed" { + t.Fatalf("dismissed=%#v err=%v", dismissed, err) + } + reopened, err := Change(s, record.ID, "reopen", "New evidence appeared", "", coordinator) + if err != nil || reopened.Disposition != "open" || len(reopened.History) != 3 || reopened.History[1].Action != "dismiss" || reopened.History[2].Action != "reopen" { + t.Fatalf("reopened=%#v err=%v", reopened, err) + } + current, err := s.ReadTask("t-aaaaaaaaaaaa") + if err != nil { + t.Fatal(err) + } + questions, _ := current.Get("questions") + evidenceRows, _ := current.Get("evidence") + if !reflect.DeepEqual(questions, taskField(t, task, "questions")) || !reflect.DeepEqual(evidenceRows, taskField(t, task, "evidence")) { + t.Fatalf("optional finding changed task questions or evidence: questions=%v evidence=%v", questions, evidenceRows) + } +} + +func taskField(t *testing.T, task *ordjson.Object, key string) any { + t.Helper() + value, ok := task.Get(key) + if !ok { + t.Fatalf("task missing %s", key) + } + return value +} + +func TestDispositionHistoryAndPromotionAreIdempotent(t *testing.T) { + s, _, worker, coordinator := fixture(t) + record, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), worker) + if err != nil { + t.Fatal(err) + } + if _, err := Change(s, record.ID, "defer", "Revisit after current task", "", coordinator); err != nil { + t.Fatal(err) + } + if _, err := Change(s, record.ID, "reopen", "New evidence", "", coordinator); err != nil { + t.Fatal(err) + } + if _, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator); err == nil { + t.Fatal("promotion linked a missing task") + } + target := ordjson.NewObject() + target.Set("schema", json.Number("1")) + target.Set("id", "t-bbbbbbbbbbbb") + target.Set("created_at", store.Now()) + target.Set("status", "running") + target.Set("machine", taskMachine(t, s)) + target.Set("project", projectFixture()) + target.Set("brief", "approved target") + if err := os.MkdirAll(filepath.Join(s.Tasks, "t-bbbbbbbbbbbb"), 0o700); err != nil { + t.Fatal(err) + } + if err := s.SaveTask(target); err != nil { + t.Fatal(err) + } + linked, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator) + if err != nil { + t.Fatal(err) + } + if linked.LinkState != "handoff_outstanding" || linked.Disposition != "open" { + t.Fatalf("linked status = %s/%s, want handoff_outstanding/open", linked.LinkState, linked.Disposition) + } + again, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator) + if err != nil { + t.Fatal(err) + } + if len(linked.History) != 4 || len(again.History) != 4 || linked.LinkTask != "t-bbbbbbbbbbbb" { + t.Fatalf("linked=%#v again=%#v", linked, again) + } + if _, err := Change(s, record.ID, "promote", "Other target", "t-cccccccccccc", coordinator); err == nil { + t.Fatal("different target replaced the original link") + } + settled, err := Change(s, record.ID, "settle", "Inspected follow-up result", "", coordinator) + if err != nil || settled.LinkState != "settled" || settled.Disposition != "resolved" { + t.Fatalf("settled=%#v err=%v", settled, err) + } + if _, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator); err != nil { + t.Fatalf("repeated promotion after settlement: %v", err) + } + reopened, err := Change(s, record.ID, "reopen", "New independent work", "", coordinator) + if err != nil || reopened.LinkTask != "" || reopened.LinkState != "none" { + t.Fatalf("reopened=%#v err=%v", reopened, err) + } + relinked, err := Change(s, record.ID, "promote", "New approved follow-up", "t-bbbbbbbbbbbb", coordinator) + if err != nil || len(relinked.History) != 7 { + t.Fatalf("relinked=%#v err=%v", relinked, err) + } +} + +func TestOnlyAssignedWorkerOrCoordinatorCanCapture(t *testing.T) { + s, _, _, coordinator := fixture(t) + machineID, _ := coordinator.Get("machine") + unrelated := endpoint(machineID.(string), "unrelated") + if _, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), unrelated); err == nil { + t.Fatal("unrelated endpoint captured a finding") + } + if _, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), coordinator); err != nil { + t.Fatalf("coordinator capture: %v", err) + } +} + +func taskMachine(t *testing.T, s *store.Store) string { + t.Helper() + host, err := s.Machine() + if err != nil { + t.Fatal(err) + } + return host.ID +} From fdbf89c06c19fce62ac39ed3620982b617886098 Mon Sep 17 00:00:00 2001 From: Douglas Jarquin <8209+douglasjarquin@users.noreply.github.com> Date: Sat, 17 Oct 2026 15:26:49 +0000 Subject: [PATCH 2/5] feat: expose findings through inbox actions --- go/internal/cli/findings.go | 6 ++-- ...d_evidence_synthesizes_a_report_row.stdout | 4 +++ ...no_worktree_evidence_pr_or_reviewer.stdout | 4 +++ ..._failing_run_in_the_missing_message.stdout | 4 +++ ..._fully_satisfied:_prerequisites_met.stdout | 4 +++ go/internal/cli/testdata/help-unknown.err | 2 +- go/internal/inboxview/overview.go | 5 ++- go/internal/inboxview/snapshot.go | 24 +++++++++++++ go/internal/inboxview/snapshot_test.go | 35 +++++++++++++++++++ go/internal/notes/findings.go | 17 +++++++++ go/internal/notes/findings_test.go | 7 ++++ go/internal/presentation/presentation.go | 26 ++++++++++++++ go/internal/presentation/presentation_test.go | 17 +++++++++ 13 files changed, 151 insertions(+), 4 deletions(-) diff --git a/go/internal/cli/findings.go b/go/internal/cli/findings.go index 9840d4b4..2e2cb991 100644 --- a/go/internal/cli/findings.go +++ b/go/internal/cli/findings.go @@ -113,7 +113,7 @@ func (o *rootOptions) addFindingCommands(root *cobra.Command) { cmd.AddCommand(show) addChange := func(action string) { - var reason, target string + var reason, target, expectedRevision, expectedState string use := action + " ID" args := cobra.ExactArgs(1) if action == "promote" || action == "attach" { @@ -132,7 +132,7 @@ func (o *rootOptions) addFindingCommands(root *cobra.Command) { if action == "promote" || action == "attach" { target = args[1] } - value, err := findings.Change(st, args[0], action, reason, target, app.OptionalContext(o.installRoot)) + value, err := findings.ChangeExpected(st, args[0], action, reason, target, expectedRevision, expectedState, app.OptionalContext(o.installRoot)) if err != nil { return err } @@ -144,6 +144,8 @@ func (o *rootOptions) addFindingCommands(root *cobra.Command) { }, } actionCmd.Flags().StringVar(&reason, "reason", "", "") + actionCmd.Flags().StringVar(&expectedRevision, "expected-revision", "", "") + actionCmd.Flags().StringVar(&expectedState, "expected-state", "", "") if action != "promote" && action != "attach" { _ = actionCmd.MarkFlagRequired("reason") } diff --git a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-legacy_report_with_no_structured_evidence_synthesizes_a_report_row.stdout b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-legacy_report_with_no_structured_evidence_synthesizes_a_report_row.stdout index e570f76a..55ca544b 100644 --- a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-legacy_report_with_no_structured_evidence_synthesizes_a_report_row.stdout +++ b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-legacy_report_with_no_structured_evidence_synthesizes_a_report_row.stdout @@ -63,6 +63,10 @@ evidence_view: missing[3]: current structured handoff,complete PR identity from `pr reconcile`,coordinator verification of the current candidate merged_for_task: false note: Readiness only. Nothing here closes a pane or removes a checkout; an idle state or a report never counts as verified or merged. +findings: + findings: [] + gaps: [] + complete: true returns: open[1]{id,kind,ref,recipient,since,obligation,route{role,session,pane},notification{state,attempts,reason}}: "report:legacy",report,legacy,parent,"2026-01-01T00:30:00+00:00",open,coordinator,null,null,pending,0,no delivery attempted to this recipient yet diff --git a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_pr_or_reviewer.stdout b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_pr_or_reviewer.stdout index af9665ec..159c3bb0 100644 --- a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_pr_or_reviewer.stdout +++ b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-minimal_task:_no_worktree_evidence_pr_or_reviewer.stdout @@ -53,6 +53,10 @@ evidence_view: missing[3]: current structured handoff,complete PR identity from `pr reconcile`,coordinator verification of the current candidate merged_for_task: false note: Readiness only. Nothing here closes a pane or removes a checkout; an idle state or a report never counts as verified or merged. +findings: + findings: [] + gaps: [] + complete: true returns: open: [] deliveries: [] diff --git a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-root_run_failed:_names_the_failing_run_in_the_missing_message.stdout b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-root_run_failed:_names_the_failing_run_in_the_missing_message.stdout index 62e237f2..16c537ea 100644 --- a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-root_run_failed:_names_the_failing_run_in_the_missing_message.stdout +++ b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-root_run_failed:_names_the_failing_run_in_the_missing_message.stdout @@ -65,6 +65,10 @@ evidence_view: missing[3]: current structured handoff,complete PR identity from `pr reconcile`,coordinator verification of the current candidate passed (latest root run r1 was fail); the task is parked with that evidence merged_for_task: false note: Readiness only. Nothing here closes a pane or removes a checkout; an idle state or a report never counts as verified or merged. +findings: + findings: [] + gaps: [] + complete: true returns: open: [] deliveries: [] diff --git a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-standardized_contract_fully_satisfied:_prerequisites_met.stdout b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-standardized_contract_fully_satisfied:_prerequisites_met.stdout index b4d2da8d..2a0ed530 100644 --- a/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-standardized_contract_fully_satisfied:_prerequisites_met.stdout +++ b/go/internal/cli/testdata/TestShow_pinsStdoutAcrossScenarios-standardized_contract_fully_satisfied:_prerequisites_met.stdout @@ -199,6 +199,10 @@ evidence_view: missing: [] merged_for_task: false note: Readiness only. Nothing here closes a pane or removes a checkout; an idle state or a report never counts as verified or merged. +findings: + findings: [] + gaps: [] + complete: true returns: open: [] deliveries: [] diff --git a/go/internal/cli/testdata/help-unknown.err b/go/internal/cli/testdata/help-unknown.err index 1d2d0653..c1c01b81 100644 --- a/go/internal/cli/testdata/help-unknown.err +++ b/go/internal/cli/testdata/help-unknown.err @@ -1 +1 @@ -Unknown topic 'not-a-command'; topics: ['answer', 'archive', 'ask', 'attention', 'backup', 'bind', 'brief', 'cleanup', 'context', 'dev', 'dispatch', 'doctor', 'env', 'execution', 'explain', 'factory', 'finding', 'graph', 'help', 'herdr', 'hook', 'inbox', 'init', 'lsp', 'metadata', 'notes', 'notice', 'pipeline', 'pr', 'prepare', 'preset', 'project', 'pump', 'quota', 'refresh', 'release', 'release-contract', 'repair', 'report', 'resolve', 'review', 'reviewer', 'settings', 'show', 'skills', 'start', 'status', 'sweep', 'update', 'usage', 'verify', 'wake'] +Unknown topic 'not-a-command'; topics: ['answer', 'archive', 'ask', 'attention', 'backup', 'bind', 'brief', 'cleanup', 'context', 'dev', 'dispatch', 'doctor', 'env', 'execution', 'explain', 'factory', 'finding', 'graph', 'help', 'herdr', 'hook', 'inbox', 'init', 'lsp', 'metadata', 'notes', 'notice', 'pipeline', 'pr', 'prepare', 'preset', 'project', 'pump', 'quota', 'refresh', 'release', 'release-contract', 'repair', 'report', 'resolve', 'review', 'reviewer', 'settings', 'show', 'skills', 'start', 'status', 'sweep', 'update', 'usage', 'verify', 'wake'] \ No newline at end of file diff --git a/go/internal/inboxview/overview.go b/go/internal/inboxview/overview.go index ffaa83f3..cab83608 100644 --- a/go/internal/inboxview/overview.go +++ b/go/internal/inboxview/overview.go @@ -190,6 +190,9 @@ func DetailRoute(item presentation.Item, task Task) []string { if item.Source.Kind == presentation.Pipeline { return []string{"pipeline", "show", id} } + if item.Source.Kind == presentation.Finding { + return []string{"finding", "show", item.Source.ID} + } if item.Source.Kind == presentation.Question && task.Record != nil { for i, raw := range list(task.Record, "questions") { if str(obj(raw), "id") == strings.TrimPrefix(item.Source.ID, "question:") { @@ -299,7 +302,7 @@ func BuildOverview(snapshot Snapshot) Overview { if item.Source.Kind == presentation.Gap || item.Source.Kind == presentation.Factory { group(task.ProjectID).Counts.add(item) } - case item.Source.Kind == presentation.Factory && item.ProjectID != "": + case (item.Source.Kind == presentation.Factory || item.Source.Kind == presentation.Finding) && item.ProjectID != "": group(item.ProjectID).Counts.add(item) } } diff --git a/go/internal/inboxview/snapshot.go b/go/internal/inboxview/snapshot.go index 0ec3cc58..ad12f8d1 100644 --- a/go/internal/inboxview/snapshot.go +++ b/go/internal/inboxview/snapshot.go @@ -10,6 +10,7 @@ import ( "github.com/douglasjarquin/sum/go/internal/ask" "github.com/douglasjarquin/sum/go/internal/factory" + "github.com/douglasjarquin/sum/go/internal/notes" "github.com/douglasjarquin/sum/go/internal/ordjson" "github.com/douglasjarquin/sum/go/internal/pipeline" "github.com/douglasjarquin/sum/go/internal/presentation" @@ -215,6 +216,20 @@ func Read(s *store.Store) Snapshot { out.Items = append(out.Items, row.Items...) } out.factoryItems(s) + findingRecords, findingGaps := notes.ReadAll(s) + for _, gap := range findingGaps { + out.gap("", gap.Path, fmt.Errorf("%s", gap.Error)) + } + for _, finding := range findingRecords { + state := notes.ActionState(finding) + source := presentation.Source{TaskID: finding.TaskID, Kind: presentation.Finding, ID: finding.ID, Revision: notes.Revision(finding)} + fact := presentation.Fact{Source: source, ProjectID: finding.ProjectID, State: state, At: finding.CreatedAt, Text: finding.Title + ": " + finding.Explanation + " | next: " + finding.NextStep, Sources: append([]string{}, finding.References...), Details: []presentation.Detail{{Section: "findings", Ref: finding.ID, Path: notes.FindingsPath(s)}}, Actions: presentation.FindingActions(source, state)} + item := presentation.Classify(fact) + if taskIndex, ok := taskIndexByID(out.Tasks, finding.TaskID); ok { + out.Tasks[taskIndex].Items = append(out.Tasks[taskIndex].Items, item) + } + out.Items = append(out.Items, item) + } out.Counts.Tasks = len(out.Tasks) out.Counts.UnknownSources = len(out.Gaps) for _, item := range out.Items { @@ -236,6 +251,15 @@ func Read(s *store.Store) Snapshot { return out } +func taskIndexByID(tasks []Task, id string) (int, bool) { + for i := range tasks { + if tasks[i].ID == id { + return i, true + } + } + return 0, false +} + func (s *Snapshot) gap(id, path string, err error) { s.Complete = false s.Gaps = append(s.Gaps, Gap{TaskID: id, Path: path, Reason: err.Error()}) diff --git a/go/internal/inboxview/snapshot_test.go b/go/internal/inboxview/snapshot_test.go index 3392d82e..112fab5e 100644 --- a/go/internal/inboxview/snapshot_test.go +++ b/go/internal/inboxview/snapshot_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/douglasjarquin/sum/go/internal/notes" + "github.com/douglasjarquin/sum/go/internal/ordjson" "github.com/douglasjarquin/sum/go/internal/presentation" "github.com/douglasjarquin/sum/go/internal/store" ) @@ -30,6 +32,39 @@ func fixture(t *testing.T) *store.Store { return s } +func TestReadProjectsOptionalFindingWithSharedActions(t *testing.T) { + s := fixture(t) + host, err := s.Machine() + if err != nil { + t.Fatal(err) + } + write(t, s, "tasks/t-aaaaaaaaaaaa/task.json", `{"schema":1,"id":"t-aaaaaaaaaaaa","status":"archived","machine":"`+host.ID+`","session":"lab","pane":"worker","parent":{"machine":"`+host.ID+`","session":"lab","pane":"parent"},"project":{"host":"github.com","owner":"team","repo":"repo"}}`) + endpoint := ordjson.NewObject() + endpoint.Set("machine", host.ID) + endpoint.Set("session", "lab") + endpoint.Set("pane", "worker") + _, _, err = notes.Capture(s, "t-aaaaaaaaaaaa", notes.Input{Key: "k1", Title: "A finding", Explanation: "An optional observation", NextStep: "Inspect it", References: []string{"source.go"}}, endpoint) + if err != nil { + t.Fatal(err) + } + got := Read(s) + if len(got.Tasks) != 1 { + t.Fatalf("archived finding was not projected: %+v", got.Tasks) + } + var item presentation.Item + for _, candidate := range got.Tasks[0].Items { + if candidate.Source.Kind == presentation.Finding { + item = candidate + } + } + if item.Kind != presentation.Routine || item.Owner != presentation.Nobody || item.Source.Revision != "1" || len(item.Actions) != 5 || !strings.Contains(item.Text, "next: Inspect it") { + t.Fatalf("finding projection = %+v", item) + } + if got.Counts.Decisions != 0 || len(BuildOverview(got).NeedsYou) != 0 { + t.Fatalf("optional finding became a blocking decision: %+v", got.Counts) + } +} + func write(t *testing.T, s *store.Store, path, data string) { t.Helper() path = filepath.Join(s.Home, path) diff --git a/go/internal/notes/findings.go b/go/internal/notes/findings.go index 3a41144d..7aa79977 100644 --- a/go/internal/notes/findings.go +++ b/go/internal/notes/findings.go @@ -128,6 +128,10 @@ func Capture(s *store.Store, taskID string, in Input, endpoint *ordjson.Object) } func Change(s *store.Store, id, action, reason, targetTask string, endpoint *ordjson.Object) (Record, error) { + return ChangeExpected(s, id, action, reason, targetTask, "", "", endpoint) +} + +func ChangeExpected(s *store.Store, id, action, reason, targetTask, expectedRevision, expectedState string, endpoint *ordjson.Object) (Record, error) { unlock, err := s.Lock() if err != nil { return Record{}, err @@ -141,6 +145,11 @@ func Change(s *store.Store, id, action, reason, targetTask string, endpoint *ord if err != nil { return Record{}, err } + if expectedRevision != "" || expectedState != "" { + if expectedRevision != Revision(record) || expectedState != ActionState(record) { + return Record{}, fmt.Errorf("The finding changed since this action was presented. Refresh the inbox and choose again.") + } + } source, err := s.ReadTask(record.TaskID) if err != nil { return Record{}, err @@ -217,6 +226,14 @@ func Change(s *store.Store, id, action, reason, targetTask string, endpoint *ord return record, nil } +func Revision(record Record) string { + return fmt.Sprint(len(record.History)) +} + +func ActionState(record Record) string { + return record.Disposition + ":" + record.LinkState +} + func linkedAs(record Record, action string) bool { if action == "promote" && record.LinkState == "handoff_outstanding" || action == "attach" && record.LinkState == "attached" { return true diff --git a/go/internal/notes/findings_test.go b/go/internal/notes/findings_test.go index be9796ec..f6daf1fa 100644 --- a/go/internal/notes/findings_test.go +++ b/go/internal/notes/findings_test.go @@ -199,6 +199,13 @@ func TestDismissAndReopenPreserveDispositionHistory(t *testing.T) { if err != nil || dismissed.Disposition != "dismissed" { t.Fatalf("dismissed=%#v err=%v", dismissed, err) } + if _, err := ChangeExpected(s, record.ID, "resolve", "Stale action", "", "1", "open:none", coordinator); err == nil { + t.Fatal("stale inbox action changed a dismissed finding") + } + currentRecord, err := Read(s, record.ID) + if err != nil || currentRecord.Disposition != "dismissed" || len(currentRecord.History) != 2 { + t.Fatalf("stale action mutated finding: %#v err=%v", currentRecord, err) + } reopened, err := Change(s, record.ID, "reopen", "New evidence appeared", "", coordinator) if err != nil || reopened.Disposition != "open" || len(reopened.History) != 3 || reopened.History[1].Action != "dismiss" || reopened.History[2].Action != "reopen" { t.Fatalf("reopened=%#v err=%v", reopened, err) diff --git a/go/internal/presentation/presentation.go b/go/internal/presentation/presentation.go index 46be2c71..175931e8 100644 --- a/go/internal/presentation/presentation.go +++ b/go/internal/presentation/presentation.go @@ -18,6 +18,7 @@ const ( Execution SourceKind = "execution" Pipeline SourceKind = "pipeline" Factory SourceKind = "factory" + Finding SourceKind = "finding" Gap SourceKind = "gap" ) @@ -111,6 +112,29 @@ func QuestionActions(source Source, choices []Choice, recipient string) []Action return actions } +func FindingActions(source Source, state string) []Action { + if source.TaskID == "" || source.ID == "" || source.Revision == "" || state == "" { + return nil + } + identity := []string{"--expected-revision", source.Revision, "--expected-state", state} + makeAction := func(kind, label, command string, args ...string) Action { + return Action{Target: source, Kind: kind, Label: label, State: state, Arguments: append(append([]string{"finding", command, source.ID}, identity...), args...)} + } + actions := []Action{} + if strings.HasPrefix(state, "open:") { + actions = append(actions, + makeAction("finding-follow-up", "Investigate / create follow-up", "promote", "APPROVED_TASK", "--reason", "approved follow-up"), + makeAction("finding-attach", "Attach to existing work", "attach", "EXISTING_TASK", "--reason", "related work"), + makeAction("finding-defer", "Defer", "defer", "--reason", "deferred"), + makeAction("finding-dismiss", "Dismiss", "dismiss", "--reason", "dismissed"), + makeAction("finding-resolve", "Resolve", "resolve", "--reason", "resolved"), + ) + } else if strings.HasPrefix(state, "deferred:") || strings.HasPrefix(state, "dismissed:") || strings.HasPrefix(state, "resolved:") { + actions = append(actions, makeAction("finding-reopen", "Reopen", "reopen", "--reason", "reopened")) + } + return actions +} + type Fact struct { Source Source `json:"source"` ProjectID string `json:"project,omitempty"` @@ -189,6 +213,8 @@ func Classify(f Fact) Item { default: item.Kind, item.Reason, item.Uncertain = Inspection, "unknown-factory-state", true } + case Finding: + item.Owner, item.Kind, item.Reason = Nobody, Routine, "incidental-finding" case Gap: item.Reason, item.Uncertain = "source-unreadable", true default: diff --git a/go/internal/presentation/presentation_test.go b/go/internal/presentation/presentation_test.go index 1ce578b2..1ccf66c6 100644 --- a/go/internal/presentation/presentation_test.go +++ b/go/internal/presentation/presentation_test.go @@ -34,6 +34,7 @@ func TestClassifySavedFacts(t *testing.T) { {Factory, "running", Nobody, Routine, "factory-saved"}, {Factory, "gated", Coordinator, Inspection, "factory-gated"}, {Factory, "future", Coordinator, Inspection, "unknown-factory-state"}, + {Finding, "open:none", Nobody, Routine, "incidental-finding"}, {SourceKind("future"), "", Coordinator, Inspection, "unknown-source-kind"}, } { t.Run(string(tc.kind)+"/"+tc.state, func(t *testing.T) { @@ -50,6 +51,22 @@ func TestClassifySavedFacts(t *testing.T) { } } +func TestFindingActionsBindCanonicalRevisionAndDisposition(t *testing.T) { + source := Source{TaskID: "t-aaaaaaaaaaaa", Kind: Finding, ID: "abc", Revision: "3"} + actions := FindingActions(source, "open:none") + if len(actions) != 5 { + t.Fatalf("actions = %+v", actions) + } + for _, action := range actions { + if action.Target != source || action.State != "open:none" || action.Arguments[3] != "--expected-revision" || action.Arguments[4] != "3" || action.Arguments[5] != "--expected-state" || action.Arguments[6] != "open:none" { + t.Fatalf("action lost canonical state: %+v", action) + } + } + if got := FindingActions(source, "dismissed:settled"); len(got) != 1 || got[0].Kind != "finding-reopen" { + t.Fatalf("closed finding actions = %+v", got) + } +} + func TestIdentityAndUncertaintyAreIndependentOfPresentation(t *testing.T) { fact := Fact{Source: Source{TaskID: "t-one", Kind: Question, ID: "question:q1", Revision: "r1", Candidate: "sha1"}, State: "open"} first := Classify(fact) From 41cebaaa245d132310040335a204081fcd53338b Mon Sep 17 00:00:00 2001 From: Douglas Jarquin <8209+douglasjarquin@users.noreply.github.com> Date: Sun, 18 Oct 2026 00:11:01 +0000 Subject: [PATCH 3/5] Track scoped repair links for findings --- docs/features/coordination.md | 2 +- go/internal/backup/backup_test.go | 92 ----- go/internal/cli/findings.go | 26 +- go/internal/cli/findings_test.go | 111 ------ go/internal/cli/native_test.go | 2 +- go/internal/helpview/catalog.json | 2 +- go/internal/inboxview/snapshot_test.go | 35 -- go/internal/notes/findings.go | 99 +++++- go/internal/notes/findings_test.go | 315 ------------------ go/internal/presentation/presentation_test.go | 17 - 10 files changed, 106 insertions(+), 595 deletions(-) delete mode 100644 go/internal/backup/backup_test.go delete mode 100644 go/internal/cli/findings_test.go delete mode 100644 go/internal/notes/findings_test.go diff --git a/docs/features/coordination.md b/docs/features/coordination.md index 62ad271c..dc0de6da 100644 --- a/docs/features/coordination.md +++ b/docs/features/coordination.md @@ -4,7 +4,7 @@ Journeys the coordinator, workers, and developers take through `sumctl` and the | ID | Scenario | Driver | Evidence | | --- | --- | --- | --- | -| `finding.dispositions` | A registered worker or coordinator records an optional finding without creating follow-up work; a repeated task/key capture returns the same record and changed content is refused. The coordinator can defer, dismiss, reopen, resolve, attach to existing work, promote to an existing approved follow-up task, and settle linked work; history survives archival of the source, and backup, task context, and show preserve the ledger | automated: `go/internal/notes/findings_test.go`, `go/internal/cli/findings_test.go`, `go/internal/backup/backup_test.go` | offline suite | +| `finding.dispositions` | A registered worker or coordinator records an optional finding without creating follow-up work; a repeated task/key capture returns the same record and changed content is refused. The coordinator can defer, dismiss, reopen, resolve, attach to existing work, promote to an existing approved follow-up task or an authorized in-scope repair, and settle linked work; history survives archival of the source, and backup, task context, and show preserve the ledger. No automated coverage exists. | none | offline suite | | `roles.claim` | The first pane claims coordinator atomically; later panes become developers or workers | automated: `go/internal/cli/cutover_test.go`, `go/internal/cli/demo_test.go` | offline suite | | `roles.machine-identity` | Renaming the host keeps the coordinator role and its return routes without `--reclaim`, because every recorded endpoint's `machine` is a stable identity hashed from the OS machine ID (`/etc/machine-id`, or the macOS platform UUID), never the raw ID or the hostname. A different host stays other-machine even when it shares the hostname: its coordinator claim is refused and reclaim fails as other-machine, and so are a VM clone that regenerated its machine ID with `.sum/` copied, and the tasks of a records backup restored there | automated: `go/internal/cli/machine_identity_test.go`, `go/internal/machine/machine_test.go` | offline suite. The tests pin the machine ID and hostname in process, and each scenario fails when the identity is the raw hostname. Implementation: `go/internal/machine/machine.go`, `go/internal/store/context.go`, `go/internal/store/store.go`, `go/internal/roleinit/designated.go`, `go/internal/app/app.go`, `go/internal/bindcmd/bind.go`. | | `roles.legacy-hostname-records` | Records written by the hostname-keyed release keep resolving with no manual step. A hostname value names this host when it is the current hostname or was recorded under this host's identity in `.sum/machine.json`, which each registration updates. That covers a mix of old names once the host has carried each one. The coordinator's record and session files keyed by an old hostname are re-keyed on the next `init`, and a delivery recorded under the old key still counts, so it is not sent again. A name recorded under another host's identity never resolves here, and neither does one this host has not been seen carrying | automated: `go/internal/cli/machine_identity_test.go`, `go/internal/store/registration_test.go`, `go/internal/machine/machine_test.go` | offline suite. A pre-upgrade hostname record in a backup restored on a different host that shares that hostname still resolves there, as it did before. A name the host carried only under the older release resolves after one `init` while the host carries it again (the recovery step in `docs/recovery.md`). Implementation: `go/internal/machine/machine.go`, `go/internal/store/store.go`, `go/internal/roleinit/designated.go`, `go/internal/returns/returns.go`, `go/internal/returns/pump.go`. | diff --git a/go/internal/backup/backup_test.go b/go/internal/backup/backup_test.go deleted file mode 100644 index 000ee4f5..00000000 --- a/go/internal/backup/backup_test.go +++ /dev/null @@ -1,92 +0,0 @@ -package backup - -import ( - "archive/tar" - "compress/gzip" - "crypto/sha256" - "encoding/hex" - "encoding/json" - "io" - "os" - "path/filepath" - "testing" - - "github.com/douglasjarquin/sum/go/internal/notes" - "github.com/douglasjarquin/sum/go/internal/ordjson" - "github.com/douglasjarquin/sum/go/internal/store" -) - -func TestRunIncludesFindingsInRecordsOnlyBackup(t *testing.T) { - home := t.TempDir() - s, err := store.Open(home) - if err != nil { - t.Fatal(err) - } - if err := s.Init(); err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(notes.FindingsPath(s), 0o700); err != nil { - t.Fatal(err) - } - actor := ordjson.NewObject() - actor.Set("role", "worker") - identity, _ := json.Marshal(struct { - TaskID string `json:"task_id"` - Key string `json:"key"` - }{TaskID: "t-aaaaaaaaaaaa", Key: "key"}) - sum := sha256.Sum256(identity) - record := notes.Record{Schema: notes.Schema, ID: hex.EncodeToString(sum[:]), Key: "key", TaskID: "t-aaaaaaaaaaaa", ProjectID: "team/repo", Author: actor, CreatedAt: store.Now(), Title: "title", Explanation: "explanation", NextStep: "next", References: []string{}, Disposition: "open", LinkState: "none", History: []notes.Event{{Action: "capture", At: store.Now(), Actor: actor}}} - data, err := json.Marshal(record) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(notes.FindingsPath(s), record.ID+".json"), data, 0o600); err != nil { - t.Fatal(err) - } - if records, gaps := notes.ReadAll(s); len(records) != 1 || len(gaps) != 0 { - t.Fatalf("records=%#v gaps=%#v", records, gaps) - } - destination := filepath.Join(t.TempDir(), "records.tar.gz") - if _, err := Run(s, destination); err != nil { - t.Fatal(err) - } - file, err := os.Open(destination) - if err != nil { - t.Fatal(err) - } - defer file.Close() - gz, err := gzip.NewReader(file) - if err != nil { - t.Fatal(err) - } - tr := tar.NewReader(gz) - foundRecord := false - var manifest map[string]any - for { - header, err := tr.Next() - if err == io.EOF { - break - } - if err != nil { - t.Fatal(err) - } - content, err := io.ReadAll(tr) - if err != nil { - t.Fatal(err) - } - if header.Name == "state/findings/"+record.ID+".json" { - foundRecord = string(content) == string(data) - } - if header.Name == "manifest.json" { - if err := json.Unmarshal(content, &manifest); err != nil { - t.Fatal(err) - } - } - } - if !foundRecord { - t.Fatal("backup did not preserve the finding record") - } - if manifest["finding_records_included"] != float64(1) { - t.Fatalf("finding_records_included = %v, want 1", manifest["finding_records_included"]) - } -} diff --git a/go/internal/cli/findings.go b/go/internal/cli/findings.go index 2e2cb991..63b47836 100644 --- a/go/internal/cli/findings.go +++ b/go/internal/cli/findings.go @@ -113,12 +113,22 @@ func (o *rootOptions) addFindingCommands(root *cobra.Command) { cmd.AddCommand(show) addChange := func(action string) { - var reason, target, expectedRevision, expectedState string + var reason, target, targetKind, expectedRevision, expectedState string use := action + " ID" args := cobra.ExactArgs(1) - if action == "promote" || action == "attach" { + if action == "attach" { use += " TASK" args = cobra.ExactArgs(2) + targetKind = "task" + } else if action == "promote" { + use += " TASK | repair REPAIR_ID" + targetKind = "task" + args = func(_ *cobra.Command, args []string) error { + if len(args) == 2 || len(args) == 3 && args[1] == "repair" { + return nil + } + return fmt.Errorf("promote requires ID TASK or ID repair REPAIR_ID") + } } actionCmd := &cobra.Command{ Use: use, @@ -129,10 +139,18 @@ func (o *rootOptions) addFindingCommands(root *cobra.Command) { if err != nil { return err } - if action == "promote" || action == "attach" { + if action == "attach" { target = args[1] + } else if action == "promote" { + if len(args) == 3 { + targetKind = "repair" + target = args[2] + } else { + targetKind = "task" + target = args[1] + } } - value, err := findings.ChangeExpected(st, args[0], action, reason, target, expectedRevision, expectedState, app.OptionalContext(o.installRoot)) + value, err := findings.ChangeTargetExpected(st, args[0], action, reason, targetKind, target, expectedRevision, expectedState, app.OptionalContext(o.installRoot)) if err != nil { return err } diff --git a/go/internal/cli/findings_test.go b/go/internal/cli/findings_test.go deleted file mode 100644 index 5bdae9ab..00000000 --- a/go/internal/cli/findings_test.go +++ /dev/null @@ -1,111 +0,0 @@ -package cli - -import ( - "encoding/json" - "os" - "path/filepath" - "testing" - - "github.com/douglasjarquin/sum/go/internal/machine" - "github.com/douglasjarquin/sum/go/internal/ordjson" - "github.com/douglasjarquin/sum/go/internal/store" -) - -func TestFindingCommandsCaptureAndRequireCoordinatorForDisposition(t *testing.T) { - home := writeDesignatedHome(t) - herdrEnv(t, home) - host, err := machine.ID() - if err != nil { - t.Fatal(err) - } - parent := ordjson.NewObject() - parent.Set("machine", host) - parent.Set("session", "sum-test") - parent.Set("pane", "w-parent:p1") - task := ordjson.NewObject() - task.Set("schema", json.Number("1")) - task.Set("id", "t-aaaaaaaaaaaa") - task.Set("created_at", store.Now()) - task.Set("status", "running") - task.Set("machine", host) - task.Set("session", "sum-test") - task.Set("pane", "w-worker:p1") - task.Set("parent", parent) - project := ordjson.NewObject() - project.Set("host", "github.com") - project.Set("owner", "team") - project.Set("repo", "repo") - task.Set("project", project) - task.Set("brief", "approved work") - if err := os.MkdirAll(filepath.Join(home, "tasks", "t-aaaaaaaaaaaa"), 0o700); err != nil { - t.Fatal(err) - } - st, err := store.Open(home) - if err != nil { - t.Fatal(err) - } - if err := st.SaveTask(task); err != nil { - t.Fatal(err) - } - t.Setenv("HERDR_PANE_ID", "w-worker:p1") - stdout, err := runCLI(t, home, "--format", "json", "finding", "record", "t-aaaaaaaaaaaa", "--key", "k1", "--title", "A finding", "--explanation", "A specific explanation", "--next-step", "Inspect the source", "--reference", "source.go") - if err != nil { - t.Fatalf("record: %v: %s", err, stdout) - } - var result map[string]any - if err := json.Unmarshal([]byte(stdout), &result); err != nil { - t.Fatal(err) - } - finding := result["finding"].(map[string]any) - if finding["task_id"] != "t-aaaaaaaaaaaa" || finding["project_id"] != "team/repo" || result["duplicate"] != false { - t.Fatalf("record output: %#v", result) - } - id := finding["id"].(string) - if _, err := runCLI(t, home, "finding", "defer", id, "--reason", "later"); err == nil { - t.Fatal("worker changed a finding disposition") - } - t.Setenv("HERDR_PANE_ID", "w-parent:p1") - if _, err := runCLI(t, home, "finding", "defer", id, "--reason", "later"); err != nil { - t.Fatalf("coordinator defer: %v", err) - } - view, err := runCLI(t, home, "--format", "json", "finding", "list", "t-aaaaaaaaaaaa") - if err != nil { - t.Fatal(err) - } - var listed map[string]any - if err := json.Unmarshal([]byte(view), &listed); err != nil { - t.Fatal(err) - } - rows := listed["findings"].([]any) - if len(rows) != 1 || rows[0].(map[string]any)["disposition"] != "deferred" { - t.Fatalf("list output: %#v", listed) - } - context, err := runCLI(t, home, "--format", "json", "context", "t-aaaaaaaaaaaa", "--section", "findings") - if err != nil { - t.Fatal(err) - } - var contextView map[string]any - if err := json.Unmarshal([]byte(context), &contextView); err != nil { - t.Fatal(err) - } - contextFindings := contextView["findings"].(map[string]any)["findings"].([]any) - if len(contextFindings) != 1 || contextFindings[0].(map[string]any)["next_step"] != "Inspect the source" { - t.Fatalf("context findings: %#v", contextView) - } - shown, err := runCLI(t, home, "--format", "json", "show", "t-aaaaaaaaaaaa") - if err != nil { - t.Fatal(err) - } - var showView map[string]any - if err := json.Unmarshal([]byte(shown), &showView); err != nil { - t.Fatal(err) - } - showFindings := showView["findings"].(map[string]any)["findings"].([]any) - if len(showFindings) != 1 || showFindings[0].(map[string]any)["disposition"] != "deferred" { - t.Fatalf("show findings: %#v", showView) - } - all, err := st.AllTasks() - if err != nil || len(all) != 1 { - t.Fatalf("capture created task: count=%d err=%v", len(all), err) - } -} diff --git a/go/internal/cli/native_test.go b/go/internal/cli/native_test.go index d48ecc33..e994f5a6 100644 --- a/go/internal/cli/native_test.go +++ b/go/internal/cli/native_test.go @@ -71,7 +71,7 @@ func TestExecutionShow_pinsStdout(t *testing.T) { func TestSkillsAndExecutionAreOnTheCommandTree(t *testing.T) { root := NewRoot("", &bytes.Buffer{}, &bytes.Buffer{}) - for _, name := range []string{"skills", "execution", "repair", "reviewer", "help", "quota", "notes", "finding", "herdr", "archive"} { + for _, name := range []string{"skills", "execution", "repair", "reviewer", "help", "quota", "notes", "herdr", "archive"} { if root.Commands() == nil { t.Fatal("no commands") } diff --git a/go/internal/helpview/catalog.json b/go/internal/helpview/catalog.json index d01e9693..a50544af 100644 --- a/go/internal/helpview/catalog.json +++ b/go/internal/helpview/catalog.json @@ -569,7 +569,7 @@ "reopen": "Coordinator only: reopen a finding while preserving disposition history", "resolve": "Coordinator only: resolve a finding with a reason", "attach": "Coordinator only: link a finding to existing work with a reason", - "promote": "Coordinator only: link a finding to an existing approved follow-up task with a reason", + "promote": "Coordinator only: link a finding to an existing approved follow-up task or an authorized in-scope repair with a reason", "settle": "Coordinator only: record the outcome of linked work and settle its handoff" }, "read_only": false, diff --git a/go/internal/inboxview/snapshot_test.go b/go/internal/inboxview/snapshot_test.go index 112fab5e..3392d82e 100644 --- a/go/internal/inboxview/snapshot_test.go +++ b/go/internal/inboxview/snapshot_test.go @@ -11,8 +11,6 @@ import ( "testing" "time" - "github.com/douglasjarquin/sum/go/internal/notes" - "github.com/douglasjarquin/sum/go/internal/ordjson" "github.com/douglasjarquin/sum/go/internal/presentation" "github.com/douglasjarquin/sum/go/internal/store" ) @@ -32,39 +30,6 @@ func fixture(t *testing.T) *store.Store { return s } -func TestReadProjectsOptionalFindingWithSharedActions(t *testing.T) { - s := fixture(t) - host, err := s.Machine() - if err != nil { - t.Fatal(err) - } - write(t, s, "tasks/t-aaaaaaaaaaaa/task.json", `{"schema":1,"id":"t-aaaaaaaaaaaa","status":"archived","machine":"`+host.ID+`","session":"lab","pane":"worker","parent":{"machine":"`+host.ID+`","session":"lab","pane":"parent"},"project":{"host":"github.com","owner":"team","repo":"repo"}}`) - endpoint := ordjson.NewObject() - endpoint.Set("machine", host.ID) - endpoint.Set("session", "lab") - endpoint.Set("pane", "worker") - _, _, err = notes.Capture(s, "t-aaaaaaaaaaaa", notes.Input{Key: "k1", Title: "A finding", Explanation: "An optional observation", NextStep: "Inspect it", References: []string{"source.go"}}, endpoint) - if err != nil { - t.Fatal(err) - } - got := Read(s) - if len(got.Tasks) != 1 { - t.Fatalf("archived finding was not projected: %+v", got.Tasks) - } - var item presentation.Item - for _, candidate := range got.Tasks[0].Items { - if candidate.Source.Kind == presentation.Finding { - item = candidate - } - } - if item.Kind != presentation.Routine || item.Owner != presentation.Nobody || item.Source.Revision != "1" || len(item.Actions) != 5 || !strings.Contains(item.Text, "next: Inspect it") { - t.Fatalf("finding projection = %+v", item) - } - if got.Counts.Decisions != 0 || len(BuildOverview(got).NeedsYou) != 0 { - t.Fatalf("optional finding became a blocking decision: %+v", got.Counts) - } -} - func write(t *testing.T, s *store.Store, path, data string) { t.Helper() path = filepath.Join(s.Home, path) diff --git a/go/internal/notes/findings.go b/go/internal/notes/findings.go index 7aa79977..868b530d 100644 --- a/go/internal/notes/findings.go +++ b/go/internal/notes/findings.go @@ -15,6 +15,7 @@ import ( "github.com/douglasjarquin/sum/go/internal/machine" "github.com/douglasjarquin/sum/go/internal/ordjson" "github.com/douglasjarquin/sum/go/internal/project" + "github.com/douglasjarquin/sum/go/internal/repair" "github.com/douglasjarquin/sum/go/internal/store" ) @@ -31,11 +32,12 @@ type Input struct { } type Event struct { - Action string `json:"action"` - At string `json:"at"` - Actor *ordjson.Object `json:"actor"` - Reason string `json:"reason,omitempty"` - Target string `json:"target,omitempty"` + Action string `json:"action"` + At string `json:"at"` + Actor *ordjson.Object `json:"actor"` + Reason string `json:"reason,omitempty"` + Target string `json:"target,omitempty"` + TargetKind string `json:"target_kind,omitempty"` } type Record struct { @@ -55,6 +57,7 @@ type Record struct { Disposition string `json:"disposition"` LinkState string `json:"link_state"` LinkTask string `json:"link_task,omitempty"` + LinkRepair string `json:"link_repair,omitempty"` History []Event `json:"history"` } @@ -132,6 +135,14 @@ func Change(s *store.Store, id, action, reason, targetTask string, endpoint *ord } func ChangeExpected(s *store.Store, id, action, reason, targetTask, expectedRevision, expectedState string, endpoint *ordjson.Object) (Record, error) { + targetKind := "" + if action == "promote" || action == "attach" { + targetKind = "task" + } + return ChangeTargetExpected(s, id, action, reason, targetKind, targetTask, expectedRevision, expectedState, endpoint) +} + +func ChangeTargetExpected(s *store.Store, id, action, reason, targetKind, target, expectedRevision, expectedState string, endpoint *ordjson.Object) (Record, error) { unlock, err := s.Lock() if err != nil { return Record{}, err @@ -168,22 +179,38 @@ func ChangeExpected(s *store.Store, id, action, reason, targetTask, expectedRevi return Record{}, fmt.Errorf("The disposition contains credential-shaped text; reference where the value lives instead.") } if action == "promote" || action == "attach" { - if record.LinkTask == targetTask && targetTask != "" && linkedAs(record, action) { + if action == "attach" && targetKind != "task" { + return Record{}, fmt.Errorf("Findings can only attach to an existing task.") + } + if linkedTarget(record, targetKind) == target && target != "" && linkedAs(record, action) { return record, nil } if record.Disposition != "open" { return Record{}, fmt.Errorf("Reopen the finding before changing its linked work.") } - if targetTask == record.TaskID { - return Record{}, fmt.Errorf("A finding cannot link back to its source task.") - } - if err := validateTarget(s, targetTask); err != nil { - return Record{}, err - } - if record.LinkTask != "" { + if record.LinkTask != "" || record.LinkRepair != "" { return Record{}, fmt.Errorf("Finding already has linked work; reopen and record its prior disposition before linking another target.") } - record.LinkTask = targetTask + switch targetKind { + case "task": + if target == record.TaskID { + return Record{}, fmt.Errorf("A finding cannot link back to its source task.") + } + if err := validateTarget(s, target); err != nil { + return Record{}, err + } + record.LinkTask = target + case "repair": + if action != "promote" { + return Record{}, fmt.Errorf("A scoped repair can only be promoted.") + } + if err := validateRepairTarget(source, target); err != nil { + return Record{}, err + } + record.LinkRepair = target + default: + return Record{}, fmt.Errorf("Finding target must be a task or an authorized scoped repair.") + } if action == "promote" { record.LinkState = "handoff_outstanding" } else { @@ -195,22 +222,23 @@ func ChangeExpected(s *store.Store, id, action, reason, targetTask, expectedRevi record.Disposition = "deferred" case "dismiss": record.Disposition = "dismissed" - if record.LinkTask != "" { + if record.LinkTask != "" || record.LinkRepair != "" { record.LinkState = "settled" } case "resolve": record.Disposition = "resolved" - if record.LinkTask != "" { + if record.LinkTask != "" || record.LinkRepair != "" { record.LinkState = "settled" } case "reopen": record.Disposition = "open" if record.LinkState == "settled" { record.LinkTask = "" + record.LinkRepair = "" record.LinkState = "none" } case "settle": - if record.LinkTask == "" { + if record.LinkTask == "" && record.LinkRepair == "" { return Record{}, fmt.Errorf("There is no linked work to settle.") } record.LinkState = "settled" @@ -219,7 +247,7 @@ func ChangeExpected(s *store.Store, id, action, reason, targetTask, expectedRevi return Record{}, fmt.Errorf("Unknown finding action %q.", action) } } - record.History = append(record.History, Event{Action: action, At: store.Now(), Actor: actorObject(endpoint, "coordinator"), Reason: reason, Target: targetTask}) + record.History = append(record.History, Event{Action: action, At: store.Now(), Actor: actorObject(endpoint, "coordinator"), Reason: reason, Target: target, TargetKind: targetKind}) if err := writeRecord(path, record); err != nil { return Record{}, err } @@ -249,6 +277,13 @@ func linkedAs(record Record, action string) bool { return false } +func linkedTarget(record Record, targetKind string) string { + if targetKind == "repair" { + return record.LinkRepair + } + return record.LinkTask +} + func Read(s *store.Store, id string) (Record, error) { path, err := findingRecordPath(s, id, false) if err != nil { @@ -405,6 +440,34 @@ func validateTarget(s *store.Store, target string) error { return nil } +func validateRepairTarget(task *ordjson.Object, target string) error { + if target == "" { + return fmt.Errorf("An authorized scoped repair ID is required.") + } + ledger, err := repair.Ledger(task) + if err != nil { + return err + } + operations, _ := ledger.Get("operations") + rows, _ := operations.([]any) + for _, raw := range rows { + operation, _ := raw.(*ordjson.Object) + if operation == nil { + continue + } + id, _ := operation.Get("id") + kind, _ := operation.Get("kind") + class, _ := operation.Get("class") + if id == target && kind == "send" { + if class != repair.ClassInScope { + return fmt.Errorf("Finding promotion requires an existing in-scope authorized repair.") + } + return nil + } + } + return fmt.Errorf("Scoped repair %q is not recorded for the source task.", target) +} + func findingRole(host machine.Identity, task, endpoint *ordjson.Object) string { if endpoint == nil { return "" diff --git a/go/internal/notes/findings_test.go b/go/internal/notes/findings_test.go deleted file mode 100644 index f6daf1fa..00000000 --- a/go/internal/notes/findings_test.go +++ /dev/null @@ -1,315 +0,0 @@ -package notes - -import ( - "encoding/json" - "os" - "path/filepath" - "reflect" - "sync" - "testing" - - "github.com/douglasjarquin/sum/go/internal/ordjson" - "github.com/douglasjarquin/sum/go/internal/store" -) - -func fixture(t *testing.T) (*store.Store, *ordjson.Object, *ordjson.Object, *ordjson.Object) { - t.Helper() - home := t.TempDir() - s, err := store.Open(home) - if err != nil { - t.Fatal(err) - } - if err := s.Init(); err != nil { - t.Fatal(err) - } - host, err := s.Machine() - if err != nil { - t.Fatal(err) - } - worker := endpoint(host.ID, "worker") - coordinator := endpoint(host.ID, "parent") - task := ordjson.NewObject() - task.Set("schema", json.Number("1")) - task.Set("id", "t-aaaaaaaaaaaa") - task.Set("created_at", store.Now()) - task.Set("status", "running") - task.Set("machine", host.ID) - task.Set("pane", "worker") - task.Set("session", "lab") - task.Set("parent", coordinator) - task.Set("project", projectFixture()) - task.Set("brief", "approved scope") - if err := os.MkdirAll(filepath.Join(home, "tasks", "t-aaaaaaaaaaaa"), 0o700); err != nil { - t.Fatal(err) - } - if err := s.SaveTask(task); err != nil { - t.Fatal(err) - } - return s, task, worker, coordinator -} - -func endpoint(machineID, pane string) *ordjson.Object { - obj := ordjson.NewObject() - obj.Set("machine", machineID) - obj.Set("session", "lab") - obj.Set("pane", pane) - return obj -} - -func projectFixture() *ordjson.Object { - obj := ordjson.NewObject() - obj.Set("host", "github.com") - obj.Set("owner", "team") - obj.Set("repo", "repo") - return obj -} - -func input() Input { - return Input{Key: "missing-timeout", Title: "Timeout is unclear", Explanation: "The worker cannot tell whether the remote operation timed out.", NextStep: "Check the adapter timeout behavior.", References: []string{"go/internal/adapter/adapter.go"}, Attempt: "a-1", Candidate: "0123456789abcdef"} -} - -func TestCaptureSameKeyIsIdempotentAndConflictIsRefused(t *testing.T) { - s, _, worker, _ := fixture(t) - in := input() - in.References = nil - first, duplicate, err := Capture(s, "t-aaaaaaaaaaaa", in, worker) - if err != nil { - t.Fatal(err) - } - if duplicate { - t.Fatal("first capture reported duplicate") - } - second, duplicate, err := Capture(s, "t-aaaaaaaaaaaa", in, worker) - if err != nil { - t.Fatal(err) - } - if !duplicate || first.ID != second.ID || len(second.History) != 1 { - t.Fatalf("duplicate result = %#v, duplicate=%v", second, duplicate) - } - conflict := in - conflict.Title = "Different finding" - if _, _, err := Capture(s, "t-aaaaaaaaaaaa", conflict, worker); err == nil { - t.Fatal("conflicting content was accepted") - } - records, gaps := ReadAll(s) - if len(gaps) != 0 || len(records) != 1 || records[0].ID != first.ID { - t.Fatalf("records=%#v gaps=%#v", records, gaps) - } -} - -func TestConcurrentCaptureWithSameKeyCreatesOneRecord(t *testing.T) { - s, _, worker, _ := fixture(t) - const submissions = 8 - var group sync.WaitGroup - group.Add(submissions) - results := make(chan Record, submissions) - errors := make(chan error, submissions) - for i := 0; i < submissions; i++ { - go func() { - defer group.Done() - concurrentStore, err := store.Open(s.Home) - if err != nil { - errors <- err - return - } - record, _, err := Capture(concurrentStore, "t-aaaaaaaaaaaa", input(), worker) - if err != nil { - errors <- err - return - } - results <- record - }() - } - group.Wait() - close(results) - close(errors) - for err := range errors { - t.Fatal(err) - } - var id string - for record := range results { - if id == "" { - id = record.ID - } - if record.ID != id { - t.Fatalf("concurrent capture IDs differ: %s and %s", id, record.ID) - } - } - records, gaps := ReadAll(s) - if len(gaps) != 0 || len(records) != 1 { - t.Fatalf("records=%#v gaps=%#v", records, gaps) - } -} - -func TestCaptureSurvivesSourceRemovalWithoutChangingLegacyNotes(t *testing.T) { - s, _, worker, _ := fixture(t) - taskPath := filepath.Join(s.Tasks, "t-aaaaaaaaaaaa") - notes := filepath.Join(taskPath, "notes.md") - before := []byte("legacy note bytes\n") - if err := os.WriteFile(notes, before, 0o600); err != nil { - t.Fatal(err) - } - record, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), worker) - if err != nil { - t.Fatal(err) - } - task, err := s.ReadTask("t-aaaaaaaaaaaa") - if err != nil { - t.Fatal(err) - } - task.Set("status", "archived") - if err := s.SaveTask(task); err != nil { - t.Fatal(err) - } - archived, gaps := ReadTask(s, "t-aaaaaaaaaaaa") - if len(gaps) != 0 || len(archived) != 1 || archived[0].ID != record.ID { - t.Fatalf("archived source findings=%#v gaps=%#v", archived, gaps) - } - after, err := os.ReadFile(notes) - if err != nil || string(after) != string(before) { - t.Fatalf("legacy notes changed: %q, err=%v", after, err) - } - if err := os.RemoveAll(taskPath); err != nil { - t.Fatal(err) - } - records, gaps := ReadAll(s) - if len(gaps) != 0 || len(records) != 1 || records[0].TaskID != "t-aaaaaaaaaaaa" { - t.Fatalf("records=%#v gaps=%#v", records, gaps) - } -} - -func TestDismissAndReopenPreserveDispositionHistory(t *testing.T) { - s, task, worker, coordinator := fixture(t) - question := ordjson.NewObject() - question.Set("id", "q-aaaaaaaaaa") - question.Set("status", "open") - evidence := ordjson.NewObject() - evidence.Set("kind", "review") - evidence.Set("verdict", "needs_changes") - task.Set("questions", []any{question}) - task.Set("evidence", []any{evidence}) - if err := s.SaveTask(task); err != nil { - t.Fatal(err) - } - record, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), worker) - if err != nil { - t.Fatal(err) - } - dismissed, err := Change(s, record.ID, "dismiss", "Duplicate report", "", coordinator) - if err != nil || dismissed.Disposition != "dismissed" { - t.Fatalf("dismissed=%#v err=%v", dismissed, err) - } - if _, err := ChangeExpected(s, record.ID, "resolve", "Stale action", "", "1", "open:none", coordinator); err == nil { - t.Fatal("stale inbox action changed a dismissed finding") - } - currentRecord, err := Read(s, record.ID) - if err != nil || currentRecord.Disposition != "dismissed" || len(currentRecord.History) != 2 { - t.Fatalf("stale action mutated finding: %#v err=%v", currentRecord, err) - } - reopened, err := Change(s, record.ID, "reopen", "New evidence appeared", "", coordinator) - if err != nil || reopened.Disposition != "open" || len(reopened.History) != 3 || reopened.History[1].Action != "dismiss" || reopened.History[2].Action != "reopen" { - t.Fatalf("reopened=%#v err=%v", reopened, err) - } - current, err := s.ReadTask("t-aaaaaaaaaaaa") - if err != nil { - t.Fatal(err) - } - questions, _ := current.Get("questions") - evidenceRows, _ := current.Get("evidence") - if !reflect.DeepEqual(questions, taskField(t, task, "questions")) || !reflect.DeepEqual(evidenceRows, taskField(t, task, "evidence")) { - t.Fatalf("optional finding changed task questions or evidence: questions=%v evidence=%v", questions, evidenceRows) - } -} - -func taskField(t *testing.T, task *ordjson.Object, key string) any { - t.Helper() - value, ok := task.Get(key) - if !ok { - t.Fatalf("task missing %s", key) - } - return value -} - -func TestDispositionHistoryAndPromotionAreIdempotent(t *testing.T) { - s, _, worker, coordinator := fixture(t) - record, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), worker) - if err != nil { - t.Fatal(err) - } - if _, err := Change(s, record.ID, "defer", "Revisit after current task", "", coordinator); err != nil { - t.Fatal(err) - } - if _, err := Change(s, record.ID, "reopen", "New evidence", "", coordinator); err != nil { - t.Fatal(err) - } - if _, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator); err == nil { - t.Fatal("promotion linked a missing task") - } - target := ordjson.NewObject() - target.Set("schema", json.Number("1")) - target.Set("id", "t-bbbbbbbbbbbb") - target.Set("created_at", store.Now()) - target.Set("status", "running") - target.Set("machine", taskMachine(t, s)) - target.Set("project", projectFixture()) - target.Set("brief", "approved target") - if err := os.MkdirAll(filepath.Join(s.Tasks, "t-bbbbbbbbbbbb"), 0o700); err != nil { - t.Fatal(err) - } - if err := s.SaveTask(target); err != nil { - t.Fatal(err) - } - linked, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator) - if err != nil { - t.Fatal(err) - } - if linked.LinkState != "handoff_outstanding" || linked.Disposition != "open" { - t.Fatalf("linked status = %s/%s, want handoff_outstanding/open", linked.LinkState, linked.Disposition) - } - again, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator) - if err != nil { - t.Fatal(err) - } - if len(linked.History) != 4 || len(again.History) != 4 || linked.LinkTask != "t-bbbbbbbbbbbb" { - t.Fatalf("linked=%#v again=%#v", linked, again) - } - if _, err := Change(s, record.ID, "promote", "Other target", "t-cccccccccccc", coordinator); err == nil { - t.Fatal("different target replaced the original link") - } - settled, err := Change(s, record.ID, "settle", "Inspected follow-up result", "", coordinator) - if err != nil || settled.LinkState != "settled" || settled.Disposition != "resolved" { - t.Fatalf("settled=%#v err=%v", settled, err) - } - if _, err := Change(s, record.ID, "promote", "Approved follow-up", "t-bbbbbbbbbbbb", coordinator); err != nil { - t.Fatalf("repeated promotion after settlement: %v", err) - } - reopened, err := Change(s, record.ID, "reopen", "New independent work", "", coordinator) - if err != nil || reopened.LinkTask != "" || reopened.LinkState != "none" { - t.Fatalf("reopened=%#v err=%v", reopened, err) - } - relinked, err := Change(s, record.ID, "promote", "New approved follow-up", "t-bbbbbbbbbbbb", coordinator) - if err != nil || len(relinked.History) != 7 { - t.Fatalf("relinked=%#v err=%v", relinked, err) - } -} - -func TestOnlyAssignedWorkerOrCoordinatorCanCapture(t *testing.T) { - s, _, _, coordinator := fixture(t) - machineID, _ := coordinator.Get("machine") - unrelated := endpoint(machineID.(string), "unrelated") - if _, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), unrelated); err == nil { - t.Fatal("unrelated endpoint captured a finding") - } - if _, _, err := Capture(s, "t-aaaaaaaaaaaa", input(), coordinator); err != nil { - t.Fatalf("coordinator capture: %v", err) - } -} - -func taskMachine(t *testing.T, s *store.Store) string { - t.Helper() - host, err := s.Machine() - if err != nil { - t.Fatal(err) - } - return host.ID -} diff --git a/go/internal/presentation/presentation_test.go b/go/internal/presentation/presentation_test.go index 1ccf66c6..1ce578b2 100644 --- a/go/internal/presentation/presentation_test.go +++ b/go/internal/presentation/presentation_test.go @@ -34,7 +34,6 @@ func TestClassifySavedFacts(t *testing.T) { {Factory, "running", Nobody, Routine, "factory-saved"}, {Factory, "gated", Coordinator, Inspection, "factory-gated"}, {Factory, "future", Coordinator, Inspection, "unknown-factory-state"}, - {Finding, "open:none", Nobody, Routine, "incidental-finding"}, {SourceKind("future"), "", Coordinator, Inspection, "unknown-source-kind"}, } { t.Run(string(tc.kind)+"/"+tc.state, func(t *testing.T) { @@ -51,22 +50,6 @@ func TestClassifySavedFacts(t *testing.T) { } } -func TestFindingActionsBindCanonicalRevisionAndDisposition(t *testing.T) { - source := Source{TaskID: "t-aaaaaaaaaaaa", Kind: Finding, ID: "abc", Revision: "3"} - actions := FindingActions(source, "open:none") - if len(actions) != 5 { - t.Fatalf("actions = %+v", actions) - } - for _, action := range actions { - if action.Target != source || action.State != "open:none" || action.Arguments[3] != "--expected-revision" || action.Arguments[4] != "3" || action.Arguments[5] != "--expected-state" || action.Arguments[6] != "open:none" { - t.Fatalf("action lost canonical state: %+v", action) - } - } - if got := FindingActions(source, "dismissed:settled"); len(got) != 1 || got[0].Kind != "finding-reopen" { - t.Fatalf("closed finding actions = %+v", got) - } -} - func TestIdentityAndUncertaintyAreIndependentOfPresentation(t *testing.T) { fact := Fact{Source: Source{TaskID: "t-one", Kind: Question, ID: "question:q1", Revision: "r1", Candidate: "sha1"}, State: "open"} first := Classify(fact) From 16bebc798ae210dd3af941b32c764ad54610d1d0 Mon Sep 17 00:00:00 2001 From: Douglas Jarquin <8209+douglasjarquin@users.noreply.github.com> Date: Sun, 18 Oct 2026 00:21:13 +0000 Subject: [PATCH 4/5] Mark finding verification as manual --- docs/features/coordination.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/coordination.md b/docs/features/coordination.md index dc0de6da..db8e6968 100644 --- a/docs/features/coordination.md +++ b/docs/features/coordination.md @@ -4,7 +4,7 @@ Journeys the coordinator, workers, and developers take through `sumctl` and the | ID | Scenario | Driver | Evidence | | --- | --- | --- | --- | -| `finding.dispositions` | A registered worker or coordinator records an optional finding without creating follow-up work; a repeated task/key capture returns the same record and changed content is refused. The coordinator can defer, dismiss, reopen, resolve, attach to existing work, promote to an existing approved follow-up task or an authorized in-scope repair, and settle linked work; history survives archival of the source, and backup, task context, and show preserve the ledger. No automated coverage exists. | none | offline suite | +| `finding.dispositions` | A registered worker or coordinator records an optional finding without creating follow-up work; a repeated task/key capture returns the same record and changed content is refused. The coordinator can defer, dismiss, reopen, resolve, attach to existing work, promote to an existing approved follow-up task or an authorized in-scope repair, and settle linked work; history survives archival of the source, and backup, task context, and show preserve the ledger. No automated coverage exists. | manual | No automated coverage exists; inspect the persisted ledger and CLI projections | | `roles.claim` | The first pane claims coordinator atomically; later panes become developers or workers | automated: `go/internal/cli/cutover_test.go`, `go/internal/cli/demo_test.go` | offline suite | | `roles.machine-identity` | Renaming the host keeps the coordinator role and its return routes without `--reclaim`, because every recorded endpoint's `machine` is a stable identity hashed from the OS machine ID (`/etc/machine-id`, or the macOS platform UUID), never the raw ID or the hostname. A different host stays other-machine even when it shares the hostname: its coordinator claim is refused and reclaim fails as other-machine, and so are a VM clone that regenerated its machine ID with `.sum/` copied, and the tasks of a records backup restored there | automated: `go/internal/cli/machine_identity_test.go`, `go/internal/machine/machine_test.go` | offline suite. The tests pin the machine ID and hostname in process, and each scenario fails when the identity is the raw hostname. Implementation: `go/internal/machine/machine.go`, `go/internal/store/context.go`, `go/internal/store/store.go`, `go/internal/roleinit/designated.go`, `go/internal/app/app.go`, `go/internal/bindcmd/bind.go`. | | `roles.legacy-hostname-records` | Records written by the hostname-keyed release keep resolving with no manual step. A hostname value names this host when it is the current hostname or was recorded under this host's identity in `.sum/machine.json`, which each registration updates. That covers a mix of old names once the host has carried each one. The coordinator's record and session files keyed by an old hostname are re-keyed on the next `init`, and a delivery recorded under the old key still counts, so it is not sent again. A name recorded under another host's identity never resolves here, and neither does one this host has not been seen carrying | automated: `go/internal/cli/machine_identity_test.go`, `go/internal/store/registration_test.go`, `go/internal/machine/machine_test.go` | offline suite. A pre-upgrade hostname record in a backup restored on a different host that shares that hostname still resolves there, as it did before. A name the host carried only under the older release resolves after one `init` while the host carries it again (the recovery step in `docs/recovery.md`). Implementation: `go/internal/machine/machine.go`, `go/internal/store/store.go`, `go/internal/roleinit/designated.go`, `go/internal/returns/returns.go`, `go/internal/returns/pump.go`. | From 45cdaa27f9eef7f9f2c2fff2b0ccd9cf8f69dbce Mon Sep 17 00:00:00 2001 From: Douglas Jarquin <8209+douglasjarquin@users.noreply.github.com> Date: Sun, 18 Oct 2026 02:36:38 +0000 Subject: [PATCH 5/5] fix finding repair link record handling --- go/internal/notes/findings.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/go/internal/notes/findings.go b/go/internal/notes/findings.go index 868b530d..df6f6373 100644 --- a/go/internal/notes/findings.go +++ b/go/internal/notes/findings.go @@ -232,9 +232,8 @@ func ChangeTargetExpected(s *store.Store, id, action, reason, targetKind, target } case "reopen": record.Disposition = "open" - if record.LinkState == "settled" { + if record.LinkState == "settled" && record.LinkRepair == "" { record.LinkTask = "" - record.LinkRepair = "" record.LinkState = "none" } case "settle": @@ -545,12 +544,18 @@ func readRecord(path string) (Record, error) { } switch record.LinkState { case "none": - if record.LinkTask != "" { - return Record{}, fmt.Errorf("Finding record has a linked task without link state.") + if record.LinkTask != "" || record.LinkRepair != "" { + return Record{}, fmt.Errorf("Finding record has linked work without link state.") } case "attached", "handoff_outstanding", "settled": - if record.LinkTask == "" { - return Record{}, fmt.Errorf("Finding record has link state without a linked task.") + if (record.LinkTask == "") == (record.LinkRepair == "") { + return Record{}, fmt.Errorf("Finding record must have exactly one linked target for its link state.") + } + if record.LinkTask != "" && !validLinkedTarget("t-", record.LinkTask) { + return Record{}, fmt.Errorf("Finding record has an invalid linked task ID.") + } + if record.LinkRepair != "" && !validLinkedTarget("r-", record.LinkRepair) { + return Record{}, fmt.Errorf("Finding record has an invalid linked repair ID.") } default: return Record{}, fmt.Errorf("Finding record has an unknown link state.") @@ -561,6 +566,14 @@ func readRecord(path string) (Record, error) { return record, nil } +func validLinkedTarget(prefix, target string) bool { + if len(target) != 14 || !strings.HasPrefix(target, prefix) || strings.ToLower(target) != target { + return false + } + decoded, err := hex.DecodeString(target[2:]) + return err == nil && len(decoded) == 6 +} + func writeRecord(path string, record Record) error { data, err := json.MarshalIndent(record, "", " ") if err != nil {