Files
harness-tools/docs/workflow/develop-workflow-record.md
T

26 KiB
Raw Blame History

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 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, 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:
    • ebbf3c3d19 refactor(skills): simplify checkpoint locking
    • c257f31c21 docs: approve checkpoint simplification
    • 435a12107f docs: approve cooperative checkpoint locks
    • 26eeab209e docs: record lock remediation rejection
    • 4178ee4857 docs: pin exceptional remediation review
    • 52576c7556 fix(skills): preserve replaced checkpoint locks
    • 444483ade6 docs: approve focused lock remediation
    • ebab671969 docs: record post-rebase review finding
    • b556bcf147 docs: pin post-rebase develop workflow review
    • b4e71b47db docs: record dirty source resolution
    • 1782d1a0c6 docs: record final develop workflow approval
    • a241fb05ae docs: pin final develop workflow review
    • 7a58ec0eef fix(skills): remove checkpoint helper dead code
    • 08ebfb50ad docs: record second develop workflow review
    • e65ea4c0aa docs: pin remediated develop workflow review
    • 1e213db45a fix(skills): address develop workflow review
    • 1bab01799c docs: record develop workflow review findings
    • 353278aa96 docs: pin develop workflow review brief
    • f8a4362287 docs: checkpoint develop workflow verification
    • 12495473ee docs: supersede the legacy workflow engine
    • f1b525cec9 feat(skills): enforce independent workflow review
    • b6bb5db033 feat(subagents): add independent reviewer profile
    • 8c44c07c6a feat(skills): add develop workflow
    • a794a1e10a docs: checkpoint develop workflow implementation
    • 6389d22f15 docs: checkpoint develop workflow plan approval
    • fbf1a52b64 docs: record develop workflow implementation plan
    • 06e631504c 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.