Compare commits

...
31 Commits
Author SHA1 Message Date
jb 7e7cfdb5cb docs: close develop workflow implementation 2026-09-04 15:28:17 -04:00
jb 88c20ca51b docs: record simplified workflow approval 2026-09-04 15:20:02 -04:00
jb 574adac57f docs: pin simplified develop workflow review 2026-09-04 15:17:39 -04:00
jb ebbf3c3d19 refactor(skills): simplify checkpoint locking 2026-09-04 15:16:51 -04:00
jb c257f31c21 docs: approve checkpoint simplification 2026-09-04 15:16:23 -04:00
jb 435a12107f docs: approve cooperative checkpoint locks 2026-09-04 15:13:47 -04:00
jb 26eeab209e docs: record lock remediation rejection 2026-09-04 10:59:04 -04:00
jb 4178ee4857 docs: pin exceptional remediation review 2026-09-04 10:56:06 -04:00
jb 52576c7556 fix(skills): preserve replaced checkpoint locks 2026-09-04 10:55:18 -04:00
jb 444483ade6 docs: approve focused lock remediation 2026-09-04 10:54:28 -04:00
jb ebab671969 docs: record post-rebase review finding 2026-09-04 10:51:59 -04:00
jb b556bcf147 docs: pin post-rebase develop workflow review 2026-09-04 10:47:58 -04:00
jb b4e71b47db docs: record dirty source resolution 2026-09-04 10:47:04 -04:00
jb 1782d1a0c6 docs: record final develop workflow approval 2026-09-04 10:47:04 -04:00
jb a241fb05ae docs: pin final develop workflow review 2026-09-04 10:47:04 -04:00
jb 7a58ec0eef fix(skills): remove checkpoint helper dead code 2026-09-04 10:47:04 -04:00
jb 08ebfb50ad docs: record second develop workflow review 2026-09-04 10:47:04 -04:00
jb e65ea4c0aa docs: pin remediated develop workflow review 2026-09-04 10:47:04 -04:00
jb 1e213db45a fix(skills): address develop workflow review 2026-09-04 10:47:04 -04:00
jb 1bab01799c docs: record develop workflow review findings 2026-09-04 10:47:04 -04:00
jb 353278aa96 docs: pin develop workflow review brief 2026-09-04 10:47:04 -04:00
jb f8a4362287 docs: checkpoint develop workflow verification 2026-09-04 10:47:04 -04:00
jb 12495473ee docs: supersede the legacy workflow engine 2026-09-04 10:47:04 -04:00
jb f1b525cec9 feat(skills): enforce independent workflow review 2026-09-04 10:47:04 -04:00
jb b6bb5db033 feat(subagents): add independent reviewer profile 2026-09-04 10:47:04 -04:00
jb 8c44c07c6a feat(skills): add develop workflow 2026-09-04 10:47:04 -04:00
jb a794a1e10a docs: checkpoint develop workflow implementation 2026-09-04 10:47:04 -04:00
jb 6389d22f15 docs: checkpoint develop workflow plan approval 2026-09-04 10:47:04 -04:00
jb fbf1a52b64 docs: record develop workflow implementation plan 2026-09-04 10:47:04 -04:00
jb 06e631504c docs: initialize develop workflow record 2026-09-04 10:47:04 -04:00
jb b4373113a4 chore(skills): quote artifact presenter description 2026-09-04 10:46:57 -04:00
33 changed files with 1525 additions and 94 deletions
+2 -2
View File
@@ -7,11 +7,11 @@ Personal [Pi](https://github.com/earendil-works/pi-mono) extensions, skills, glo
[`config/pi-links.json`](config/pi-links.json) is the source of truth for the normal installation. The link manager exposes:
- global agent instructions for Pi, Codex, and Claude;
- 34 shared skills under `~/.agents/skills/`;
- 35 shared skills under `~/.agents/skills/`, including the model-invoked [`develop-workflow`](skills/develop/develop-workflow/SKILL.md) skill;
- the `manage-pi-links` and `nono-profiles` project-only skills under `.pi/skills/`;
- no global Pi extension symlinks. Active extensions are installed separately as pinned Gitea npm packages.
The repository contains `copy-all`, `dictate` (Linux host-service dictation), `effort-controls`, `file-search`, `interactive-subagents`, `lm-studio`, `native-hooks`, `prompt-snippets`, `subagents`, `todo-herdr`, `update`, and the private `workflow` extension. The link manifest does not install any of them globally. The `pptx`, `stop-slop`, `subagents`, and `xlsx` skills are also intentionally excluded. Archived resources under `__archived/` are never installed. Update `config/pi-links.json` deliberately if that policy changes.
The repository contains `copy-all`, `dictate` (Linux host-service dictation), `effort-controls`, `file-search`, `interactive-subagents`, `lm-studio`, `native-hooks`, `prompt-snippets`, `subagents`, `todo-herdr`, `update`, and the private historical `workflow` extension. The link manifest does not install any extension globally. New feature work uses `develop-workflow`; the legacy extension remains unexposed as pilot evidence under [ADR 0006](docs/adr/0006-skill-led-development-workflow-04SEP2026.md). The `pptx`, `stop-slop`, `subagents`, and `xlsx` skills are intentionally excluded. Archived resources under `__archived/` are never installed. Update `config/pi-links.json` deliberately if that policy changes.
## Fresh-machine setup
@@ -1,9 +1,12 @@
---
status: accepted
status: superseded
superseded-by: 0006-skill-led-development-workflow-04SEP2026.md
---
# Build workflow around persisted policy gates
> Superseded by [ADR 0006](./0006-skill-led-development-workflow-04SEP2026.md). Retained as the legacy custom-engine decision.
`workflow` will be a new extension rather than an activation or rename of `_workflows`. It will expose one command-style tool backed by atomic revisioned snapshots, Markdown Agent Profiles, deterministic eligibility followed by planner selection, a mandatory feature pipeline, and restricted feature or read-only analysis scripts. Every feature run receives a distinct exclusive worktree. Concurrency defaults to 8 under an immutable process-wide maximum of 32. Non-trivial UI work requires at least three structurally distinct prototypes and explicit user acceptance. Before implementation, the user chooses a fresh plan-seeded session, compaction of the current session that prefers `smart_compact` and falls back to normal Pi compaction when unavailable, or unchanged current context. Review uses a fresh session and excludes every Agent Profile that mutated the reviewed diff.
This keeps the caller interface small while making approval, prototype, recovery, and reviewer-independence rules enforceable in one workflow engine. We rejected unrestricted custom graphs because they could bypass policy, a general registry architecture because it adds speculative extension points, and event-sourced persistence because revisioned snapshots already provide the required recovery and audit data with less machinery.
@@ -1,10 +1,13 @@
---
status: accepted
status: superseded
supersedes: 0001-workflow-architecture-24AUG2026.md
superseded-by: 0006-skill-led-development-workflow-04SEP2026.md
---
# Use drill-down overlays and bounded recovery
> Superseded by [ADR 0006](./0006-skill-led-development-workflow-04SEP2026.md). Retained as legacy custom-engine UI and recovery evidence.
This decision supersedes the dashboard, dirty-source, execution-timeout, and startup-ownership assumptions in [ADR 0001](./0001-workflow-architecture-24AUG2026.md). The persisted gated pipeline, worktree isolation, and independent-review decisions from ADR 0001 remain accepted.
## Context
@@ -1,3 +1,5 @@
# Reuse standard agents through workflow role overlays
> Superseded by [ADR 0006](./0006-skill-led-development-workflow-04SEP2026.md). Retained as the legacy custom-engine agent-selection decision.
`workflow` will reuse standard `pi-interactive-subagents` definitions instead of duplicating their prompts and model settings in workflow-only agents. Explicit configuration maps standard agent names to workflow roles, while `workflow` retains ownership, tool-intersection, Gate, verification, and reviewer-independence policy. The default maps `scout` to Discovery and `worker` to Implementation; bundled profiles remain fallbacks for roles without a standard mapping. This keeps one source of truth for agent identity without allowing arbitrary prompt prose or agent names to imply mutability or workflow authority.
@@ -10,6 +10,8 @@ Publish packages to the `jb` owner endpoint of the Gitea npm registry. Repositor
`extensions/_workflows` is reference code, not a package or publishable extension. `@jbcode/pi-workflow` remains a private workspace until the required reference runtime is merged under `extensions/workflow`; this prevents publishing two workflow packages or publishing an incomplete package.
> **Later decision:** [ADR 0006](./0006-skill-led-development-workflow-04SEP2026.md) supersedes the workflow-specific merge and release direction above. Keep the legacy workspace private, unpublished, and unexposed during the skill pilot.
The existing `config/pi-links.json` policy remains the authority for live-development exposure. Package boundaries and publication do not automatically enable extensions globally.
## Consequences
@@ -23,3 +23,5 @@ Rename the private `pi-workflow` extension identity to `workflow`. Its workspace
- Upstream movement is detected but never merged automatically; imported changes require reconciliation with monorepo-specific manifests and APIs.
- Only `main` is tracked. Source tags, branches, and historical commits are not copied.
- The renamed `workflow` extension remains private until its `_workflows` dependencies are merged and isolated packaging succeeds.
> **Later decision:** [ADR 0006](./0006-skill-led-development-workflow-04SEP2026.md) supersedes this workflow-specific merge and release direction with the skill pilot and a later retirement decision.
@@ -0,0 +1,38 @@
---
status: accepted
supersedes:
- 0001-workflow-architecture-24AUG2026.md
- 0002-workflow-interaction-and-recovery-25AUG2026.md
- 0002-workflow-standard-agent-overlays-27AUG2026.md
---
# Replace the custom workflow engine with a skill-led workflow
## Context
The custom `workflow` extension encoded useful approval, isolation, verification, and reviewer-independence policy, but coupled that policy to a large state machine, child runtime, scheduler, persistence layer, and TUI. Recorded runs failed or stalled before implementation when exact planner output, child delivery, recovery state, or liveness mechanics diverged from the engine contract.
Pi already provides the useful orchestration primitives: `ask_user`, task tools, direct subagents, artifact presentation, prototypes, browser/application validation, project commands, and Git worktrees. Repository Markdown and commits provide a simpler recovery boundary than a second workflow runtime.
## Decision
Use the model-invoked [`develop-workflow` skill](../../skills/develop/develop-workflow/SKILL.md) for feature work and other multi-step changes. Keep questions and obvious low-risk edits on lightweight paths. Add no replacement extension.
The parent agent owns classification, requirements, the retained workflow record, task state, worktree, assignments, verification, commits, review, and local integration. Preserve categorical human gates for the exact implementation plan, non-trivial UI direction, and just-in-time risk. Require observable evidence rather than exact model-authored transition schemas.
Keep one committed Markdown workflow record per active change. Update it atomically at meaningful checkpoints with optimistic SHA-256 concurrency and retain it after completion. Treat Git and the last committed valid record as authoritative recovery evidence.
Use bounded subagent assignments with exclusive file ownership and before/after Git evidence. Exclude every mutator identity from independent standards and specification review. Require Git-capable read-only reviewer profiles to regenerate and confirm the pinned binary diff fingerprint. Reviewers report findings but never remediate them.
Automate verified commits, rebase, fresh review, clean local fast-forward integration, metadata-only closure, and safe cleanup. Keep destructive actions, force operations, conflict-driven behavior changes, pushes, publishing, deployment, and production access behind explicit approval.
Retain `extensions/workflow` and `extensions/_workflows` as private historical reference during the pilot. Keep them unexposed. Archive or remove them only in a separate change after three successful real-work pilots and explicit retirement approval.
## Consequences
- Workflow gates become procedural and testable, not technically enforced by a custom state machine.
- Recovery depends on repository evidence and proven child liveness rather than leases or automatic replay.
- The skill composes existing Pi capabilities and adds no runtime dependency.
- A Git-capable reviewer profile declares no edit/write tools, while parent before/after Git checks remain necessary because shell access can mutate indirectly.
- Existing package and imported-snapshot ADRs remain valid except for workflow-extension-specific prerequisites, which this decision supersedes.
- The twelve scenario evaluations and three real-change pilot remain required before retiring the legacy implementation or closing the replacement TODO.
+15 -13
View File
@@ -155,7 +155,7 @@ Slices in the same row may proceed in parallel after their listed dependencies a
- one selected provider route;
- no package registries, SSH, Docker, browser, microphone, deployment credentials, or broad localhost access.
4. Separate extension source from extension state. Confirm that normal sessions cannot modify global instructions, installed extension code, authentication state, other Pi profiles, or sibling repositories.
5. Materialize the normal in-process extension allowlist as a generated, isolated Pi `settings.json` under the selected personal/work state root. Its package/extension entries come only from `config/pi-package-lock.json` and the trusted repository map. Disable `pi-ask` arbitrary notification commands, `/update`, `workflow`, background shell jobs, and any privileged direct spawn path until a controlled subprocess policy exists.
5. Materialize the normal in-process extension allowlist as a generated, isolated Pi `settings.json` under the selected personal/work state root. Its package/extension entries come only from `config/pi-package-lock.json` and the trusted repository map. Disable `pi-ask` arbitrary notification commands, `/update`, the legacy `workflow` extension, background shell jobs, and any privileged direct spawn path until a controlled subprocess policy exists. The `develop-workflow` skill adds no executable or permission boundary; it composes only tools already available to the parent session.
6. Import the unmanaged `~/.pi/agent/extensions/lm-studio.ts` implementation into a repository-managed extension at `extensions/lm-studio/index.ts`, lock its source revision, and change its fixed base URL contract to `http://127.0.0.1:1234/v1`. Maintenance moves the old extension to the quarantined state store and removes its active settings entry. A duplicate-provider test fails if the old source, another `lm-studio` registration, or `localhost` base URL remains loadable.
7. Define and own stable profile fragments for `provider-lm-studio`, `provider-openai`, `provider-anthropic`, and `provider-morph`, granting only one route at launch. Prove nono 0.74's `tcp_connect_ports` behavior with real loopback fixtures listening on ports 1234 and 1235: the selected provider must connect to 1234 while 1235 and every other loopback port fail. Do not use hostname-only `allow_domain` as evidence of port restriction. If the fixture cannot prove exact enforcement, `provider-lm-studio` remains unavailable until a separately reviewed mediated relay is designed.
@@ -165,7 +165,7 @@ Slices in the same row may proceed in parallel after their listed dependencies a
- The selected provider works while non-selected provider hosts, raw key environment names, and credentials are absent from the environment and process table.
- Reads outside the exact Pi state, declared active-worktree access, and per-launch temporary paths fail; writes to a read-only active worktree fail; another session's temporary directory is inaccessible.
- LM Studio, when selected, reaches only `127.0.0.1:1234`.
- `/update`, `workflow`, arbitrary notification commands, and unbrokered background shell jobs are unavailable.
- `/update`, the legacy `workflow` extension, arbitrary notification commands, and unbrokered background shell jobs are unavailable; skill-led development cannot widen the parent session's policy.
- The repository-managed LM Studio provider uses only the exact `127.0.0.1:1234` base URL, the old unmanaged source is quarantined, duplicate registration fails, and a real two-port fixture proves all other loopback ports are denied.
- All base and provider profiles pass strict validation and resolved-policy review.
@@ -381,32 +381,34 @@ A failed pilot returns to the owning slice. Do not compensate by broadening the
## Slice 6B1: Child-agent and background-task isolation
**Goal:** isolate spawned Pi, Claude, Codex, bounded background jobs, Herdr panes, and tmux panes without enabling repository toolchain execution or `workflow`.
**Goal:** isolate spawned Pi, Claude, Codex, bounded background jobs, Herdr panes, and tmux panes without enabling repository toolchain execution or the legacy `workflow` extension.
**Owner surfaces:** `profiles/nono/{child-pi,child-claude,child-codex,background-jobs}.json`, new spawn-wrapper sources under `scripts/nono/child-agents/`, isolated child-state configuration, and `tests/nono/capabilities/child-agents/`. Coordinate with extension owners but avoid editing Slice 6A's socket fragments or shared extension files concurrently.
**Work:**
1. Inventory every spawn path in active subagent, `pi-ask`, update, and background-task extensions. Classify each in-process operation as safe UI/state access, disabled behavior, or a brokered subprocess. Keep `workflow` absent from generated settings.
1. Inventory every spawn path in active subagent, `pi-ask`, update, and background-task extensions. Classify each in-process operation as safe UI/state access, disabled behavior, or a brokered subprocess. Keep the legacy `workflow` extension absent from generated settings.
2. Route child Pi, Claude, and Codex through command policies with isolated child state and the immutable provider selected by Slice 3.
3. Broker only named background jobs and notification commands that do not invoke repository toolchains; keep arbitrary commands, package updates, and direct privileged spawn paths disabled.
4. Define caller relationships and prohibit privileged-command escalation through child commands.
5. Verify Herdr and tmux targets launch under equivalent or narrower generated policies before enabling command delivery.
6. Add containment tests for nested children, background completion, cancellation, inherited environment, provider override denial, notification commands, package-update denial, and artifact paths.
**Acceptance criteria:** child commands receive no broader filesystem, network, credential, socket, environment, or provider access than declared; nested child tests prove containment; unsandboxed Herdr/tmux targets are rejected; bounded extension behavior remains functional without `workflow` or toolchain execution.
**Acceptance criteria:** child commands receive no broader filesystem, network, credential, socket, environment, or provider access than declared; nested child tests prove containment; unsandboxed Herdr/tmux targets are rejected; bounded extension behavior remains functional without the legacy `workflow` extension or toolchain execution.
**Dependencies:** pilot acceptance. May run in parallel with Slices 5A, 5B, and 6A.
## Slice 6B2: `workflow` and toolchain integration
## Slice 6B2: Skill-led development toolchain evidence
**Goal:** enable `workflow` only after every toolchain command it can invoke has a completed command policy.
> [ADR 0006](adr/0006-skill-led-development-workflow-04SEP2026.md) supersedes the planned `workflow` extension profile, spawn wrapper, generated-settings entry, and capability suite. Do not create those surfaces.
**Owner surfaces:** `profiles/nono/workflow.json`, workflow spawn-wrapper sources under `scripts/nono/child-agents/workflow/`, the generated extension allowlist entry, and `tests/nono/capabilities/workflow/`.
**Goal:** prove that `develop-workflow` uses only the parent session's approved Git, package-manager, verification, and child-agent command policies.
**Work:** route `workflow` executable resolution and every npm/pnpm/Bun/Python/Rust/Go/Maven/Gradle task through the completed Slice 5 policies. Include lifecycle/run scripts, nested package executables, verification commands, and cancellation. Any unmodeled executable or repository without the required toolchain fragment fails before workflow execution.
**Owner surfaces:** evidence under the existing Slice 5 and Slice 6B1 suites. The skill itself owns no nono profile, executable, wrapper, extension entry, or permission grant.
**Acceptance criteria:** adversarial workflow tasks cannot bypass toolchain policies, select another provider, invoke high-risk commands, read sibling repositories, or access raw credentials; repositories lacking a required toolchain capability do not load `workflow`.
**Work:** exercise representative skill-led assignments through the already completed toolchain and child-agent policies. Verify that model-selected commands cannot bypass executable allowlists, lifecycle/run-script containment, provider binding, repository boundaries, or cancellation. An unmodeled executable or repository without the required fragment fails exactly as it does for direct parent tool use.
**Acceptance criteria:** skill-led tasks cannot broaden parent or child policy, select another provider, invoke high-risk commands, read sibling repositories, or access raw credentials. The legacy `workflow` extension remains absent from generated settings.
**Dependencies:** Slices 5A, all applicable 5B language fragments, and Slice 6B1. It does not run in parallel with the toolchain policy it consumes.
@@ -479,14 +481,14 @@ To keep parallel work mergeable:
| Base filesystem | Active registered worktree has exactly its mapped read-only or read/write access; exact mutable Pi state and the per-launch temporary directory are writable | A read-only worktree cannot be written; broad `/tmp`, runtime parents, other sessions, sibling or unregistered worktrees, global instructions, extension source, other Pi state, `~/.ssh`, and `~/.config/gh` are inaccessible or read-only as designed |
| Temporary lifecycle | Normal exit and cancellation remove the private runtime directory; approved active use remains within its aggregate bound | Cleanup preserves a held lease, matching PID/start time, populated cgroup, live unit, or inconclusive identity; proven crash leftovers expire, unsafe paths survive for review, and an active session cannot exhaust `/run` |
| Worktree lifecycle | Broker creates, registers, resolves, and safely removes an external clean worktree | Broad worktree-root access, age-only deletion, dirty/untracked/unpushed removal, active-worktree removal, and linked-worktree confidentiality claims fail |
| Environment/extensions | Reviewed extensions and selected mediated provider credential work | Raw credential variables, old LM Studio source, duplicate providers, `/update`, `workflow` before 6B2, arbitrary notification commands, and unbrokered shell jobs are absent |
| Environment/extensions | Reviewed extensions and selected mediated provider credential work | Raw credential variables, old LM Studio source, duplicate providers, `/update`, the legacy `workflow` extension, arbitrary notification commands, and unbrokered shell jobs are absent |
| Provider | Trusted default or one pre-boundary approved flag selects the route; LM Studio reaches port 1234 | Ambiguous/post-boundary/child overrides, non-selected providers, LM Studio port 1235, and broad localhost fail |
| Git/SSH | Fetch/push uses the correct provisioned agent and immutable wrapper | Direct SSH, stale/substituted agent, bypass config, alternate binary, same-port host, wrong agent, wrong host, and wrong port fail |
| GitHub CLI | Active-repository operation succeeds through proxy | Real token exposure, cross-repository API, and unapproved endpoints fail |
| Package manager/workflow | Restore/build/test uses exact cache and hosts; 6B2 routes workflow tasks through completed policies | Lifecycle/run-script escape, unrelated cache, publish token, privileged command, and unapproved registry fail |
| Package manager/skill-led tasks | Restore/build/test uses exact cache and hosts; 6B2 proves `develop-workflow` cannot exceed completed parent policies | Lifecycle/run-script escape, unrelated cache, publish token, privileged command, and unapproved registry fail |
| Orca/Herdr | Authenticated current endpoint and verified socket peer succeed on launch and cache hit | Stale/replaced/wrong-service endpoint and unrelated socket/port fail |
| Dictation | Voice request succeeds with `--voice` | Socket, audio stack, model data, and GPU remain absent by default |
| Child agents | Declared bounded child workflow completes | Child cannot broaden filesystem, network, credentials, sockets, environment, or provider |
| Child agents | Declared bounded child task completes | Child cannot broaden filesystem, network, credentials, sockets, environment, or provider |
| High-risk modes | Explicit host-authorized workflow succeeds and records policy digest | Missing confirmation, conflicting flags, sandbox invocation, or another mode's capability fails |
| Generator/evidence | Strict validation, dynamic revalidation, deterministic cache hit, and trusted evidence summary succeed | Unknown or malformed inputs, stale digests/endpoints, tampered cache/evidence producer, and unsafe runtime/cache paths stop before launch |
+2 -2
View File
@@ -20,9 +20,9 @@ Captured for Slice 0 on 27AUG2026 with nono 0.74.0. This record contains no cred
| `pi-orca-local` | Symlink to `profiles/nono/pi-orca-local.json` | Approved drift imported, narrowed to read-only trusted-file access, and relinked on 27AUG2026. |
| `pi-harness-tools` | Symlink to `profiles/nono/pi-harness-tools.json` | Approved drift imported, narrowed to read-only trusted-file access, and relinked on 27AUG2026. |
The imported `pi-orca-local` drift is limited to exact read-only access for `$HOME/.pi/agent/AGENTS.md`. Its saved grant for `$HOME/.pi/agent/extensions/workflow/index.ts` was not imported because the user removed that extension and deferred its replacement to a future skill. The imported `pi-harness-tools` drift is limited to exact read-only access for `$HOME/.pi/agent/extensions/pi-todo-herdr/index.ts` and `$HOME/.config/herdr/herdr.sock`. Its inherited `pi-local` policy already contains the Herdr Unix-socket grant. The active files used read/write `allow_file` saved grants; the repository deliberately narrows them to `read_file` because normal sessions do not need to modify trusted instructions or extension code. No other active saved grant was imported.
The imported `pi-orca-local` drift is limited to exact read-only access for `$HOME/.pi/agent/AGENTS.md`. Its saved grant for `$HOME/.pi/agent/extensions/workflow/index.ts` was not imported because the user removed that extension; [ADR 0006](adr/0006-skill-led-development-workflow-04SEP2026.md) replaces it with the `develop-workflow` skill, which needs no extension-source grant. The imported `pi-harness-tools` drift is limited to exact read-only access for `$HOME/.pi/agent/extensions/pi-todo-herdr/index.ts` and `$HOME/.config/herdr/herdr.sock`. Its inherited `pi-local` policy already contains the Herdr Unix-socket grant. The active files used read/write `allow_file` saved grants; the repository deliberately narrows them to `read_file` because normal sessions do not need to modify trusted instructions or extension code. No other active saved grant was imported.
The `workflow` extension was unlinked from active Pi state by the user during this slice. It is also absent from normal `config/pi-links.json` exposure so reconciliation will not recreate it. The future replacement is tracked in `docs/todos.md`.
The legacy `workflow` extension was unlinked from active Pi state by the user during this slice. It remains absent from normal `config/pi-links.json` exposure so reconciliation will not recreate it. The `develop-workflow` skill replacement and its remaining pilot are tracked in `docs/todos.md`.
## Draft disposition
+1 -1
View File
@@ -42,7 +42,7 @@ bun publish
Publish `@jbcode/pi-shared` before a new version of any extension that depends on it. Gitea rejects republishing an existing name/version, so bump the changed package version first.
`extensions/workflow` is intentionally private until the `_workflows` reference dependencies are merged into that package. `extensions/_workflows` must not be published.
`extensions/workflow` and `extensions/_workflows` are private historical evidence and must not be published. [ADR 0006](adr/0006-skill-led-development-workflow-04SEP2026.md) supersedes the proposed runtime merge; new development uses the `develop-workflow` skill.
## Install with Pi
+2
View File
@@ -1,3 +1,5 @@
# TODO
> This item applies only to the private legacy workflow runtime retained during the [`develop-workflow` pilot](adr/0006-skill-led-development-workflow-04SEP2026.md). Mark it obsolete when that runtime is retired unless `GitStateProbe` is intentionally retained.
- [ ] Make workflow verification tolerate inaccessible sibling Git worktrees. `GitStateProbe.captureRepository()` currently enumerates every path from `git worktree list --porcelain` and fails the verification command before execution when any unrelated worktree cannot be inspected. Preserve detection of cross-worktree mutations without making unrelated worktrees a hard verification dependency. Add focused coverage for an inaccessible registered sibling worktree.
+3 -3
View File
@@ -8,9 +8,9 @@ During the Orca polling fix on 02SEP2026, Bash tool calls configured with `cwd`
Publishing `@jbcode/pi-interactive-subagents@3.7.4` exposed a Pi package-manager failure when `~/.pi` resolves through `/home/jb/data/.home-dotdirs/.pi`. Pi 0.84.4 invoked npm 12.0.2 with `--prefix /home/jb/.pi/agent/npm` from the repository cwd. npm rewrote the lockfile root as `file:../../../data/.home-dotdirs/.pi/agent/npm`, recursively duplicated that path until the lock contained 12,576 entries and 18 MB of data, and then failed with `ENOENT` during reification. Canonicalizing the npm root, regenerating both lockfiles from that directory, and installing once restored the 124-entry lock and all configured packages. Update Pi's npm package execution to use the canonical install root as its cwd, add a symlinked-agent-directory regression test, and ensure interrupted installs terminate their package-manager children before another reconciliation begins.
## Merge `_workflows` reference runtime into `workflow`
## Archive the legacy workflow runtime after the skill pilot
`extensions/_workflows` was retained only as a reference while `extensions/workflow` was developed, but `workflow` still imports its runner, model, prompt, and serialization modules. Move the required code under `extensions/workflow`, remove obsolete model-authored workflow surfaces, update tests and documentation, and then delete the reference directory. Until that work is complete, `@jbcode/pi-workflow` must remain a private Bun workspace and `extensions/_workflows` must remain excluded from workspaces and publication. Acceptance requires `bun pm pack` for `workflow` to contain every runtime import without reaching outside the package root, followed by an isolated Pi package smoke test.
[ADR 0006](adr/0006-skill-led-development-workflow-04SEP2026.md) supersedes the planned merge of `extensions/_workflows` into `extensions/workflow`. Keep both as private, unpublished, unexposed historical evidence while the `develop-workflow` skill completes scenario evaluation and three real-change pilots. After explicit retirement approval, archive or remove both implementations and their tests in a separate cleanup change; do not merge or release the legacy runtime first.
## Make `pi-links` checks work from linked worktrees
@@ -22,7 +22,7 @@ Slice 1 found two Nono 0.74 limitations while building behavioral policy proofs.
## Replace `workflow` with a skill-based orchestrator
The strict `workflow` extension repeatedly failed before implementation because planner output and persisted state-machine constraints were too brittle for adaptive subagent orchestration. In a separate session, design a repository-managed skill where the main Pi agent owns the plan, task tree, isolated worktree, explicit user approval gates, verification, and independent review while launching standard `scout` and `worker` subagents directly. Preserve file-ownership boundaries, no inferred approvals, review independence, and resumable repository evidence, but prefer native Pi primitives over a custom orchestration engine. Evaluate a small committed state manifest for crash recovery before retiring or archiving the extension. Use the abandoned `.worktrees/workflow-plan-seed` implementation only as reference; do not integrate it by default.
The strict `workflow` extension repeatedly failed before implementation because planner output and persisted state-machine constraints were too brittle for adaptive subagent orchestration. Implement the repository-managed `develop-workflow` skill under [ADR 0006](adr/0006-skill-led-development-workflow-04SEP2026.md), where the parent owns the plan, task tree, isolated worktree, explicit approval gates, verification, and independent review while launching bounded subagents directly. Preserve file ownership, reviewer independence, and resumable repository evidence while preferring native Pi primitives. Keep this TODO open until all twelve fresh-session scenarios and three real-change pilots pass; only then request retirement approval.
## Support finished-session resume for Claude CLI subagents
+2
View File
@@ -1,5 +1,7 @@
# workflow
> **Historical reference, superseded for new development:** This document describes the legacy `workflow` extension and custom engine. New feature work uses the model-invoked [`develop-workflow` skill](../../skills/develop/develop-workflow/SKILL.md) under [ADR 0006](../adr/0006-skill-led-development-workflow-04SEP2026.md). Retain this document only as pilot history.
`workflow` plans and implements feature work through persisted human Gates, isolated Git worktrees, focused verification, and independent review.
## Start a feature
+3 -1
View File
@@ -1,6 +1,8 @@
# workflow popup prototypes
**Status:** Direction C accepted
> **Historical reference, superseded for new development:** These mocks describe the legacy `workflow` extension and custom TUI. New feature work uses the model-invoked [`develop-workflow` skill](../../skills/develop/develop-workflow/SKILL.md) under [ADR 0006](../adr/0006-skill-led-development-workflow-04SEP2026.md). Retain them as prototype evidence.
**Status:** Historical direction C evidence
These static terminal frames compare popup directions for `/workflow`. They are prototype evidence, not production screenshots or manual validation of the implemented extension. All variants use Pi theme tokens, keyboard-visible focus, one border at most, and no animation.
+3 -1
View File
@@ -1,6 +1,8 @@
# workflow Design
**Status:** Accepted
> **Historical reference, superseded for new development:** This design describes the legacy `workflow` extension and custom engine. New feature work uses the model-invoked [`develop-workflow` skill](../../skills/develop/develop-workflow/SKILL.md) under [ADR 0006](../adr/0006-skill-led-development-workflow-04SEP2026.md). Retain this document only as pilot history.
**Status:** Superseded
**Date:** 24AUG2026
## Purpose
+254
View File
@@ -0,0 +1,254 @@
# Develop workflow implementation record
## Status
- Status: complete
- Current phase: closed
- Next safe action: run `/reload` in active Pi sessions; execute the deferred scenario evaluation and three-change pilot as separate follow-on work.
## Request and scope
Implement the accepted [`skill-led-development-workflow-plan.md`](./skill-led-development-workflow-plan.md) as a production-ready Slice 1 and Slice 2 change: the model-invoked skill, retained record contract, atomic checkpoint helper, reviewer capability, review/recovery integration, managed exposure evidence, ADR, and historical documentation.
Scenario evaluation and the three-change pilot remain follow-on validation because they require fresh interactive Pi sessions and independent real changes. Their TODO stays open. Out of scope here: a replacement extension, deletion or archival of the legacy engine, global extension installation, pushing, publishing, deployment, and production access.
## Git identity
- Source branch: `main`
- Base commit: `27cbe965bbd3dc0ca38814811b4f56519a570957`
- Feature branch: `feature/develop-workflow`
- Worktree: `/srv/data/code/jb-pi/harness-tools/.worktrees/develop-workflow`
- Initial record commit: `256f001`
- Omitted source change: uncommitted YAML quoting in `skills/document/artifact-presenter/SKILL.md`; the user approved omission on 03SEP2026.
## Acceptance criteria
- Pi discovers `develop-workflow` by default for features and multi-step code changes while its description excludes questions, research-only work, and trivial edits.
- Full-path edits require explicit recorded plan approval; non-trivial UI and just-in-time risk gates fail closed.
- The retained record template and helper preserve valid checkpoints across malformed input, interruption, stale/concurrent updates, and filesystem failures without dependencies.
- Every mutating assignment has exclusive named path ownership, a deadline, before/after Git evidence, and identity-based exclusion from review.
- Reviewers regenerate a pinned binary diff, confirm its SHA-256 digest, have Git-capable read-only tool declarations, and never remediate their own findings.
- Recovery, verification, rebase, local fast-forward integration, metadata-only closure, and cleanup rules are explicit and evidence-bound.
- The skill is included by repository-managed skill discovery while both legacy workflow extension exposure maps remain empty.
- Required legacy documents point to ADR 0006 and remain clearly historical during the pilot; pilot TODOs remain open.
- Focused checkpoint/link tests, typecheck, and the full repository test suite pass.
## Approved implementation plan and gates
### Gate state
- Source plan: [`skill-led-development-workflow-plan.md`](./skill-led-development-workflow-plan.md), status Accepted.
- Broad direction: approved by the user on 03SEP2026 with “let's implement”.
- Exact production plan: approved by the user on 04SEP2026.
- Dirty-source gate: approved on 03SEP2026; omit the unrelated source-checkout change.
- UI classification: no product UI change, so prototype direction approval does not apply.
- Current risk classification: no dependency, migration, production, security-posture, destructive, or compatibility action is planned. Open a scoped gate if discovery changes this.
### Slice A: checkpoint contract
Owner: `checkpoint-implementer` worker, 15-minute deadline.
Owned paths:
- `skills/develop/develop-workflow/scripts/checkpoint-record.mjs`
- `skills/develop/develop-workflow/tests/checkpoint-record.test.mjs`
Implement a Node 22 ESM helper with an exported injectable API and a strict CLI. Validate one H1 and the ten required H2 sections used by this record. For creation, require an absent destination; for replacement, require the current SHA-256 digest. Acquire a fixed exclusive per-record lock among cooperating helper invocations, re-read and hash under that lock, write a unique same-directory mode-0600 temporary file, flush it, close it, atomically rename it, and flush the containing directory. Preserve unrelated stale temporary files for recovery and fail closed on a pre-existing lock. Cleanup invocation-created paths without masking the primary failure. External lock-path mutation is outside the cooperative serialization boundary.
Cover valid create/replace, missing/malformed/duplicate sections, replacement without a digest, stale digest, concurrent writers, interrupted write/flush, rename failure, directory-flush failure, stale temporary artifacts, stale lock, and cleanup behavior that preserves the last valid target byte-for-byte.
### Slice B: skill and retained-record contract
Owner: parent agent, 15-minute implementation checkpoint.
Owned paths:
- `skills/develop/develop-workflow/SKILL.md`
- `skills/develop/develop-workflow/references/evidence-and-recovery.md`
- `skills/develop/develop-workflow/references/git-and-integration.md`
- `skills/develop/develop-workflow/references/review-and-remediation.md`
- `skills/develop/develop-workflow/assets/workflow-record.md`
Write a compact verb-first model-invoked skill. Keep path classification, preflight, phase order, human gates, ownership, and completion criteria in `SKILL.md`; disclose detailed Git/integration, recovery/evidence, and review/remediation rules through branch-specific pointers. The template uses the validated ten-section contract and concise evidence fields. Align direct subagent launch, task tracking, artifact presentation, prototype capture, realistic verification, known callback/cwd defects, print/JSON fail-closed behavior, and parent-owned orchestration without duplicating reference prose.
### Slice C: independent-review capability and contracts
Owner: `reviewer-profile-implementer` worker, 15-minute deadline.
Owned paths:
- `extensions/interactive-subagents/agents/reviewer.md`
- `extensions/interactive-subagents/README.md`
- focused reviewer-profile tests under `extensions/interactive-subagents/test/`
Add a reviewer profile that declares Git-capable `bash` and read tools but no `write`, `edit`, or nested-agent capability. State its read-only, fingerprint-verification, identity-reporting contract. Add focused profile-discovery/tool-declaration coverage. The parent still proves Git status/tree/paths before and after review because shell access can mutate indirectly.
Owner: parent agent, 15-minute implementation checkpoint.
Owned paths:
- `skills/develop/code-review/SKILL.md`
- `skills/design/implement/SKILL.md`
Make a workflow record a first-class specification source without requiring issue-tracker configuration. Pin base and product commits, clean status, tree hash, commit list, exact binary full-index diff command, and SHA-256 digest. Require separate direct reviewers, mutator-identity exclusion, reviewer digest regeneration, before/after mutation checks, non-reviewer remediation, re-verification, and a fresh review. Convert `implement` into a user-only compatibility pointer for bounded assignments and route multi-step implementation through `develop-workflow`; remove self-review and child commit/integration instructions.
### Slice D: exposure, ADR, and historical documentation
Owner: parent agent, 20-minute implementation checkpoint.
Owned paths:
- `package.json`
- `scripts/pi-links.test.mjs`
- `README.md`
- `docs/adr/0006-skill-led-development-workflow-04SEP2026.md`
- workflow-specific notes in ADRs 0001, both 0002 files, 0004, and 0005
- `docs/workflow/README.md`
- `docs/workflow/design.md`
- `docs/workflow/dashboard-mocks.md`
- `docs/workflow/usability-plan.md`
- `extensions/workflow/README.md`
- `docs/nono-dynamic-permissions-plan.md`
- `docs/npm-packages.md`
- `docs/todos.md` only for a progress annotation, if useful
Add ADR 0006 superseding the three custom-engine workflow ADRs while preserving broader package/import decisions in ADRs 0004/0005. Add path-correct historical banners to legacy workflow docs. Mark the extension private, historical, and unrelated to current skill exposure. Replace Nono’s planned legacy workflow-executor integration with the skill’s use of already-approved parent tools and command policies. Keep retirement and pilot TODOs unresolved.
Add `test:develop-workflow` to the full `bun run test` chain. Add a link regression proving the new shared skill is discovered globally while both workflow extension maps remain empty. Update the shared-skill count from 34 to 35; make no manifest exposure entry because discovery is automatic.
### Slice E: verification, review, and integration
Owner: parent agent.
1. Inspect every worker result against before/after branch, HEAD, status, changed paths, and owned paths; reject unexpected commits or paths.
2. Checkpoint the record through the new helper after it exists.
3. Run focused tests after each slice, then `node --test skills/develop/develop-workflow/tests/checkpoint-record.test.mjs`, `node --test scripts/pi-links.test.mjs`, `./scripts/pi-links.mjs check` from the primary checkout where linked-worktree ownership is recognized, `bun run typecheck`, and `bun run test`.
4. Commit focused verified changes. Fetch `origin/main`, rebase when the remote is available, and rerun affected/full checks if the effective tree changes.
5. Pin base, product HEAD, clean status, tree hash, commit list, and `git diff --binary --full-index --find-renames <base>...<product-head> --`; hash those exact bytes with SHA-256 in this record.
6. Launch separate direct standards and specification reviewers using the new reviewer identity. Require matching digest, read-only before/after evidence, and identities distinct from all mutators. Valid findings go to a non-reviewer, followed by verification and fresh review. Escalate after two failed remediation cycles or material scope/architecture change.
7. If local `main` and its checkout remain safe, fast-forward it, add one metadata-only record closure commit, run a cheap sanity check, prove both commits reachable, and remove the worktree/feature branch. The pre-existing dirty source file currently blocks this step; stop for evidence-backed resolution if it remains dirty.
### Rollback
Before integration, abandon only by preserving evidence and obtaining cleanup approval. After integration, revert focused commits rather than rewriting shared history. No force operation, destructive cleanup, push, publish, deployment, or production action is authorized.
## Ownership
- Parent agent: orchestration, record, skill prose/template/references, review and implement skills, link/package integration, ADR/docs, verification, commits, review brief, integration.
- `checkpoint-implementer`: checkpoint helper and its test only; mutator and reviewer-ineligible.
- `reviewer-profile-implementer`: reviewer profile, README section, and focused profile test only; mutator and reviewer-ineligible.
- `checkpoint-implementer`: launched 04SEP2026 from clean HEAD `2e53940`, 15-minute deadline, stable child ID unavailable. Completed in 9m54s without a commit; changed only its two owned paths. Parent reran 13 focused tests successfully. Identity is reviewer-ineligible.
- `reviewer-profile-implementer`: launched 04SEP2026 from clean HEAD `2e53940`, 15-minute deadline, stable child ID unavailable. Completed in 5m11s without a commit; changed only its three owned paths. Parent reran all 181 package tests successfully. Identity is reviewer-ineligible.
- Parent agent: active mutator for all other recorded paths and the post-assignment nested-directory checkpoint improvement.
## Verification matrix
| Surface | Command | Outcome |
| --- | --- | --- |
| Checkpoint helper | `node --test skills/develop/develop-workflow/tests/checkpoint-record.test.mjs` | Exit 0, 13 tests passed after parent improvement |
| Managed-link regression | `node --test scripts/pi-links.test.mjs` | Exit 0, 8 tests passed during implementation |
| Host exposure | `./scripts/pi-links.mjs check` from the primary checkout after integration | Exit 0; 40 links and 0 local package entries match the manifest |
| TypeScript | `bun run typecheck` | Exit 0 after `bun install --frozen-lockfile` restored the worktree environment without tracked dependency changes |
| Full repository | `bun run test` | Exit 0; all configured Python, Node, package, workflow, and Vitest suites completed |
| Skill behavior | Fresh-session scenario evaluation and three-change pilot | Deferred and still tracked in `docs/todos.md` |
## Review
- Review fixed point: base `b4373113a404bb523bef306a66fb8a22fc706223`, product HEAD `ebbf3c3d19677d161c1fd05229e9faf0852e1ee5`.
- Specification sources: this record and the accepted skill-led workflow plan.
- Mutators: parent agent, `checkpoint-implementer`, and `reviewer-profile-implementer`; the worker names identify ambiguous runs because stable child IDs were unavailable.
- Product status: clean at pin time.
- Product tree: `a9c951c9ab0349e7e2ea2fbbd5238acf9dc715c9`.
- Commit list:
- ebbf3c3d19677d161c1fd05229e9faf0852e1ee5 refactor(skills): simplify checkpoint locking
- c257f31c21de68e5ac26620d05c0740d4e130893 docs: approve checkpoint simplification
- 435a12107ff860b01a6cd02097b1abbb043d1b9e docs: approve cooperative checkpoint locks
- 26eeab209ed79c8cca5e6f315ad0d63385b81196 docs: record lock remediation rejection
- 4178ee48578c9cd1ce8e10777a9411844715d9d5 docs: pin exceptional remediation review
- 52576c755666f95a40907b0500f095c6d314c346 fix(skills): preserve replaced checkpoint locks
- 444483ade6be2591bd163f663af54cf343fb58b8 docs: approve focused lock remediation
- ebab671969afbf0ad7fb22b7a81d1bfa0241a278 docs: record post-rebase review finding
- b556bcf147c9a94b1a172634d2ca8a263f2fe737 docs: pin post-rebase develop workflow review
- b4e71b47db4af3455ab665050f51b96c0bed3959 docs: record dirty source resolution
- 1782d1a0c6afabcdf590c8a47a9ff848a10d3481 docs: record final develop workflow approval
- a241fb05aefa930d5fef543b24cf162e3d998985 docs: pin final develop workflow review
- 7a58ec0eefbe533ac754fdff728ec97314e2f5d4 fix(skills): remove checkpoint helper dead code
- 08ebfb50adb5d7d2d0924bbbe79b78586ed2541b docs: record second develop workflow review
- e65ea4c0aaff081f3e151513951dc197f1a030f5 docs: pin remediated develop workflow review
- 1e213db45a5152b763bb24c0f0310ae3972fd891 fix(skills): address develop workflow review
- 1bab01799caa2aee566dbbc88c87ccb095f0e55a docs: record develop workflow review findings
- 353278aa965b35e0287f889fa96c7757ba7eb040 docs: pin develop workflow review brief
- f8a4362287aa5e09ffa3568b2efb47246d856429 docs: checkpoint develop workflow verification
- 12495473ee60642e92a5760ea6cbf9563c962b5e docs: supersede the legacy workflow engine
- f1b525cec98f44b4b9f831b05b585fe75d73180c feat(skills): enforce independent workflow review
- b6bb5db03330410f4e417498e9441a9e31a11238 feat(subagents): add independent reviewer profile
- 8c44c07c6abde9cdd815da6784d245e6774a6539 feat(skills): add develop workflow
- a794a1e10a37f15b39f9b55686168f6987b614b7 docs: checkpoint develop workflow implementation
- 6389d22f1505669587d0809c1ed59a040630d3d9 docs: checkpoint develop workflow plan approval
- fbf1a52b647d3aed7f9b275097384144af5a0eef docs: record develop workflow implementation plan
- 06e631504cb7df74ded1f4a4651fcf00c86e6ed4 docs: initialize develop workflow record
- Exact diff command: `git diff --binary --full-index --find-renames b4373113a404bb523bef306a66fb8a22fc706223...ebbf3c3d19677d161c1fd05229e9faf0852e1ee5 --`.
- Exact diff SHA-256: `6ff7dfffd896870e7d4cfb2411b78a0b8fbb6f293e33d9b87801728ad51d2740`.
- Cycle 1 standards reviewer: `develop-workflow-standards-review` / `precedent-locator` / child `b374d808` / session `01a06cc9-0b63-7d1d-ab7d-47c8286ac5f1`; digest confirmed; CHANGES REQUESTED.
- Cycle 1 specification reviewer: `develop-workflow-spec-review` / `claim-verifier` / child `8e5f72cd` / session `01a06cc9-4d3f-7a6d-a6ab-37a383956368`; digest confirmed; CHANGES REQUESTED.
- Before/after reviewer evidence: both reported unchanged branch, HEAD, status, tree, paths, metadata-only delta, product tree, and digest; repository remains clean.
- Remediation cycle 1 accepted findings: restore immutable ADR 0004/0005 decision prose and append supersession notes; remove duplicated detailed rules from the main workflow skill; remove unused checkpoint API aliases; enumerate complete just-in-time risk triggers with evidence/recommended-option procedure; document todo-herdr and artifact-presenter degradation. Parent agent completed all five fixes in `913b7f6`; reviewers remain excluded.
- Cycle 2 standards reviewer: `develop-workflow-standards-review-2` / `precedent-locator` / child `a7b954ee` / session `01a06cce-3f6b-7df2-bb6b-521f1411930b`; digest confirmed; CHANGES REQUESTED for one low dead-code finding.
- Cycle 2 specification reviewer: `develop-workflow-spec-review-2` / `claim-verifier` / child `f5d00b41` / session `01a06cce-7ebd-7d9c-a48f-9c26f0517d26`; digest confirmed; APPROVE with no findings.
- Cycle 2 before/after evidence: both reviewers reported the pinned tree/digest, metadata-only delta, and clean unchanged state.
- Remediation cycle 2 accepted finding: remove unused `validateRecord` test import and unused `directory` return from `openUniqueTemp`; parent completed the cleanup in the current product HEAD.
- Cycle 3 standards reviewer: `develop-workflow-standards-review-3` / `precedent-locator` / child `dae51eb0` / session `01a06cd1-df86-75bc-bcbd-58e17557971f`; digest confirmed; APPROVE with no findings.
- Cycle 3 specification reviewer: `develop-workflow-spec-review-3` / `claim-verifier` / child `9cb40c3b` / session `01a06cd2-1667-7561-8d23-7d752714e41f`; digest confirmed; APPROVE with no findings.
- Cycle 3 before/after evidence: both reviewers reported matching product tree/digest, metadata-only delta, and clean unchanged state. Both review axes approved the pre-rebase product fingerprint.
- Post-rebase review standards reviewer: `develop-workflow-post-rebase-standards` / `precedent-locator` / child `b476565e` / session `01a06ce4-3bf5-707e-900f-34509a9801b4`; digest confirmed; APPROVE with no findings.
- Post-rebase review specification reviewer: display `:1`, profile/child unavailable, session `01a06ce5-2764-7596-96a7-50a52309b921`; digest confirmed; CHANGES REQUESTED.
- Accepted medium finding: checkpoint cleanup unconditionally unlinked the lock pathname. If the invocation-owned lock was removed and replaced while its original handle remained open, cleanup could delete another invocation’s lock.
- Exceptional remediation: capture the acquired lock inode/device identity and unlink the lock pathname only when its current identity still matches; regression test replaces the lock during active cleanup and proves the replacement remains.
- Exceptional remediation standards reviewer: `develop-workflow-exception-standards` / `precedent-locator` / child `f28de7de` / session `01a06ceb-bd79-7050-86d5-693b50a01fe7`; digest confirmed; CHANGES REQUESTED.
- Exceptional remediation specification reviewer: `develop-workflow-exception-spec` / `claim-verifier` / child `4d059ea9` / session `01a06ceb-eace-7afb-afdd-df094bb66f6a`; digest confirmed; CHANGES REQUESTED.
- Remaining medium finding: `stat(lock)` followed by `unlink(lock)` is still a check-then-act race, and `stat` follows symlinks. An external replacement in that gap could be deleted. The regression replaced the lock after closing the original handle and did not model the exact scenario.
- Approved architecture revision: the user selected a cooperative lock contract. Conforming helper invocations serialize through exclusive `wx` acquisition and never replace an existing lock. Concurrent out-of-protocol pathname mutation is not a supported safety boundary.
- Simplification decision: the user directed the parent to remove the disproportionate adversarial lock machinery and move on. Keep only practical cooperative lock behavior and its existing concurrency/stale-lock tests.
- Simplification result: inode/device guards and the adversarial replacement test were removed; the public reference now states the cooperative boundary. Focused tests pass 13/13, link tests pass 8/8, typecheck and the full suite exit 0.
- Final simplified standards reviewer: `develop-workflow-simplified-standards` / `precedent-locator`; digest confirmed; APPROVE with no findings.
- Final simplified specification reviewer: `develop-workflow-simplified-spec` / `claim-verifier`; digest confirmed; APPROVE with no findings.
- Final review evidence: both reviewers confirmed product tree `a9c951c9ab0349e7e2ea2fbbd5238acf9dc715c9`, metadata-only review delta, and clean unchanged state.
- This review-brief checkpoint is necessarily a metadata-only commit after the pinned product HEAD because a record cannot contain the digest of bytes that include its own digest. Prove it touches only this record.
## Integration and rollback
Local `main` fast-forwarded to approved feature commit `88c20ca51b14ab88ff3b1fea5ba136a1b91503cc` from clean source and feature checkouts. The repository link manager created the authorized `develop-workflow` shared-skill link, and its final check passed. This metadata-only closure commit touches only this record. Rollback follows the approved plan above: revert focused commits rather than rewriting history.
## Checkpoints and blockers
- Initial record created and committed at `256f001` before production mutation.
- Three read-only discovery assignments completed without file ownership or mutation.
- Exact plan checkpoint prepared at `b75de2d` and explicitly approved by the user on 04SEP2026.
- Pre-assignment evidence: branch `feature/develop-workflow`, HEAD `b75de2d`, clean status, changed paths none.
- Both bounded assignments completed within deadline, stayed on HEAD `2e53940`, made no commits, and changed only owned paths. Parent inspection accepted their diffs.
- Focused evidence: checkpoint tests 13/13; interactive-subagents tests 181/181; link tests 8/8.
- Environment recovery: initial typecheck exposed missing installed packages; `bun install --frozen-lockfile` restored the worktree environment with no manifest/lockfile diff. OpenSrc cached exact `effect@4.0.0-beta.101` and `@effect/platform-node@4.0.0-beta.101`; its first lockfile inference selected older versions and was corrected with explicit versions.
- Full evidence: `bun run typecheck` and `bun run test` exited 0.
- Synchronization evidence: fetched `origin/main`; local and remote main remain `27cbe965`; no rebase was needed.
- Review cycle 1: both independent identities confirmed the pinned digest and remained read-only; both requested changes.
- Remediation cycle 1: parent fixed all accepted findings; focused checks, `bun run typecheck`, and `bun run test` exited 0.
- Review cycle 2: specification approved; standards requested removal of two dead-code items. Both reviewers remained read-only.
- Remediation cycle 2: parent removed both items; focused tests, `bun run typecheck`, and `bun run test` exited 0.
- Review cycle 3: standards and specification approved the exact fingerprint with no findings; both reviewers remained read-only.
- Source-checkout decision: on 04SEP2026 the user explicitly chose to preserve the exact pre-existing YAML quoting change in a separate `main` commit. This authorizes only that one-file commit; it does not broaden feature scope.
- Source result: committed the exact YAML quoting change on `main` at `b4373113a404bb523bef306a66fb8a22fc706223`; source checkout became clean.
- Rebase result: feature rebased cleanly onto `b4373113a404bb523bef306a66fb8a22fc706223`; focused checkpoint/link tests, `bun run typecheck`, and `bun run test` all exited 0.
- Post-rebase review brief: product `b4e71b47db4af3455ab665050f51b96c0bed3959`, tree `1efcde6ccbd81063a44c46a3b75b34d50d3af4b0`, digest `3fd063ad1a4591e776ef2a29c26f31482bbcb5329a6541126fd991c72a0d247f`.
- Post-rebase review outcome: standards approved; specification requested a focused lock-ownership fix. Both reviewers reported unchanged clean Git state.
- Escalation decision: after two earlier remediation cycles, the user explicitly authorized one exceptional focused remediation cycle for this lock-ownership finding.
- Approved remediation scope: capture the acquired lock identity, unlink only when the pathname still resolves to that identity, and add a regression test that replaces the pathname during an active checkpoint.
- Remediation result: implemented the approved scope at `52576c755666f95a40907b0500f095c6d314c346`; focused tests pass 14/14, link tests pass 8/8, `bun run typecheck` exits 0, and `bun run test` exits 0.
- Review result: both axes rejected the remediation because identity verification and pathname deletion are not atomic; reviewers remained read-only with unchanged clean state.
- Architecture constraint: another conforming helper invocation cannot replace an existing lock because acquisition uses `open(..., "wx")`; the disputed race requires out-of-protocol filesystem mutation. Portable Node filesystem APIs provide no atomic compare-and-delete primitive for defending a path lock against that mutation.
- Architecture decision: the user approved narrowing the contract to cooperative helper invocations rather than broadening Slice 1–2 with a broker or platform-specific native primitive.
- Approved follow-up: remove inode/device identity tracking and adversarial replacement tests, retain exclusive `wx` acquisition plus ordinary owned-lock cleanup, reverify once, and complete final review/integration without further hardening scope.
- Simplified product: `ebbf3c3d19677d161c1fd05229e9faf0852e1ee5`, tree `a9c951c9ab0349e7e2ea2fbbd5238acf9dc715c9`, digest `6ff7dfffd896870e7d4cfb2411b78a0b8fbb6f293e33d9b87801728ad51d2740`.
- Final review result: both standards and specification approved with no findings; reviewers remained read-only.
- Integration result: clean local `main` fast-forwarded to `88c20ca51b14ab88ff3b1fea5ba136a1b91503cc`; `origin/main` remained `27cbe965bbd3dc0ca38814811b4f56519a570957`; no push occurred.
- Exposure result: user authorized the single managed symlink; `./scripts/pi-links.mjs sync` created it and the final check reported 40 matching links and 0 local package entries.
- Closure: this final metadata-only commit records completion. After its sanity and reachability checks, remove the clean workflow-owned worktree and merged feature branch.
+3 -1
View File
@@ -1,6 +1,8 @@
# workflow usability update plan
Status: manual recovery plan after managed Workflow Runs `512d6c31`, `51b0c1cb`, and `ecb11e1e` failed or were interrupted before implementation.
> **Historical reference, superseded for new development:** This plan describes legacy `workflow` extension recovery and TUI work. New feature work uses the model-invoked [`develop-workflow` skill](../../skills/develop/develop-workflow/SKILL.md) under [ADR 0006](../adr/0006-skill-led-development-workflow-04SEP2026.md). Retain it only as failure and pilot evidence.
Status: historical manual recovery plan after managed Workflow Runs `512d6c31`, `51b0c1cb`, and `ecb11e1e` failed or were interrupted before implementation.
## Locked decisions
+2 -1
View File
@@ -82,8 +82,9 @@ If the reply arrives while the sub-agent is still mid-turn, it is absorbed into
| **scout** | `openai-codex/gpt-5.6-luna` (`xhigh`) | `read`, `grep`, `find`, `ls` | Fast read-only codebase recon |
| **researcher** | `openai-codex/gpt-5.6-luna` (`xhigh`) | `web_search`, `web_fetch`, `safe_bash` | Web research, synthesized into a sourced brief |
| **worker** | `openai-codex/gpt-5.6-luna` (`xhigh`) | `read`, `write`, `edit`, `bash`, `web_search`, `web_fetch` + spawning | General implementer; may spawn `scout` and `researcher` |
| **reviewer** | `openai-codex/gpt-5.6-luna` (`xhigh`) | `read`, `grep`, `find`, `ls`, `bash` | Independent pinned-diff reviewer; never remediates |
All three are autonomous (`auto-exit: true`) and carry their identity in the system prompt (`system-prompt: append`).
All four are autonomous (`auto-exit: true`) and carry their identity in the system prompt (`system-prompt: append`). The `reviewer` profile regenerates the caller-supplied binary diff and confirms its SHA-256 digest before reviewing, then reports its available display/profile/session identity. Its declaration omits write/edit and nested-agent capability. Because `bash` can mutate indirectly, the profile is a procedural read-only contract rather than sandbox enforcement; the parent must validate Git state before and after review.
## Custom agents
@@ -0,0 +1,46 @@
---
name: reviewer
description: Independently reviews a pinned Git diff without modifying the repository
tools: read, grep, find, ls, bash
model: openai-codex/gpt-5.6-luna
thinking: xhigh
system-prompt: append
auto-exit: true
---
# Reviewer
You are an independent, read-only Git reviewer. Review only the caller-supplied
scope and specification, and report findings without remediation.
## Review contract
1. Report your identity before reviewing and in the final report: display name,
profile (`reviewer`), and child/session or parent-session identity when the
caller or runtime makes them available. Write `unavailable` rather than
inventing an identifier.
2. Use Git and the supplied review brief as the source of truth. Run the exact
caller-supplied binary diff command, unchanged, and regenerate its raw output
bytes. Use `sha256sum` (or an equivalent byte-preserving hash command) to
calculate SHA-256 over those exact bytes and compare it with the
caller-supplied digest before assessing the diff. Do not substitute a diff
command, normalize or re-render output, or proceed when the command or digest
is missing or the digest does not match.
3. Capture Git status, `HEAD`, and the relevant tree/path evidence before and
after review when requested by the caller. Report any change as a failed
read-only review and do not issue a verdict.
4. Assess specification conformance and repository standards separately. Report
only concrete, evidence-backed findings with path, location, severity, and a
concise remediation suggestion. State an explicit approval or changes-
requested verdict for each requested axis.
## Read-only boundary
The declared tools intentionally include `bash` for Git inspection and hashing,
but bash can mutate a repository indirectly. This profile is procedural and
provides no sandbox-enforcement claim: use bash only for non-mutating inspection
and, if needed, temporary digest capture outside the repository. Never edit or
write repository files, stage, commit, checkout, reset, restore, clean, rebase,
or delete; do not use write/edit tools (they are not granted), spawn nested
agents, or fix any finding yourself. A non-reviewer must perform remediation
and a fresh review must verify it.
@@ -1144,6 +1144,28 @@ describe("status.ts", () => {
describe("subagent discovery", () => {
const testApi = (subagentsModule as any).__test__;
it("discovers the bundled reviewer with only Git-capable read tools", async () => {
await withIsolatedAgentEnv(async () => {
const reviewer = testApi.loadAgentDefaults("reviewer");
assert.ok(reviewer, "expected bundled reviewer to be discoverable");
assert.deepEqual(
reviewer.tools?.split(",").map((tool: string) => tool.trim()),
["read", "grep", "find", "ls", "bash"],
);
assert.equal(reviewer.subagentAgents, undefined, "reviewer must not spawn nested agents");
assert.equal(reviewer.systemPromptMode, "append");
assert.equal(reviewer.autoExit, true);
assert.match(reviewer.body ?? "", /caller-supplied binary diff command/);
assert.match(reviewer.body ?? "", /SHA-256/);
assert.match(reviewer.body ?? "", /never remediates|remediation/);
const listed = testApi.discoverAgentDefinitions().find(
(agent: { name: string }) => agent.name === "reviewer",
);
assert.equal(listed?.source, "package");
});
});
it("loads session-mode from frontmatter", async () => {
await withIsolatedAgentEnv(async ({ projectAgentsDir }) => {
writeAgentFile(
+2 -2
View File
@@ -1,5 +1,5 @@
# workflow
Persisted, gated feature workflows for Pi. The private Bun workspace retains the npm package name `@jbcode/pi-workflow` under the repository's scoped package convention.
> **Historical reference:** This private legacy extension was superseded for new development by the model-invoked [`develop-workflow` skill](../../skills/develop/develop-workflow/SKILL.md) under [ADR 0006](../../docs/adr/0006-skill-led-development-workflow-04SEP2026.md).
This workspace is currently private and must not be published. Its remaining runtime dependencies on the `_workflows` reference implementation must first be merged into this directory; see `docs/todos.md` at the repository root.
Keep this workspace private, unpublished, and unexposed during the skill pilot. It is historical implementation evidence, not the skill implementation or a prerequisite for skill development. Archive or remove it only in the separately approved post-pilot cleanup.
+2 -1
View File
@@ -44,10 +44,11 @@
"prepare": "effect-tsgo patch",
"imports:check": "bun scripts/import-package-snapshots.mjs check all",
"typecheck": "tsc --noEmit",
"test": "bun run test:runner && bun run test:artifact-presenter && bun run test:dictate-service && bun run test:links && bun run test:packages && bun run test:imported && bun run test:node && bun run test:file-search",
"test": "bun run test:runner && bun run test:artifact-presenter && bun run test:dictate-service && bun run test:develop-workflow && bun run test:links && bun run test:packages && bun run test:imported && bun run test:node && bun run test:file-search",
"test:runner": "python -m unittest discover -s scripts -p 'test_*.py'",
"test:artifact-presenter": "python -m unittest discover -s skills/document/artifact-presenter/tests -p 'test_*.py'",
"test:dictate-service": "PYTHONPATH=services/dictate/src python -m unittest discover -s services/dictate/tests -p 'test_*.py'",
"test:develop-workflow": "node --test skills/develop/develop-workflow/tests/checkpoint-record.test.mjs",
"test:links": "node --test scripts/pi-links.test.mjs",
"test:packages": "node --test scripts/package-workspaces.test.mjs",
"test:imported": "bun run --filter '@jbcode/pi-interactive-subagents' test && bun run --filter '@jbcode/pi-native-hooks' test && bun run --filter '@jbcode/pi-todo-herdr' test",
+25
View File
@@ -250,6 +250,31 @@ test("normal exposure and package evidence retain the Slice 0 security boundary"
{},
"published extensions must not be re-exposed as development symlinks",
);
assert.deepEqual(
links.extensions.globalLocalPackages,
{},
"the legacy workflow extension must remain unexposed",
);
const exposureRoot = await mkdtemp(
path.join(os.tmpdir(), "develop-workflow-exposure-test-"),
);
const exposurePlan = await buildPlan({
repoRoot: repositoryRoot,
homeDir: path.join(exposureRoot, "home"),
agentDir: path.join(exposureRoot, "home", ".pi", "agent"),
globalSkillsDir: path.join(exposureRoot, "home", ".agents", "skills"),
settingsFile: path.join(exposureRoot, "home", ".pi", "agent", "settings.json"),
});
assert.ok(
exposurePlan.operations.some(
(operation) =>
operation.type === "create-link" &&
operation.link.name === "develop-workflow" &&
operation.link.scope === "global",
),
"develop-workflow must be discovered as a shared global skill",
);
const lock = JSON.parse(
await readFile(
+5 -7
View File
@@ -1,15 +1,13 @@
---
name: implement
description: "Implement a piece of work based on a spec or set of tickets."
description: "Implement one bounded assignment from an approved plan."
disable-model-invocation: true
---
Implement the work described by the user in the spec or tickets.
# Implement
Use /tdd where possible, at pre-agreed seams.
For feature work and other multi-step changes, invoke `develop-workflow`; let its parent orchestrator own approval gates, assignments, commits, independent review, and integration.
Run typechecking regularly, single test files regularly, and the full test suite once at the end.
For one bounded assignment, require the approved plan, acceptance criteria, exclusive owned paths, test seam, allowed commands, and deadline. Use `tdd` where practical. Change only owned paths, run focused checks, and return the diff and command evidence to the parent.
Once done, use /code-review to review the work.
Commit your work to the current branch.
Leave user interaction, mutation delegation, commits, remediation decisions, review, and integration to the parent orchestrator.
+52 -56
View File
@@ -1,87 +1,83 @@
---
name: code-review
description: "Review the changes since a fixed point (commit, branch, tag, or merge-base) along two axes: Standards (does the code follow this repo's documented coding standards?) and Spec (does the code match what the originating issue/spec asked for?). Runs both reviews in parallel sub-agents and reports them side by side. Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to \"review since X\"."
description: Review changes since a fixed commit, branch, tag, or merge-base along separate repository-standards and specification axes. Use for branch, PR, work-in-progress, or workflow-record review after implementation and verification.
---
Two-axis review of the diff between `HEAD` and a fixed point the user supplies:
# Code review
- **Standards**: does the code conform to this repo's documented coding standards?
- **Spec**: does the code faithfully implement the originating issue / spec?
Run two independent, read-only reviews over one pinned diff:
Both axes run as **parallel sub-agents** so they don't pollute each other's context, then this skill aggregates their findings.
- **Standards:** repository instructions and documented conventions.
- **Specification:** the originating issue, spec, or approved workflow record.
The issue tracker should have been provided to you. If `docs/agents/issue-tracker.md` is missing, tell the user to run `/configure-skills`.
Keep the axes separate so conformity cannot hide incorrect scope and correct scope cannot hide standards violations.
## Process
## 1. Pin the review bytes
### 1. Pin the fixed point
Require a fixed point; ask when none was supplied. Resolve and record:
Whatever the user said is the fixed point (a commit SHA, branch name, tag, `main`, `HEAD~5`, etc.). If they didn't specify one, ask for it.
- review base commit;
- immutable product HEAD commit, never mutable `HEAD` in the reviewer brief;
- clean status and product tree hash;
- `git log <base>..<product-head> --oneline`;
- exact command `git diff --binary --full-index --find-renames <base>...<product-head> --`.
Capture the diff command once: `git diff <fixed-point>...HEAD` (three-dot, so the comparison is against the merge-base). Also note the list of commits via `git log <fixed-point>..HEAD --oneline`.
Run that command once in the parent, hash its exact bytes with SHA-256, and record the digest. Stop on a bad ref, dirty product state, or empty diff unless an empty comparison was explicitly expected.
Before going further, confirm the fixed point resolves (`git rev-parse <fixed-point>`) and the diff is non-empty. A bad ref or empty diff should fail here, not inside two parallel sub-agents.
## 2. Resolve specification and standards
### 2. Identify the spec source
Accept the first available specification source:
Look for the originating spec, in this order:
1. a caller-supplied approved workflow record or spec path;
2. issue references in commits, using `docs/agents/issue-tracker.md` when configured;
3. a matching file under `docs/`, `specs/`, or `.scratch/`;
4. an explicit user statement that no specification exists.
1. Issue references in the commit messages (`#123`, `Closes #45`, GitLab `!67`, etc.), fetched via the workflow in `docs/agents/issue-tracker.md`.
2. A path the user passed as an argument.
3. A spec file under `docs/`, `specs/`, or `.scratch/` matching the branch name or feature.
4. If nothing is found, ask the user where the spec is. If they say there isn't one, the **Spec** sub-agent will skip and report "no spec available".
A workflow record is a complete specification source and never requires issue-tracker configuration. Ask only when no source or explicit no-spec decision exists. Skip the specification reviewer and report “no specification available” only after that explicit decision.
### 3. Identify the standards sources
Collect applicable `AGENTS.md`, `CLAUDE.md`, contributing, coding-standard, and ADR sources. Add this smell baseline where repository standards do not override it; treat each as a judgment call and skip tooling-enforced issues:
Anything in the repo that documents how code should be written, such as `CODING_STANDARDS.md` or `CONTRIBUTING.md`.
- **Mysterious Name:** a name hides purpose.
- **Duplicated Code:** the same logic shape recurs.
- **Feature Envy:** behavior reaches into another object's data more than its own.
- **Data Clumps:** fields or parameters repeatedly travel together.
- **Primitive Obsession:** primitives stand in for a domain concept.
- **Repeated Switches:** the same type cascade recurs.
- **Shotgun Surgery:** one change requires scattered edits.
- **Divergent Change:** one module changes for unrelated reasons.
- **Speculative Generality:** abstraction exists without a current requirement.
- **Message Chains:** callers navigate long object chains.
- **Middle Man:** code delegates without adding value.
- **Refused Bequest:** an implementation ignores most inherited behavior.
On top of whatever the repo documents, the Standards axis always carries the **smell baseline** below: a fixed set of Fowler code smells (_Refactoring_, ch.3) that applies even when a repo documents nothing. Two rules bind it:
## 3. Enforce reviewer independence
- **The repo overrides.** A documented repo standard always wins; where it endorses something the baseline would flag, suppress the smell.
- **Always a judgement call.** Each smell is a labelled heuristic ("possible Feature Envy"), never a hard violation. Like any standard here, skip anything tooling already enforces.
List every display name, profile, child/session ID, and ambiguous run that authored or mutated the diff. Select two different reviewer identities outside that set. Require repository read access plus Git-capable `bash`, with no declared `write`, `edit`, or nested-agent capability.
Each smell reads *what it is* → *how to fix*; match it against the diff:
Capture branch, HEAD, status, tree hash, and changed paths before review. Launch the standards and specification reviewers as separate direct subagent calls rather than a wrapped parallel call. Give both the pinned commits, exact diff command, expected digest, commit list, mutator exclusions, and identity-reporting requirement.
- **Mysterious Name**: a function, variable, or type whose name doesn't reveal what it does or holds. → rename it; if no honest name comes, the design's murky.
- **Duplicated Code**: the same logic shape appears in more than one hunk or file in the change. → extract the shared shape, call it from both.
- **Feature Envy**: a method that reaches into another object's data more than its own. → move the method onto the data it envies.
- **Data Clumps**: the same few fields or params keep travelling together (a type wanting to be born). → bundle them into one type, pass that.
- **Primitive Obsession**: a primitive or string standing in for a domain concept that deserves its own type. → give the concept its own small type.
- **Repeated Switches**: the same `switch`/`if`-cascade on the same type recurs across the change. → replace with polymorphism, or one map both sites share.
- **Shotgun Surgery**: one logical change forces scattered edits across many files in the diff. → gather what changes together into one module.
- **Divergent Change**: one file or module is edited for several unrelated reasons. → split so each module changes for one reason.
- **Speculative Generality**: abstraction, parameters, or hooks added for needs the spec doesn't have. → delete it; inline back until a real need shows.
- **Message Chains**: long `a.b().c().d()` navigation the caller shouldn't depend on. → hide the walk behind one method on the first object.
- **Middle Man**: a class or function that mostly just delegates onward. → cut it, call the real target direct.
- **Refused Bequest**: a subclass or implementer that ignores or overrides most of what it inherits. → drop the inheritance, use composition.
Require each reviewer to:
### 4. Spawn both sub-agents in parallel
1. regenerate the exact diff and confirm its SHA-256 before analysis;
2. remain read-only and perform no remediation;
3. bind every finding to file/hunk evidence and severity;
4. report display/profile/child/session identity when exposed;
5. stay under 400 words.
**Standards sub-agent prompt** should include:
For standards, include every standards source and the full baseline above. Ask for documented violations separately from baseline judgments. For specification, include the approved source and ask for missing/partial requirements, scope creep, and apparently implemented but incorrect behavior with quoted requirements.
- The full diff command and commit list.
- The list of standards-source files you found in step 3, **plus the smell baseline from step 3** pasted in full (the sub-agent has no other access to it).
- The brief: "Report, per file/hunk where relevant, (a) every place the diff violates a documented standard: cite the standard (file + the rule); and (b) any baseline smell you spot: name it and quote the hunk. Distinguish hard violations from judgement calls: documented-standard breaches can be hard, but baseline smells are always judgement calls, and a documented repo standard overrides the baseline. Skip anything tooling enforces. Under 400 words."
Treat missing delivery, digest mismatch, reviewer/mutator identity overlap, or before/after repository mutation as no verdict. Any reviewer mutation also makes that identity a mutator.
**Spec sub-agent prompt** should include:
## 4. Aggregate without reranking
- The diff command and commit list.
- The path or fetched contents of the spec.
- The brief: "Report: (a) requirements the spec asked for that are missing or partial; (b) behaviour in the diff that wasn't asked for (scope creep); (c) requirements that look implemented but where the implementation looks wrong. Quote the spec line for each finding. Under 400 words."
Present the reports under `## Standards` and `## Specification`. Lightly clean formatting only; keep findings and axis severity separate. End with finding counts and the worst issue within each axis, not one combined winner.
If the spec is missing, skip the Spec sub-agent and note this in the final report.
Bind approval to the base, product HEAD, clean status, tree hash, and diff digest.
### 5. Aggregate
## 5. Remediate through non-reviewers
Present the two reports under `## Standards` and `## Spec` headings, verbatim or lightly cleaned. Do **not** merge or rerank findings, because the two axes are deliberately separate (see _Why two axes_).
Validate findings against repository evidence. Assign accepted fixes to an original implementer or another non-reviewer. Rerun affected checks and the full required suite, create a new pinned fingerprint, and launch two fresh independent reviewers.
End with a one-line summary: total findings per axis, and the worst issue _within each axis_ (if any). Don't pick a single winner across axes: that's the reranking the separation exists to prevent.
Escalate after two unsuccessful remediation cycles or when a finding materially changes scope, acceptance criteria, ownership, verification, or architecture. Return material changes to plan approval before production mutation.
## Why two axes
A change can pass one axis and fail the other:
- Code that follows every standard but implements the wrong thing → **Standards pass, Spec fail.**
- Code that does exactly what the issue asked but breaks the project's conventions → **Spec pass, Standards fail.**
Reporting them separately stops one axis from masking the other.
**Complete when:** both axes approve the exact verified fingerprint and before/after evidence proves reviewers stayed read-only.
+97
View File
@@ -0,0 +1,97 @@
---
name: develop-workflow
description: Runs feature work and other multi-step code changes through repository research, an approved implementation plan, isolated Git worktrees, explicit UI and risk gates, verification, independent review, and local integration. Use by default for features, non-trivial bug fixes, refactors, migrations, dependency changes, security-sensitive work, and UI changes. Excludes questions, research-only requests, and obvious low-risk edits requiring at most two steps.
---
# Develop workflow
Own orchestration in the parent agent. Treat Git and the retained workflow record as recovery evidence; use task tools only as the live progress view. Give subagents bounded assignments, then verify their claims against repository state.
## 0. Classify the request
Choose the smallest sufficient path before mutation:
- **Answer:** explain or research read-only. Make no changes.
- **Light:** make one obvious low-risk edit requiring at most two steps. Inspect, edit, run focused checks, and summarize. Record why this is safe when near the boundary.
- **Full:** use the phases below for three or more steps, multiple files or modules, ambiguity, architecture, dependencies, migrations, security implications, non-trivial UI, or meaningful regression risk.
Promote light work to full before editing when discovery reveals meaningful scope or risk.
**Complete when:** observed scope and risk justify the selected path.
## Risk gates for every path
Ask just in time before adding, removing, or materially upgrading dependencies; schema/data migration; production or live-database access; public API or compatibility breaks; destructive Git/filesystem actions outside the narrowly proven final cleanup; force push or shared-history rewrite; non-fast-forward integration; conflict resolution that changes approved behavior; material scope or acceptance expansion; security-posture changes; publishing; or deployment.
Present concrete evidence, scope the authorization, and mark one option recommended with its reason. Checkpoint the decision before acting. Treat approval as specific to that action; ask again when later evidence broadens it. Promote light work to full before editing when meaningful risk appears.
## 1. Preflight and isolate
For the full path, require Git worktrees, a writable repository record, `ask_user`, Git-capable independent reviewers, and target-interface validation when UI applies. Stop before product mutation when a mandatory capability is unavailable. Let task tools degrade to the retained record and let `artifact-presenter` degrade to reporting the exact artifact path; neither optional convenience may bypass a gate. In print or JSON mode, emit the pending gate or validation instructions and stop.
Read repository instructions, relevant docs/ADRs/tests, and Git state. Follow [Git and integration](references/git-and-integration.md) to capture source evidence, resolve dirty-source approval, and establish the isolated branch/worktree. Prepare a unique record from [`assets/workflow-record.md`](assets/workflow-record.md) and publish it with the helper in [evidence and recovery](references/evidence-and-recovery.md). Create the available live progress view, run applicable documentation checks, and commit the initial record before product mutation.
**Complete when:** worktree identity is proven and the initial record commit names the next safe action.
## 2. Discover and decide requirements
Launch independent read-only research lanes directly when concerns can be investigated separately; state that no files are owned. Give each child a self-contained prompt. Keep the parent active on other work and verify that every required result was delivered. Treat missing delivery as missing evidence.
Reconcile repository evidence into scope, out-of-scope items, acceptance criteria, affected surfaces, constraints, and risks. Use `ask_user` only for decisions repository evidence cannot answer. Checkpoint decisions.
**Complete when:** requirements and unresolved questions are explicit.
## 3. Design and obtain plan approval
Record an exact implementation plan containing:
- acceptance criteria mapped to slices;
- one named owner and concrete path ownership per mutating assignment;
- existing seams and patterns;
- focused and full verification commands with realistic data needs;
- UI classification, dependencies, migrations, production effects, and risk gates;
- review fixed point and specification source;
- integration and rollback strategy;
- a 15-minute default deadline per mutating assignment, or an explicitly approved alternative.
Present the complete record through `artifact-presenter`, then obtain explicit approval with `ask_user`. Checkpoint and commit the decision before production edits. Return to this gate after any material scope, acceptance, ownership, verification, or architecture change.
**Complete when:** the user explicitly approved the exact committed plan.
## 4. Resolve non-trivial UI direction
For non-trivial UI, invoke `prototype` and create three structurally different throwaway candidates before production-component edits. Ask approval for a smaller explicit count when three meaningful alternatives are impossible. Validate candidates through `browser-harness` for web UI or the real PTY/application for terminal/native UI, including representative responsive sizes.
Retain candidates on a dedicated throwaway branch and record its commit. Present them and obtain direction approval through `ask_user`. Checkpoint the selected direction before production mutation. Rewrite the chosen direction as production code rather than promoting prototype code. Record why minor or non-visual UI skips this phase.
**Complete when:** target-interface evidence, retained prototype commit, selected direction, and reflected plan are recorded.
## 5. Implement owned slices
State one owner and exclusive paths for every mutating assignment. Serialize by default; parallelize only disjoint low-integration paths. Give each implementer the approved plan, decisions, acceptance criteria, test seam, allowed commands, and deadline. Tell workers to stop at the deadline and leave commits, integration, user interaction, and mutation delegation to the parent.
Follow [assignment evidence and recovery](references/evidence-and-recovery.md#assignment-evidence) before accepting, retrying, or reassigning mutation. Use TDD at agreed seams where practical. Run focused tests after behavioral slices. Inspect each returned diff and repository state before marking it complete.
**Complete when:** every acceptance criterion has intentional owned code and test evidence.
## 6. Verify the integrated worktree
Inspect the complete diff and status. Run focused behavioral tests, then repository-required typecheck, lint, build, and full tests. Exercise realistic E2E data across process/UI boundaries. Validate changed UI in its target interface and representative sizes. Synchronize affected documentation.
Record exact commands, exit codes, and concise outcomes. Keep flaky or environmental failures visible until resolved or escalated. Never infer success from a child transcript.
**Complete when:** the approved verification matrix passes against the integrated worktree state.
## 7. Commit, synchronize, and review
Create focused commits after their checks pass. Follow [Git and integration](references/git-and-integration.md) to fetch/rebase, reverify changed trees, and produce the pinned review brief and exact diff fingerprint. Follow [review and remediation](references/review-and-remediation.md) for two independent review axes, mutator exclusion, findings, and fresh-review rules.
**Complete when:** standards and specification reviewers approve the exact verified product diff.
## 8. Integrate and close
Follow [Git and integration](references/git-and-integration.md). Recheck the source branch and checkout, fast-forward only when evidence remains valid, then create one metadata-only closure commit touching only the record. Run a cheap final sanity check. Remove the workflow-owned worktree and branch only after reachability and clean-state proofs.
Mark tasks complete and report product/closure commits, review command and digest, reviewer identities, record path, and verification evidence. Keep push, publish, deployment, production access, unsafe cleanup, conflict-driven behavior changes, force operations, and non-fast-forward integration behind explicit just-in-time approval.
**Complete when:** reviewed work and closure are reachable from the intended local source branch and temporary state is safely removed.
@@ -0,0 +1,70 @@
# <Feature> workflow record
## Status
- Status: active | waiting | blocked | complete
- Current phase:
- Next safe action:
## Request and scope
- Request:
- In scope:
- Out of scope:
## Git identity
- Source branch:
- Base commit:
- Feature branch:
- Worktree:
- Omitted source changes:
## Acceptance criteria
- [ ]
## Approved implementation plan and gates
- Plan/specification source:
- Exact plan:
- Plan approval:
- UI classification and direction approval:
- Risk decisions and scope:
## Ownership
| Assignment | Owner identity | Deadline | Exclusive paths | Before/after Git evidence | Outcome |
| --- | --- | --- | --- | --- | --- |
## Verification matrix
| Surface | Command | Exit code | Outcome |
| --- | --- | --- | --- |
## Review
- Base and product HEAD:
- Clean status and tree hash:
- Commit list:
- Exact diff command:
- SHA-256:
- Mutator identities:
- Standards reviewer identity, digest confirmation, and verdict:
- Specification reviewer identity, digest confirmation, and verdict:
- Findings and remediation cycles:
- Before/after reviewer Git evidence:
## Integration and rollback
- Synchronization and re-verification:
- Fast-forward result:
- Product and closure commits:
- Reachability and cleanup:
- Rollback strategy:
## Checkpoints and blockers
- Last committed valid checkpoint:
- Blockers:
- Exact next safe action:
@@ -0,0 +1,45 @@
# Evidence and recovery
## Checkpoint records
Keep one concise Markdown record in the repository's established plan location; otherwise use `docs/plans/<feature-branch-slug>.md`. Require a unique path before creation. Store status/current phase, request/scope, Git identity, acceptance criteria, exact approved plan and gate decisions, assignment ownership, verification outcomes, review identities/findings/fingerprint, integration, blockers, and the next safe action.
Keep transcripts, leases, heartbeats, notification receipts, full command output, and model-authored execution graphs out of the record. Link durable artifacts or commits instead. Update only at meaningful boundaries; task tools hold high-frequency progress.
Prepare the complete candidate outside the record's same-directory temporary namespace and run:
```bash
node skills/develop/develop-workflow/scripts/checkpoint-record.mjs \
<record.md> --input <candidate.md> --expected-sha256 <current-digest>
```
Omit `--expected-sha256` only for first creation; require an absent target. Commit the initial record and every approved gate before dependent product mutation. The exclusive lock serializes cooperating helper invocations; external mutation of its pathname is outside the contract. A stale digest, lock, temporary artifact with no valid record, validation failure, or filesystem error blocks progress. Preserve artifacts for inspection; never hand-delete recovery evidence to make a command pass.
## Assignment evidence
Before every mutating assignment, record:
- display name, profile, stable child/session IDs when exposed;
- deadline and exclusive owned paths;
- branch, HEAD, clean/dirty status, changed paths, and patch artifact.
After delivery, capture the same Git evidence and inspect the patch. Treat the entire ambiguous profile/run as a mutator when stable identity is unavailable. Reject unexpected commits and paths outside ownership.
On an ownership or Git invariant failure, freeze mutation and present evidence-backed choices: inspect, accept and replan, restore with a non-destructive patch, or abort. Obtain a user decision before proceeding.
On deadline expiry or missing delivery, request stop through the available control surface and prove the process/session terminal before reassignment. A completed UI indicator is not output or liveness proof. Keep the workflow blocked for human termination/takeover when liveness remains unknown.
## Fresh-session recovery
Reconstruct observed reality before trusting prose:
1. Read repository instructions; inspect worktrees, branches, refs, status, commits, and diff.
2. Discover candidate records from observed branches/worktrees and repository conventions.
3. Treat a workflow branch/worktree without a record as an orphan. Preserve it; inspect branch, base, reflog, status, commits, and diff; ask whether to adopt it with a record, quarantine it, or remove it after separate cleanup approval.
4. Compare the record's base, branch, and checkpoint with Git evidence.
5. Preserve atomic-update temporary files. Use the last committed valid record until recovery is approved.
6. Rebuild task state from completed evidence and the recorded next action.
7. Freeze interrupted mutation until its diff is inspected and prior child liveness is terminal.
8. Resume from the last proven checkpoint.
Ask when evidence conflicts, scope may have changed, child liveness is unknown, or recovery needs risk. Treat the record as a recovery map, not a transaction log.
@@ -0,0 +1,40 @@
# Git and local integration
## Isolation
Record the source branch and exact base before creating `./.worktrees/<feature>` on `feature/<feature>`. Prove every shell location with explicit `cd` or `git -C`; verify `git rev-parse --show-toplevel` before mutation. Block on collisions, unmerged paths, unexpected ownership, or an unignored worktree root. Never rename, reset, clean, stash, copy source changes, or remove an orphan silently.
A dirty source checkout requires bounded status/path evidence and explicit approval because its changes are omitted. Preserve that evidence in the record.
## Synchronize and pin review
After focused commits and verification:
1. Fetch the latest source branch when a remote exists.
2. Rebase the feature branch onto the latest source commit.
3. Rerun affected checks and full required verification whenever rebase changes the effective tree or resolves conflicts.
4. Record the review base, product HEAD, clean status, tree hash, and commit list.
5. Generate exact review bytes:
```bash
git diff --binary --full-index --find-renames <base>...<product-head> --
```
6. Hash those bytes with SHA-256 and record the command and digest. Regenerate from the durable commits rather than committing a duplicate patch.
Ask before conflict resolution changes approved behavior, history rewrite of shared work, force operations, or non-fast-forward integration.
## Fast-forward integration
After both review axes approve the exact fingerprint:
1. Confirm the source branch still equals the final rebase base. Return to synchronize, verify, and review if it advanced.
2. Confirm the source checkout exists, is clean, and is safe to update. Stop when dirty or missing.
3. Fast-forward the source branch to the reviewed feature branch; create no merge commit.
4. In the source checkout, update only the workflow record with bound review and integration evidence.
5. Prove the closure diff touches only that record, run applicable documentation checks, and create one metadata-only closure commit.
6. Run a cheap final sanity check.
7. Prove the product and closure commits reachable from the source branch.
8. Remove the completed worktree, then delete its feature branch.
Treat cleanup in step 8 as pre-authorized only after every preceding clean-state and reachability proof. Ask before any other destructive cleanup. Pushing, publishing, deployment, and production access remain separate user actions.
@@ -0,0 +1,33 @@
# Independent review and remediation
## Identity and capability
Exclude every identity that authored or mutated the reviewed diff. Record reviewer display name, profile, child ID, and session ID when available. When stable identity is unavailable, exclude the entire ambiguous profile/run. Keep reviewers read-only and separate for:
- **Standards:** repository instructions and documented conventions.
- **Specification:** the approved workflow record and linked source plan/spec.
Use Git-capable reviewer profiles with repository read access and no declared `write` or `edit` tools. Shell access can still mutate; capture status, tree, HEAD, and changed paths before and after review. Any mutation invalidates the verdict and marks that identity as a mutator.
## Review brief
Generate the brief in the parent. Include:
- pinned base and product HEAD;
- clean status and product tree hash;
- commit list;
- exact `git diff --binary --full-index --find-renames <base>...<product-head> --` command;
- SHA-256 of those exact bytes;
- approved record/specification sources;
- repository standards sources;
- all excluded mutator identities.
Launch standards and specification reviewers through separate direct subagent calls. Avoid wrapped parallel launch until its delivery defect is proven fixed. Require each reviewer to regenerate the pinned diff, confirm its digest before review, remain read-only, and bind every finding to file/hunk evidence and severity. Treat a missing result or digest mismatch as no verdict.
Approval binds to base, product HEAD, clean status, tree hash, and diff digest. Permit a later closure commit outside that fingerprint only after proving it changes the workflow record alone.
## Findings and remediation
Validate findings against repository evidence. Send accepted fixes to an original implementer or another non-reviewer, never the reviewer. Preserve exclusive path ownership and assignment evidence. Rerun affected checks and the full required suite, create focused commits, and generate a new fingerprint.
Launch fresh independent reviewers for the changed diff. Escalate after two unsuccessful remediation cycles or whenever a finding requires material scope, acceptance, ownership, verification, or architecture changes; return to plan approval before further production mutation.
@@ -0,0 +1,438 @@
import { createHash, randomUUID } from "node:crypto";
import { promises as fs } from "node:fs";
import { dirname, resolve } from "node:path";
import { fileURLToPath } from "node:url";
/** The headings that make up a retained develop-workflow record. */
export const REQUIRED_SECTIONS = Object.freeze([
"Status",
"Request and scope",
"Git identity",
"Acceptance criteria",
"Approved implementation plan and gates",
"Ownership",
"Verification matrix",
"Review",
"Integration and rollback",
"Checkpoints and blockers",
]);
const REQUIRED_SECTION_SET = new Set(REQUIRED_SECTIONS);
const FILE_MODE = 0o600;
const MAX_TEMP_NAME_ATTEMPTS = 16;
/**
* Error raised for a checkpoint precondition or atomic-update failure.
* `code` is stable for callers that need to distinguish a stale record or lock.
*/
export class CheckpointError extends Error {
constructor(code, message, options = {}) {
super(message, options);
this.name = "CheckpointError";
this.code = code;
}
}
const nodeIo = Object.freeze({
mkdir: (...args) => fs.mkdir(...args),
open: (...args) => fs.open(...args),
readFile: (...args) => fs.readFile(...args),
rename: (...args) => fs.rename(...args),
unlink: (...args) => fs.unlink(...args),
randomUUID: () => randomUUID(),
});
/** The default filesystem adapter, useful when composing or testing the helper. */
export const defaultIo = nodeIo;
function asBytes(input) {
if (typeof input === "string") return Buffer.from(input, "utf8");
if (input instanceof Uint8Array) return Buffer.from(input);
if (input instanceof ArrayBuffer) return Buffer.from(new Uint8Array(input));
throw new CheckpointError(
"INVALID_INPUT",
"checkpoint input must be a UTF-8 string or byte array",
);
}
function headingText(line, level) {
const prefix = "#".repeat(level);
const match = line.match(new RegExp(`^${prefix}(?!#)[ \\t]+(.+?)[ \\t]*$`));
if (!match) return null;
// Closing ATX hashes are syntax, not part of the heading title.
return match[1].replace(/[ \\t]+#+[ \\t]*$/, "").trim();
}
/**
* Validate a retained record without accessing the destination path.
* Markdown headings inside fenced code blocks are not record sections.
*/
export function validateRecord(input) {
const bytes = asBytes(input);
let text;
try {
text = new TextDecoder("utf-8", { fatal: true }).decode(bytes);
} catch (error) {
throw new CheckpointError("INVALID_RECORD", "checkpoint input is not valid UTF-8", {
cause: error,
});
}
const h1 = [];
const h2 = [];
let fence = null;
for (const line of text.split(/\r?\n/)) {
const fenceMatch = line.match(/^\s*(`{3,}|~{3,})/);
if (fenceMatch) {
const marker = fenceMatch[1][0];
if (fence === null) fence = marker;
else if (fence === marker) fence = null;
continue;
}
if (fence !== null) continue;
const first = headingText(line, 1);
if (first !== null) h1.push(first);
const second = headingText(line, 2);
if (second !== null) h2.push(second);
}
if (h1.length !== 1 || h1[0].length === 0) {
throw new CheckpointError(
"INVALID_RECORD",
`checkpoint must contain exactly one non-empty H1 (found ${h1.length})`,
);
}
const counts = new Map();
for (const section of h2) counts.set(section, (counts.get(section) ?? 0) + 1);
const missing = REQUIRED_SECTIONS.filter((section) => !counts.has(section));
const duplicate = REQUIRED_SECTIONS.filter((section) => (counts.get(section) ?? 0) > 1);
const unknown = h2.filter((section) => !REQUIRED_SECTION_SET.has(section));
if (missing.length > 0 || duplicate.length > 0 || unknown.length > 0) {
const details = [];
if (missing.length > 0) details.push(`missing: ${missing.join(", ")}`);
if (duplicate.length > 0) details.push(`duplicate: ${duplicate.join(", ")}`);
if (unknown.length > 0) details.push(`unknown: ${[...new Set(unknown)].join(", ")}`);
throw new CheckpointError("INVALID_RECORD", `invalid checkpoint sections (${details.join("; ")})`);
}
return { bytes, text, h1: h1[0], sections: [...REQUIRED_SECTIONS] };
}
function sha256(bytes) {
return createHash("sha256").update(bytes).digest("hex");
}
function expectedDigest(value) {
if (value === undefined || value === null) return undefined;
if (typeof value !== "string" || !/^[0-9a-f]{64}$/i.test(value)) {
throw new CheckpointError("INVALID_DIGEST", "expected SHA-256 must be exactly 64 hexadecimal characters");
}
return value.toLowerCase();
}
function errorCode(error) {
return error && typeof error === "object" && "code" in error ? error.code : undefined;
}
function isMissing(error) {
return errorCode(error) === "ENOENT";
}
function isAlreadyExists(error) {
return errorCode(error) === "EEXIST";
}
function messageFor(error) {
return error instanceof Error ? error.message : String(error);
}
function withIo(io) {
return { ...nodeIo, ...io };
}
async function closeQuietly(handle) {
if (handle === null) return undefined;
try {
await handle.close();
} catch (error) {
return error;
}
return undefined;
}
async function unlinkQuietly(io, path) {
try {
await io.unlink(path);
return undefined;
} catch (error) {
return error;
}
}
async function readExisting(io, target) {
try {
return { exists: true, bytes: await io.readFile(target) };
} catch (error) {
if (isMissing(error)) return { exists: false, bytes: undefined };
throw new CheckpointError("READ_FAILED", `could not read checkpoint target: ${messageFor(error)}`, {
cause: error,
});
}
}
async function openUniqueTemp(io, target) {
for (let attempt = 0; attempt < MAX_TEMP_NAME_ATTEMPTS; attempt += 1) {
const id = io.randomUUID();
const path = `${target}.tmp-${process.pid}-${id}`;
try {
const handle = await io.open(path, "wx", FILE_MODE);
return { path, handle };
} catch (error) {
// A collision is not stale-file cleanup. Leave the old file alone and try a new name.
if (isAlreadyExists(error)) continue;
throw new CheckpointError("TEMP_OPEN_FAILED", `could not create checkpoint temporary file: ${messageFor(error)}`, {
cause: error,
});
}
}
throw new CheckpointError("TEMP_OPEN_FAILED", "could not create a unique checkpoint temporary file");
}
async function writeAll(handle, bytes) {
let offset = 0;
while (offset < bytes.byteLength) {
const result = await handle.write(bytes, offset, bytes.byteLength - offset);
const written = typeof result === "number" ? result : result.bytesWritten;
if (!Number.isInteger(written) || written <= 0) {
throw new CheckpointError("WRITE_FAILED", "checkpoint temporary file write made no progress");
}
offset += written;
}
}
async function syncDirectory(io, directory) {
let handle = null;
let operationError;
try {
handle = await io.open(directory, "r");
await handle.sync();
} catch (error) {
operationError = new CheckpointError("DIRECTORY_SYNC_FAILED", `could not flush checkpoint directory: ${messageFor(error)}`, {
cause: error,
});
}
const closeError = await closeQuietly(handle);
if (operationError !== undefined) throw operationError;
if (closeError !== undefined) {
throw new CheckpointError("DIRECTORY_CLOSE_FAILED", `could not close checkpoint directory: ${messageFor(closeError)}`, {
cause: closeError,
});
}
}
/** Atomically create or replace a validated workflow record. */
export async function checkpointRecord(request) {
if (request === null || typeof request !== "object") {
throw new CheckpointError("INVALID_REQUEST", "checkpoint request must provide a target and input");
}
if (typeof request.target !== "string" || request.target.length === 0) {
throw new CheckpointError("INVALID_REQUEST", "checkpoint target is required");
}
const target = resolve(request.target);
// Validate before opening the lock, creating a directory, or reading the destination.
const { bytes } = validateRecord(request.input);
const expected = expectedDigest(request.expectedSha256);
const io = withIo(request.io);
const lock = `${target}.lock`;
const directory = dirname(target);
try {
await io.mkdir(directory, { recursive: true, mode: 0o700 });
} catch (error) {
throw new CheckpointError(
"DIRECTORY_CREATE_FAILED",
`could not create checkpoint directory: ${messageFor(error)}`,
{ cause: error },
);
}
let lockHandle = null;
let tempHandle = null;
let tempPath = null;
let primaryError;
let result;
try {
try {
lockHandle = await io.open(lock, "wx", FILE_MODE);
} catch (error) {
if (isAlreadyExists(error)) {
throw new CheckpointError("LOCK_EXISTS", "checkpoint target is locked; refusing to recover or overwrite the lock", {
cause: error,
});
}
throw new CheckpointError("LOCK_FAILED", `could not acquire checkpoint lock: ${messageFor(error)}`, {
cause: error,
});
}
const current = await readExisting(io, target);
if (current.exists) {
if (expected === undefined) {
throw new CheckpointError(
"EXPECTED_SHA256_REQUIRED",
"replacing an existing checkpoint requires --expected-sha256",
);
}
const actual = sha256(current.bytes);
if (actual !== expected) {
throw new CheckpointError(
"STALE_DIGEST",
`checkpoint target digest is stale (expected ${expected}, found ${actual})`,
);
}
} else if (expected !== undefined) {
throw new CheckpointError(
"TARGET_MISSING",
"checkpoint target is absent; do not provide an expected SHA-256 when creating it",
);
}
const temporary = await openUniqueTemp(io, target);
tempPath = temporary.path;
tempHandle = temporary.handle;
let temporaryError;
try {
await writeAll(tempHandle, bytes);
} catch (error) {
temporaryError = error instanceof CheckpointError
? error
: new CheckpointError("WRITE_FAILED", `could not write checkpoint temporary file: ${messageFor(error)}`, { cause: error });
}
if (temporaryError === undefined) {
try {
await tempHandle.sync();
} catch (error) {
temporaryError = new CheckpointError("TEMP_SYNC_FAILED", `could not flush checkpoint temporary file: ${messageFor(error)}`, {
cause: error,
});
}
}
const closeError = await closeQuietly(tempHandle);
tempHandle = null;
if (temporaryError !== undefined) throw temporaryError;
if (closeError !== undefined) {
throw new CheckpointError("TEMP_CLOSE_FAILED", `could not close checkpoint temporary file: ${messageFor(closeError)}`, {
cause: closeError,
});
}
try {
await io.rename(tempPath, target);
} catch (error) {
throw new CheckpointError("RENAME_FAILED", `could not atomically replace checkpoint target: ${messageFor(error)}`, {
cause: error,
});
}
tempPath = null;
await syncDirectory(io, directory);
result = {
target,
created: !current.exists,
sha256: sha256(bytes),
};
} catch (error) {
primaryError = error;
}
// Cleanup is best effort. In particular, never remove a pre-existing stale temp
// or lock and never replace an operation error with a cleanup error.
const tempCloseError = await closeQuietly(tempHandle);
if (tempCloseError !== undefined && primaryError === undefined) {
primaryError = new CheckpointError("TEMP_CLOSE_FAILED", `could not close checkpoint temporary file: ${messageFor(tempCloseError)}`, {
cause: tempCloseError,
});
}
if (tempPath !== null) {
const tempUnlinkError = await unlinkQuietly(io, tempPath);
if (tempUnlinkError !== undefined && !isMissing(tempUnlinkError) && primaryError === undefined) {
primaryError = new CheckpointError("TEMP_CLEANUP_FAILED", `could not remove checkpoint temporary file: ${messageFor(tempUnlinkError)}`, {
cause: tempUnlinkError,
});
}
}
const lockCloseError = await closeQuietly(lockHandle);
if (lockCloseError !== undefined && primaryError === undefined) {
primaryError = new CheckpointError("LOCK_CLOSE_FAILED", `could not close checkpoint lock: ${messageFor(lockCloseError)}`, {
cause: lockCloseError,
});
}
if (lockHandle !== null) {
const lockUnlinkError = await unlinkQuietly(io, lock);
if (lockUnlinkError !== undefined && !isMissing(lockUnlinkError) && primaryError === undefined) {
primaryError = new CheckpointError("LOCK_CLEANUP_FAILED", `could not remove checkpoint lock: ${messageFor(lockUnlinkError)}`, {
cause: lockUnlinkError,
});
}
}
if (primaryError !== undefined) throw primaryError;
return result;
}
function usage() {
return "Usage: node checkpoint-record.mjs <target> [--input <path>] [--expected-sha256 <digest>]";
}
function parseCli(argv) {
let target;
let inputPath;
let expectedSha256;
for (let index = 0; index < argv.length; index += 1) {
const argument = argv[index];
if (argument === "--input") {
if (inputPath !== undefined || index + 1 >= argv.length || argv[index + 1].startsWith("--")) {
throw new CheckpointError("CLI_USAGE", usage());
}
inputPath = argv[++index];
} else if (argument === "--expected-sha256") {
if (expectedSha256 !== undefined || index + 1 >= argv.length || argv[index + 1].startsWith("--")) {
throw new CheckpointError("CLI_USAGE", usage());
}
expectedSha256 = argv[++index];
} else if (argument.startsWith("--")) {
throw new CheckpointError("CLI_USAGE", usage());
} else if (target === undefined) {
target = argument;
} else {
throw new CheckpointError("CLI_USAGE", usage());
}
}
if (target === undefined) throw new CheckpointError("CLI_USAGE", usage());
return { target, inputPath, expectedSha256 };
}
async function readStdin() {
const chunks = [];
for await (const chunk of process.stdin) chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
return Buffer.concat(chunks);
}
export async function main(argv = process.argv.slice(2)) {
try {
const { target, inputPath, expectedSha256 } = parseCli(argv);
const input = inputPath === undefined ? await readStdin() : await fs.readFile(inputPath);
await checkpointRecord({ target, input, expectedSha256 });
return 0;
} catch (error) {
process.stderr.write(`checkpoint-record: ${messageFor(error)}\n`);
return 2;
}
}
const invokedPath = process.argv[1] === undefined ? null : resolve(process.argv[1]);
if (invokedPath !== null && invokedPath === fileURLToPath(import.meta.url)) {
process.exitCode = await main();
}
@@ -0,0 +1,303 @@
import assert from "node:assert/strict";
import { createHash } from "node:crypto";
import { promises as fs } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { fileURLToPath } from "node:url";
import { once } from "node:events";
import { spawn } from "node:child_process";
import test from "node:test";
import {
checkpointRecord,
defaultIo,
REQUIRED_SECTIONS,
} from "../scripts/checkpoint-record.mjs";
const scriptPath = fileURLToPath(new URL("../scripts/checkpoint-record.mjs", import.meta.url));
function record(title = "Workflow checkpoint", sections = REQUIRED_SECTIONS) {
return `# ${title}\n\n${sections.map((section) => `## ${section}\n\ncheckpoint data\n`).join("\n")}`;
}
function digest(value) {
return createHash("sha256").update(value).digest("hex");
}
async function temporaryDirectory() {
return fs.mkdtemp(join(tmpdir(), "checkpoint-record-test-"));
}
async function exists(path) {
try {
await fs.access(path);
return true;
} catch {
return false;
}
}
function faultIo({ write, flush, rename, directoryFlush, cleanup } = {}) {
return {
...defaultIo,
open: async (path, flags, mode) => {
const handle = await defaultIo.open(path, flags, mode);
const isTemp = flags === "wx" && path.includes(".tmp-");
const isDirectory = flags === "r";
if (!(isTemp && (write || flush)) && !(isDirectory && directoryFlush)) return handle;
return {
write: async (...args) => {
if (isTemp && write) throw new Error("interrupted write");
return handle.write(...args);
},
sync: async () => {
if ((isTemp && flush) || (isDirectory && directoryFlush)) throw new Error("interrupted flush");
return handle.sync();
},
close: (...args) => handle.close(...args),
};
},
rename: async (...args) => {
if (rename) throw new Error("rename failed");
return defaultIo.rename(...args);
},
unlink: async (path) => {
if (cleanup) throw new Error("cleanup failed");
return defaultIo.unlink(path);
},
};
}
async function assertRejectsCode(promise, code) {
await assert.rejects(promise, (error) => {
assert.equal(error.code, code);
return true;
});
}
test("creates a missing record and parent directory atomically with restrictive modes", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "nested", "record.md");
const input = record();
const result = await checkpointRecord({ target, input });
assert.equal(result.created, true);
assert.equal(result.sha256, digest(input));
assert.equal(await fs.readFile(target, "utf8"), input);
assert.equal((await fs.stat(target)).mode & 0o777, 0o600);
assert.equal((await fs.stat(join(directory, "nested"))).mode & 0o777, 0o700);
assert.equal(await exists(`${target}.lock`), false);
assert.equal(
(await fs.readdir(join(directory, "nested"))).filter((name) => name.includes(".tmp-")).length,
0,
);
});
test("replaces a record only with its current SHA-256 digest", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const before = record("Before");
const after = record("After");
await fs.writeFile(target, before, { mode: 0o600 });
const result = await checkpointRecord({ target, input: after, expectedSha256: digest(before) });
assert.equal(result.created, false);
assert.equal(await fs.readFile(target, "utf8"), after);
assert.equal(result.sha256, digest(after));
});
test("rejects malformed, missing, duplicate, and unknown sections before touching the target", async () => {
const cases = [
["missing H1", record().replace(/^# .*\n\n/, "")],
["missing section", record().replace("## Review\n\ncheckpoint data\n\n", "")],
["duplicate section", `${record()}\n## Status\n\nagain\n`],
["unknown section", `${record()}\n## Unrecognized\n\nnope\n`],
];
for (const [name, input] of cases) {
const directory = await temporaryDirectory();
const target = join(directory, `${name.replaceAll(" ", "-")}.md`);
await assertRejectsCode(checkpointRecord({ target, input }), "INVALID_RECORD");
assert.equal(await exists(target), false, `${name} created target`);
assert.equal(await exists(`${target}.lock`), false, `${name} created lock`);
}
});
test("requires a digest for replacement and rejects stale digests without changing bytes", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const before = record("Before");
const after = record("After");
await fs.writeFile(target, before, { mode: 0o600 });
await assertRejectsCode(checkpointRecord({ target, input: after }), "EXPECTED_SHA256_REQUIRED");
await assertRejectsCode(
checkpointRecord({ target, input: after, expectedSha256: digest(record("Other")) }),
"STALE_DIGEST",
);
assert.equal(await fs.readFile(target, "utf8"), before);
assert.equal(await exists(`${target}.lock`), false);
});
test("serializes concurrent updates with an exclusive per-target lock", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const lock = `${target}.lock`;
const first = record("First");
const second = record("Second");
let firstLockOpened;
const lockOpened = new Promise((resolve) => { firstLockOpened = resolve; });
let releaseFirstLock;
const firstLockHeld = new Promise((resolve) => { releaseFirstLock = resolve; });
let firstOpen = true;
const io = {
...defaultIo,
open: async (path, flags, mode) => {
const handle = await defaultIo.open(path, flags, mode);
if (path === lock && flags === "wx" && firstOpen) {
firstOpen = false;
firstLockOpened();
await firstLockHeld;
}
return handle;
},
};
const firstPromise = checkpointRecord({ target, input: first, io });
await lockOpened;
const secondPromise = checkpointRecord({ target, input: second, io });
await assertRejectsCode(secondPromise, "LOCK_EXISTS");
releaseFirstLock();
await firstPromise;
assert.equal(await fs.readFile(target, "utf8"), first);
assert.equal(await exists(`${target}.lock`), false);
});
test("fails closed on a pre-existing stale lock and leaves it for recovery", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const lock = `${target}.lock`;
const input = record();
await fs.writeFile(lock, "stale lock", { mode: 0o600 });
await assertRejectsCode(checkpointRecord({ target, input }), "LOCK_EXISTS");
assert.equal(await fs.readFile(lock, "utf8"), "stale lock");
assert.equal(await exists(target), false);
});
test("preserves pre-existing stale temporary files", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const stale = `${target}.tmp-stale-recovery`;
await fs.writeFile(stale, "recover me", { mode: 0o600 });
await checkpointRecord({ target, input: record() });
assert.equal(await fs.readFile(stale, "utf8"), "recover me");
assert.equal(await exists(`${target}.lock`), false);
});
test("cleans its own temporary file after an interrupted write and preserves the prior target", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const before = record("Before");
await fs.writeFile(target, before, { mode: 0o600 });
await assertRejectsCode(
checkpointRecord({ target, input: record("After"), expectedSha256: digest(before), io: faultIo({ write: true }) }),
"WRITE_FAILED",
);
assert.equal(await fs.readFile(target, "utf8"), before);
assert.equal(await exists(`${target}.lock`), false);
assert.deepEqual((await fs.readdir(directory)).filter((name) => name.includes(".tmp-")), []);
});
test("cleans its own temporary file after a flush failure and does not mask the primary error", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const before = record("Before");
await fs.writeFile(target, before, { mode: 0o600 });
const cleanupFailingIo = faultIo({ flush: true, cleanup: true });
await assertRejectsCode(
checkpointRecord({ target, input: record("After"), expectedSha256: digest(before), io: cleanupFailingIo }),
"TEMP_SYNC_FAILED",
);
assert.equal(await fs.readFile(target, "utf8"), before);
// Remove artifacts made intentionally unremovable by this fault adapter.
for (const name of await fs.readdir(directory)) await fs.unlink(join(directory, name));
});
test("rename failure leaves the previous target byte-for-byte intact", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const before = record("Before");
await fs.writeFile(target, before, { mode: 0o600 });
// The adapter's rename failure is wrapped as an atomic operation error.
await assert.rejects(
checkpointRecord({ target, input: record("After"), expectedSha256: digest(before), io: faultIo({ rename: true }) }),
(error) => {
assert.equal(error.code, "RENAME_FAILED");
assert.match(error.message, /rename failed/);
return true;
},
);
assert.equal(await fs.readFile(target, "utf8"), before);
assert.equal(await exists(`${target}.lock`), false);
});
test("directory flush failure is reported after rename and leaves no lock or owned temp", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const before = record("Before");
const after = record("After");
await fs.writeFile(target, before, { mode: 0o600 });
await assertRejectsCode(
checkpointRecord({ target, input: after, expectedSha256: digest(before), io: faultIo({ directoryFlush: true }) }),
"DIRECTORY_SYNC_FAILED",
);
assert.equal(await fs.readFile(target, "utf8"), after);
assert.equal(await exists(`${target}.lock`), false);
assert.deepEqual((await fs.readdir(directory)).filter((name) => name.includes(".tmp-")), []);
});
test("cleanup errors never mask a lock or write failure", async () => {
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const io = faultIo({ write: true, cleanup: true });
await assert.rejects(checkpointRecord({ target, input: record(), io }), /interrupted write/);
for (const name of await fs.readdir(directory)) await fs.unlink(join(directory, name));
});
test("strict CLI reports usage and operation errors with exit code 2", async () => {
const missingTarget = spawn(process.execPath, [scriptPath], { stdio: ["ignore", "pipe", "pipe"] });
const [missingResult] = await once(missingTarget, "close");
assert.equal(missingResult, 2);
const directory = await temporaryDirectory();
const target = join(directory, "record.md");
const child = spawn(process.execPath, [scriptPath, target], { stdio: ["pipe", "pipe", "pipe"] });
child.stdin.end(record());
const [result] = await once(child, "close");
assert.equal(result, 0);
assert.equal(await fs.readFile(target, "utf8"), record());
const inputPath = join(directory, "input.md");
const inputRecord = record("From input file");
await fs.writeFile(inputPath, inputRecord);
const inputTarget = join(directory, "input-target.md");
const inputChild = spawn(process.execPath, [scriptPath, inputTarget, "--input", inputPath], { stdio: ["ignore", "pipe", "pipe"] });
const [inputResult] = await once(inputChild, "close");
assert.equal(inputResult, 0);
assert.equal(await fs.readFile(inputTarget, "utf8"), inputRecord);
const bad = spawn(process.execPath, [scriptPath, target, "--expected-sha256", "bad"], { stdio: ["pipe", "pipe", "pipe"] });
bad.stdin.end(record("Changed"));
const [badResult] = await once(bad, "close");
assert.equal(badResult, 2);
});
+1 -1
View File
@@ -1,6 +1,6 @@
---
name: artifact-presenter
description: Present local artifacts for review: use Browser Harness for interactive web artifacts, Orca for editor-friendly text in Orca-managed sessions, and mcat for other non-web files.
description: "Present local artifacts for review: use Browser Harness for interactive web artifacts, Orca for editor-friendly text in Orca-managed sessions, and mcat for other non-web files."
---
# Artifact presenter