Plan: Review Contract and Handoff Consistency

Initial Situation

The public review validator accepts eight invalid or ambiguous inputs. Distributed review and delivery guidance also contains three contradictions: installed handoff text claims source-only tests, debt follow-up has no durable resume contract, and two subagent manifest paths name obsolete skill-routing formats.

The Harness branch is the third item in this stack:

main → team/stefan/cache-release-diff-classification-requirements → team/stefan/scaffold-skill-operator-followups → team/stefan/review-contract-handoff-consistency

The dirty detached root checkout is outside the execution surface. All Harness work uses /private/tmp/hi-review-contract.ydwSZ0/repo. Reusable skill work uses a new clean shared-skills worktree based on immutable commit 408746ad5fdf10d75513cd1b63c71c6f22fa7fb7.

Problem and Solution Shape

The validator currently checks related evidence independently, so structurally valid values can describe no real review or contradictory evidence. Deepen the validator's public boundary: normalize non-empty identities, validate real UTC instants, classify each finding, derive aggregate routing in one place, and compare local report bytes with bytes resolved from the claimed commit tree.

The workflow repair stays source-first. Shared review/delivery instructions gain one durable debt branch owned by delivery and remove installed-local test claims. Harness then synchronizes the immutable shared commit and aligns both subagent manifest sources to the stable ## Skills row-trigger concept. Supported generators refresh hashes and projections after semantic work is green.

Resolved Decision Ledger

No product decision remains open. The user explicitly approved fixing both issues.

Codebase Findings

• Canonical validator and contract tests live in wearedevpunks-skills under skills/phases/review-phase/scripts/review-contract.mjs and tests/review-phase-graph.contract.test.mjs.
• The parser uses an ordinary record, source and delivery-scope normalizers allow empty arrays, timestamp validation is shape-only, ref containment is truthy, and retained local bytes are not compared with commit-tree bytes.
• Findings have no route field. Aggregate routing is validated as independent input.
• Review emits debt_follow_up; delivery durable state and router do not resume it.
• Installed skill packages exclude the shared repository's tests/ directory.
• Harness dynamic manifest text names Skill | What / when; the bundled fallback names Primary skills here. PR #127's current prompt contract uses a ## Skills table with exact triggers.
• Harness's default shared-skill pin still points at the earlier review-phase source commit. The exact-ref sync seam already accepts fully qualified immutable tags.

Research and Ambiguity Reduction

Two readonly discovery lanes reproduced #129 and #130 independently. They agreed on source ownership, test seams, delivery-owned debt capture, and header-tolerant manifest wording. parallel-research was considered but not activated as a new run because its mandatory durable report would duplicate the already-retained issues and spec. No external library behavior is involved, so no external dependency research is required.

The grilling frontier is empty: scope, interface outcomes, ownership, stack topology, and release limits are all fixed by the issues, accepted spec, and repository contracts.

Dependency Readiness

Ready.

• Parent Harness branch: team/stefan/scaffold-skill-operator-followups at 18ec41b0.
• Child Harness branch: team/stefan/review-contract-handoff-consistency.
• Retained spec commit: 5a7c5facff5bb3b56b2a734fd046908d436b7fe5.
• Shared-skill implementation base: 408746ad5fdf10d75513cd1b63c71c6f22fa7fb7.
• Issues #129 and #130 are open and link the retained spec.

Branch/Base Intent

• Keep this Harness branch based on team/stefan/scaffold-skill-operator-followups until PR #127 merges.
• Push shared-skill changes on a dedicated team/stefan/* branch based on the preceding immutable shared commit. Create an immutable sync tag at the pushed head.
• Before final push, fetch and sync the Harness stack bottom-up if either parent moved.
• After a parent merge, rebase and retarget only its direct child, preserving stack order.

Dependency Graph

T1 shared validator and handoff ───── T3 exact-ref sync and pin ──┐
                                                                  ├── T4 generated assets and docs ── T5 frozen closeout
T2 Harness manifest contract ─────────────────────────────────────┘

Parallel Execution Waves

Tasks

T1: Repair the canonical review and delivery contracts

• depends_on: []
• location: /Users/stefan/Desktop/repos/wearedevpunks-skills
• owned_paths:
  • skills/phases/review-phase/scripts/review-contract.mjs
  • skills/phases/review-phase/AUTHORING-HANDOFF.md
  • skills/phases/review-phase/phases/run-review.md
  • skills/phases/review-phase/phases/retain-report.md
  • skills/phases/review-phase/phases/return-route.md
  • skills/phases/review-phase/references/durable-report.md
  • skills/phases/review-phase/references/state-graph.md
  • skills/phases/delivery-phase/references/phase-handoff.md
  • skills/phases/delivery-phase/phases/router.md
  • skills/phases/delivery-phase/phases/review.md
  • tests/review-phase-graph.contract.test.mjs
• wave_boundary: W1
• description: From clean worktree /private/tmp/hi-review-contract-skills at the preceding immutable shared commit, add vertical RED/GREEN slices for all eight #129 bypasses. Add exact commit-tree byte input, structured finding route classification, centralized route derivation, and the durable delivery debt-capture/resume contract. Make the retention phase resolve reportCommitSha:reportPath and pass those exact bytes to the validator. Rewrite installed handoff evidence so it names reproducible source provenance without claiming packaged local suites. Push a dedicated source branch and immutable tag.
• validation: All focused contract cases fail before their slice and pass after it; source AI instructions agree on the route schema, precedence, byte proof, and debt state; the retention gate reads the committed blob and proves exact local/committed equality; the pushed tag resolves to the tested commit.
• status: Complete
• log: 2026-08-12 Seventeen vertical RED/GREEN slices completed, including frozen-review repairs for malformed scopes, durable debt routing, cold resume, and canonical activation vocabulary. Shared 119/119 passed. Pushed commit 684f98dff76a and immutable sync tag.
• files edited/created: skills/phases/review-phase/scripts/review-contract.mjs, skills/phases/review-phase/AUTHORING-HANDOFF.md, review run/retain/return and durable-report/state-graph references, delivery phase-handoff/router/review, tests/review-phase-graph.contract.test.mjs
• backlog_item_id: #129, #130
• backlog_item_url: https://github.com/wearedevpunks/harness-intelligence/issues/129, https://github.com/wearedevpunks/harness-intelligence/issues/130
• relation_mode: body-links
• assigned_skills: [writing-for-agents, tdd, codebase-design, quality-types, simplify]
• implementation_skill_guidance:
  • skill: writing-for-agents applicable_behavior: Apply the instruction and skill-mechanics contracts before every AI-context Markdown edit; keep decision branches explicit and installed bodies authoritative.
  • skill: tdd applicable_behavior: Implement one public validator or workflow behavior per observed RED/GREEN slice; never batch production edits ahead of RED evidence.
  • skill: codebase-design applicable_behavior: Keep commit-byte resolution as explicit boundary input and centralize route derivation behind one small public seam.
  • skill: quality-types applicable_behavior: Make invalid route and retention states unrepresentable at normalization boundaries without duplicating derivable aggregate state.
  • skill: simplify applicable_behavior: After GREEN, remove only duplication introduced by the new validators and route derivation; preserve public behavior.
• tdd_status: required
• tdd_target: The canonical public validator rejects one previously accepted invalid review, beginning with empty delivery scope, then advances one tracer bullet per remaining gap and debt resume behavior.
• red_command: node --test tests/review-phase-graph.contract.test.mjs
• expected_red_failure: New assertions show empty scopes/sources/goal identity and invalid UTC are accepted; prototype-sensitive frontmatter mutates parsing; string "false", mismatched commit bytes, contradictory routing, or missing debt resume are accepted.
• green_command: node --test tests/review-phase-graph.contract.test.mjs && node --test tests/*.test.mjs
• reason_not_testable:
• red_evidence: Focused runs observed the eight validator gaps and missing durable debt contract. Frozen reviews then reproduced null and empty scope failures, missing post-debt resume state, and mixed repair-plus-debt transitions that bypassed capture before each repair.
• green_evidence: Tested/tagged head passed the focused canonical-consumer contract and 119/119 full shared tests; Node syntax and diff checks passed; remote branch and tag both resolve to 684f98dff76ac94ad3db2eb1f74230cb9910e1ad.
• codebase_design_notes: The validator is the deep public module. Callers supply bytes resolved from the claimed commit; the module compares them exactly. Finding routes are primitive input and aggregate routing is derived output.
• review_mode: cli
• runtime_validation: not_required
• runtime_target: not_applicable
• runtime_evidence: not_applicable
• runtime_cleanup: not_applicable

T2: Align dynamic and bundled subagent activation

• depends_on: []
• location: apps/cli/src/content, apps/cli/src/data/subagents
• owned_paths:
  • apps/cli/src/content/subagents.ts
  • apps/cli/src/content/content.test.ts
  • apps/cli/src/data/subagents/manifest.mjs
  • apps/cli/src/data/subagents/manifest.test.ts
  • apps/cli/src/scaffold/run.test.ts
• wave_boundary: W1
• description: Add public-output RED assertions for both manifest sources, then make them direct agents to the stable ## Skills table and a matching row trigger. Exclude Primary skills here and What / when; keep progressive disclosure to selected installed SKILL.md bodies. Extend the isolated scaffold consumer test to read the generated .agents/subagents/manifest.mjs and assert the same activation semantics.
• validation: Dynamic, bundled, and isolated generated-consumer outputs satisfy the same semantic assertions and preserve existing manifest capability/skill integrity.
• status: Complete
• log: 2026-08-12 RED 3 failed/54 passed after dependency install; GREEN 57/57 plus CLI typecheck and diff check.
• files edited/created: apps/cli/src/content/subagents.ts, apps/cli/src/content/content.test.ts, apps/cli/src/data/subagents/manifest.mjs, apps/cli/src/data/subagents/manifest.test.ts, apps/cli/src/scaffold/run.test.ts
• backlog_item_id: #130
• backlog_item_url: https://github.com/wearedevpunks/harness-intelligence/issues/130
• relation_mode: body-links
• assigned_skills: [writing-for-agents, tdd, codebase-design, simplify]
• implementation_skill_guidance:
  • skill: writing-for-agents applicable_behavior: Treat JavaScript instruction strings as AI-context Markdown; use one visible trigger branch and one exact disclosure target.
  • skill: tdd applicable_behavior: Capture separate public-output RED evidence for the dynamic renderer and bundled fallback before changing either production source.
  • skill: codebase-design applicable_behavior: Keep the two existing ownership seams but assert one shared semantic contract; do not add a new generator abstraction for two strings.
  • skill: simplify applicable_behavior: Remove obsolete labels without broad manifest refactoring.
• tdd_status: required
• tdd_target: A generated specialist, bundled fallback, and isolated scaffold consumer activate skills from ## Skills row triggers and never search obsolete labels.
• red_command: bun run --cwd apps/cli test src/content/content.test.ts src/data/subagents/manifest.test.ts src/scaffold/run.test.ts
• expected_red_failure: Dynamic output still contains What / when; bundled fallback and isolated consumer still contain Primary skills here; all miss the new shared semantic assertions.
• green_command: bun run --cwd apps/cli test src/content/content.test.ts src/data/subagents/manifest.test.ts src/scaffold/run.test.ts && bun run --cwd apps/cli check-types
• reason_not_testable:
• red_evidence: Focused command exited 1 with three public-output failures: dynamic What / when, bundled Primary skills here, and stale isolated consumer guidance.
• green_evidence: Same three-file command passed 57/57; bun run --cwd apps/cli check-types and git diff --check passed.
• codebase_design_notes: Dynamic and bundled sources are distinct public asset seams. Test semantic parity without coupling their implementation storage.
• review_mode: cli
• runtime_validation: not_required
• runtime_target: not_applicable
• runtime_evidence: not_applicable
• runtime_cleanup: not_applicable

T3: Synchronize the immutable shared source and update its default pin

• depends_on: [T1]
• location: apps/cli/scripts, apps/cli/skills, apps/cli/src/scripts
• owned_paths:
  • apps/cli/scripts/sync-skills-repo.mjs
  • apps/cli/src/scripts/sync-skills-repo.test.ts
  • apps/cli/skills/phases/review-phase/**
  • apps/cli/skills/phases/delivery-phase/**
• wave_boundary: W2
• description: Point the default source selection at T1's pushed branch and exact commit, add the pin contract RED, then run supported sync from the fully qualified immutable tag. Retain only generated shared-skill changes and verify the sync receipt.
• validation: Default selection fails closed on any other commit; receipt records the immutable tag and exact T1 commit; synchronized review/delivery files are byte-identical to canonical source; no active .agents or pre-existing snapshot is hand-edited.
• status: Complete
• log: 2026-08-12 Default-pin tests captured the old selection before each source revision and passed 9/9 at final commit 684f98dff76a. Exact immutable-tag sync completed with full canonical/package parity; fixture check remained expected RED for W3.
• files edited/created: apps/cli/scripts/sync-skills-repo.mjs, apps/cli/src/scripts/sync-skills-repo.test.ts, ten generated review/delivery skill files under apps/cli/skills/phases/
• backlog_item_id: #129, #130
• backlog_item_url: https://github.com/wearedevpunks/harness-intelligence/issues/129, https://github.com/wearedevpunks/harness-intelligence/issues/130
• relation_mode: body-links
• assigned_skills: [tdd, codebase-design, repo-asset-management]
• implementation_skill_guidance:
  • skill: tdd applicable_behavior: Capture the default ref/commit expectation RED before changing the pinned production selection.
  • skill: codebase-design applicable_behavior: Preserve one source-selection seam and its fail-closed commit invariant; do not add a second sync path.
  • skill: repo-asset-management applicable_behavior: Treat synchronized skill trees as generator-owned durable assets and retain immutable source identity rather than transient worktree paths.
• tdd_status: required
• tdd_target: Default skill synchronization selects the new canonical branch and accepts only its exact immutable commit.
• red_command: bun run --cwd apps/cli test src/scripts/sync-skills-repo.test.ts
• expected_red_failure: Default selection still reports the prior branch and commit.
• green_command: bun run --cwd apps/cli test src/scripts/sync-skills-repo.test.ts && diff -qr /private/tmp/hi-review-contract-skills/skills apps/cli/skills
• reason_not_testable:
• red_evidence: Public default-selection test expected the new branch/commit but received prior team/stefan/review-phase-graph-source at 423c6d59; 1 failed, 8 passed.
• green_evidence: Pin suite passed 9/9; receipt records refs/tags/sync/review-contract-handoff-consistency-684f98dff76a and exact commit 684f98dff76ac94ad3db2eb1f74230cb9910e1ad; full canonical/package diff -qr passed.
• codebase_design_notes: Keep ref qualification and commit verification inside the existing sync adapter; the generated skill tree remains output, not authority.
• review_mode: cli
• runtime_validation: not_required
• runtime_target: not_applicable
• runtime_evidence: not_applicable
• runtime_cleanup: not_applicable

T4: Refresh generated identities and durable operator documentation

• depends_on: [T2, T3]
• location: apps/cli/test-fixtures, apps/cli/src/data, docs, apps/wiki/content/docs/project/runbooks, repository changelogs
• owned_paths:
  • apps/cli/test-fixtures/public-output/managed-assets.json
  • apps/cli/test-fixtures/public-output/context-outcome.json
  • apps/cli/src/data/bundled-baseline-identity.generated.ts
  • CHANGELOG.md
  • BASELINE_CHANGELOG.md
  • docs/README.md
  • docs/runbooks/hi-cli-scaffolding.md
  • apps/wiki/content/docs/project/runbooks/hi-cli-scaffolding.md
• wave_boundary: W3
• description: Run the supported managed-asset updater after T2/T3 are stable. Update source-selection, immutable review proof, derived routing, debt resume, and manifest activation documentation. Synchronize the wiki runbook projection and record unreleased package/baseline impact without publishing.
• validation: Fixture updater and independent fixture check agree; generated identity matches packaged bytes; docs describe the implemented boundary without copying skill bodies; wiki projection is current; changelog ledgers parse.
• status: Complete
• log: 2026-08-12 Supported fixture updater and independent check passed; 80 expected hashes changed with stable inventory. Wiki projection, formatter, changelog parsing, and diff checks passed. Mixed npm/baseline impact recorded without publication.
• files edited/created: apps/cli/test-fixtures/public-output/managed-assets.json, context-outcome.json, apps/cli/src/data/bundled-baseline-identity.generated.ts, both changelogs, docs/README.md, scaffold runbook, and its wiki projection
• backlog_item_id: #129, #130
• backlog_item_url: https://github.com/wearedevpunks/harness-intelligence/issues/129, https://github.com/wearedevpunks/harness-intelligence/issues/130
• relation_mode: body-links
• assigned_skills: [writing-for-agents, repo-asset-management, simplify]
• implementation_skill_guidance:
  • skill: writing-for-agents applicable_behavior: Keep agent-facing runbook rules concrete and branch-oriented; point to installed skill authority instead of restating full workflows.
  • skill: repo-asset-management applicable_behavior: Use supported generators for durable asset identities and retain source provenance in repository artifacts.
  • skill: simplify applicable_behavior: Co-locate new operational rules with existing sync/review guidance and avoid duplicate explanations.
• tdd_status: not_applicable
• tdd_target: Generator-owned identities and documentation reflect already-tested behavior.
• red_command:
• expected_red_failure:
• green_command: bun run --cwd apps/cli fixtures:update:managed-assets && bun run --cwd apps/cli fixtures:check:managed-assets && bun run --cwd apps/wiki check:content && git diff --check
• reason_not_testable: Generated-asset, changelog, and documentation-only task; behavior RED/GREEN is owned by T1-T3.
• red_evidence: Not applicable; generator/docs task consumed the expected stale-fixture failure recorded by T3.
• green_evidence: Fixture updater 1/1 and independent check 1/1 passed; wiki content current; all eight owned paths format-clean; both Unreleased ledgers parsed with exact source SHA; diff check passed.
• codebase_design_notes: No new runtime seam; preserve generator ownership and one durable runbook source plus projection.
• review_mode: cli
• runtime_validation: not_required
• runtime_target: not_applicable
• runtime_evidence: not_applicable
• runtime_cleanup: not_applicable

T5: Freeze, review, and retain the child delivery

• depends_on: [T4]
• location: whole child branch diff and retained implementation artifacts
• owned_paths:
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/PLAN.md
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/IMPLEMENTATION-NOTES.md
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/meta.json
• wave_boundary: W4
• description: Record task evidence and deviations, run focused and outer checks, freeze the diff against the actual PR base, perform standards/spec review and structured autoreview, repair accepted findings through the owning task, then commit, push, and open the third stacked PR with base team/stefan/scaffold-skill-operator-followups.
• validation: Every task has RED/GREEN or not-applicable evidence; shared and Harness refs are retained; frozen reviews have no accepted actionable findings; child PR base/head and bottom-up stack order are verified; issues remain linked to the immutable spec and PR.
• status: Complete
• log: 2026-08-12 Validation and AC audit passed. Successive frozen reviews returned every actionable gap to its owning task. Independent Standards and Spec review are clean; structured autoreview has no accepted actionable finding. Final shared source is 684f98dff76a. PR #131 opened against team/stefan/scaffold-skill-operator-followups; stack sync --apply verified and pushed main → #123 → #127 → #131 bottom-up.
• files edited/created:
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/SPEC.md
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/PLAN.md
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/IMPLEMENTATION-NOTES.md
  • apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/meta.json
• backlog_item_id: #129, #130
• backlog_item_url: https://github.com/wearedevpunks/harness-intelligence/issues/129, https://github.com/wearedevpunks/harness-intelligence/issues/130
• relation_mode: body-links
• assigned_skills: [writing-for-agents, review, autoreview, simplify]
• implementation_skill_guidance:
  • skill: writing-for-agents applicable_behavior: Make implementation notes a concise durable handoff with concrete evidence, deviations, and next branches.
  • skill: review applicable_behavior: Freeze the actual child diff and keep standards and spec findings separate; repair only verified findings.
  • skill: autoreview applicable_behavior: Run the structured branch review against the actual PR base until no accepted actionable finding remains.
  • skill: simplify applicable_behavior: Inspect only the changed scope for avoidable duplication or state before final retention.
• tdd_status: not_applicable
• tdd_target: Durable evidence and frozen review closeout for the already-tested implementation.
• red_command:
• expected_red_failure:
• green_command: (cd /private/tmp/hi-review-contract-skills && node --test tests/review-phase-graph.contract.test.mjs && node --test tests/*.test.mjs) && bun run --cwd apps/cli test src/content/content.test.ts src/data/subagents/manifest.test.ts src/scaffold/run.test.ts src/scripts/sync-skills-repo.test.ts && bun run --cwd apps/cli check-types && diff -qr /private/tmp/hi-review-contract-skills/skills apps/cli/skills && bun run --cwd apps/cli fixtures:check:managed-assets && bun run --cwd apps/cli check && bun run --cwd apps/wiki check:content && git diff --check
• reason_not_testable: Closeout, review, retention, and documentation task; behavior tests belong to T1-T3.
• red_evidence:
• green_evidence: Canonical shared tests passed 40/40 focused and 119/119 full. Harness prompt repair tests passed 57/57 and pin tests passed 9/9; exact source parity, managed fixture check 1/1, wiki content, formatter, and diff checks passed. All AC-001 through AC-013 are met. Frozen Standards and Spec reviews are clean. Structured autoreview has no accepted actionable finding; its remaining applicability suggestion conflicts with the accepted installed-description authority and was rejected.
• codebase_design_notes: Review the existing validator, sync, and manifest seams; no new closeout abstraction.
• review_mode: cli
• runtime_validation: not_required
• runtime_target: not_applicable
• runtime_evidence: not_applicable
• runtime_cleanup: not_applicable

Testing Strategy

• Use vertical public-interface RED/GREEN slices in the canonical shared validator test.
• Test both Harness manifest ownership seams independently with the same semantic contract.
• Test the default sync pin before generated synchronization, then prove exact byte parity.
• Refresh fixture hashes only after semantic tests are green; rerun the independent check.
• Run source-wide shared contracts, focused CLI suites, CLI type/check gates, wiki content, formatting, and diff checks before frozen review.
• Do not claim release proof. Baseline/npm publication remains parked.

Validation Gates

1. W1 gate: shared and Harness behavior suites are green; shared branch/tag retained.
2. W2 gate: exact sync receipt and canonical/package byte parity are proven.
3. W3 gate: supported fixture output, generated identity, docs, wiki projection, and changelog ledgers are current.
4. W4 gate: frozen standards/spec and structured review are clean; stack topology and remote refs match local evidence.

Risks and Mitigations

• Parent branch moves: fetch and stack-sync before final push; never force-push over uninspected divergence.
• Shared source lineage drifts: base the clean worktree on the preceding immutable tag, then create a new immutable tag at the tested head.
• Caller-supplied proof remains circular: require exact bytes resolved from the named commit/path and compare them byte-for-byte inside the validator.
• Route schema churn: add one classification field and one derivation helper; do not infer from prose or duplicate aggregate decisions across callers.
• Debt capture mutates readonly review: keep capture in delivery's review handoff handler and key it by report plus stable finding ID.
• Managed root drift contaminates the patch: never run active root update or edit .agents; validate through packaged assets and isolated consumers.
• Historical handoff evidence becomes stale again: remove current-count claims or bind any retained historical claim to immutable source provenance and label it source-only.

Backlog Projection Record

• Immutable spec URL: https://github.com/wearedevpunks/harness-intelligence/blob/5a7c5facff5bb3b56b2a734fd046908d436b7fe5/apps/wiki/content/docs/project/specs/cli/review-contract-handoff-consistency/SPEC.md
• Existing implementation stories:
  • #129 — https://github.com/wearedevpunks/harness-intelligence/issues/129
  • #130 — https://github.com/wearedevpunks/harness-intelligence/issues/130
• Observed state: both open, labeled, and linked back to the immutable spec by verified comments.
• Outcome: no-op projection. Creating duplicate Linear epic/story records would not add a requirement, dependency, or provider boundary and is outside this issue-fix request.

Wait-What Language Check

Pass. The plan uses the repository's review report, finding route, delivery handoff, shared-skill source, manifest, and stack terms. Each branch states its trigger, owner, observable result, and next state without relying on hidden context.

Unresolved Questions

None.