Repository navigation
Conversation
Update the harness-copier-template pin from v0.5.0 to v0.7.0 (via `copier update`). Template changes pulled in: - v0.6.0: hooks read input from stdin JSON; process-memory layout (docs/ -> development/, specs/ -> development/work/); ADR hard-decision criteria; role playbooks, design-principles skill, glossary scaffold. - v0.7.0: bounded subagent hand-back loops; simplification wave (fewer copier questions, merged role files); hooks no longer hard-require jq; PR template. Local migration on top of the template diff: - Moved feature specs to development/work/ and local ADRs 0002-0009 to development/adr/; docs/ now holds user documentation only. - Ported customized architecture.md and the Browser automation (MCP) harness-usage section into development/. - Updated path references in src/ulv/model.py, tests/test_help.py, pr-title workflow, ui-parity-check skill, VENDORED.md, README, and .gitignore. Frozen work-unit documents keep their historical paths. - Restored the BSD-3-Clause license line in AGENTS.md/README.md (the template dropped its license question). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| `report.md`, or the body of an existing ADR is an automatic MAJOR | ||
| defect unless the plan explicitly called for it. The two registers | ||
| have their own scope contracts instead: a decision-register row must | ||
| pair with a `DECISION-PENDING:` line in this diff, and a new |
There was a problem hiding this comment.
[code-review:xhigh] Reviewer register-row rule contradicts ADR README (CONFIRMED)
'A decision-register row must pair with a DECISION-PENDING: line in this diff' contradicts development/adr/README.md, which authorizes rows with no marker (human-granted rows, and rows whose Source is a new ADR in the same PR). A compliant feature PR adding ADR + register row would be ruled an automatic MAJOR.
| commit as the code change. | ||
| - Never silently skip, disable, or `@ignore` a failing test. If a test | ||
| must be skipped, draft an ADR under `docs/adr/` and ask before | ||
| must be skipped, draft an ADR under `development/adr/` and ask before |
There was a problem hiding this comment.
[code-review:xhigh] Test-skip ADR rule contradicts new decision framework (CONFIRMED)
'If a test must be skipped, draft an ADR' (also .agents/commands/verify.md:32) conflicts with the same diff's DECISION-PENDING escalation path (developer.md:80-83) and the new three-criteria ADR bar, under which a reversible test skip clears no criterion and takes a register row, not an ADR. Either path violates one of the two merged instructions.
| @@ -1,40 +0,0 @@ | |||
| --- | |||
There was a problem hiding this comment.
[code-review:xhigh] verify skill deleted without functional replacement (CONFIRMED)
The deleted skill auto-triggered on 'verify' / 'is this ready' / 'ready to commit' to run make verify and refuse silent test skips. Under OpenCode (no session-end gate per the branch's own harness-usage.md) nothing now fires the gate for those prompts; CI becomes the only backstop.
| <topic>" marker); the human or the Developer authors the ADR file | ||
| under `development/adr/` as a separate step. | ||
| - `scratch.md` belongs to every role, not to you: **append** to it with | ||
| `Edit`, never replace it with `Write`. Overwriting it destroys spike |
There was a problem hiding this comment.
[code-review:xhigh] scratch.md append-only rule deadlocks on first hand-back (CONFIRMED)
architect.md (and developer.md:142) says touch scratch.md only via append-with-Edit and 'never replace it with Write', but spec.md:18 says whoever needs scratch.md first creates it. Edit cannot create a nonexistent file, so the first spike hand-back cannot satisfy both instructions.
| fi | ||
|
|
||
| if command -v jq >/dev/null 2>&1; then | ||
| out=$(printf '%s' "$payload" | jq -r --arg p "$1" ' |
There was a problem hiding this comment.
[code-review:xhigh] Backends disagree on concatenated JSON payloads (CONFIRMED)
jq runs the filter per document and exits 0 emitting one value per doc ('{"a":"x"} {"a":"y"}' → two lines, rc 0); the python3 fallback rejects the same payload rc 4. Same event → allow on one machine, deny on another; on the Stop hook the jq path can return 'true\nfalse' for .stop_hook_active, defeating the loop guard.
| out=$(printf '%s' "$payload" | jq -r --arg p "$1" ' | ||
| ($p | split(".") | map(select(length > 0))) as $parts | ||
| | (try getpath($parts) catch null) | ||
| | if . == null then "" elif type == "boolean" then tostring else . end |
There was a problem hiding this comment.
[code-review:xhigh] jq/python3 output differs for objects and numbers (CONFIRMED)
jq -r without -c pretty-prints objects/arrays while the python3 fallback prints compact json.dumps; JSON 1.5e3 prints '1500' (jq) vs '1500.0' (python). Violates the header's 'identically under either backend' contract. Harmless for today's string-only consumers, but any future non-string consumer behaves differently depending on whether jq is installed.
| @@ -1,43 +0,0 @@ | |||
| # Subagents | |||
There was a problem hiding this comment.
[code-review:xhigh] Subagent schema docs deleted; mode:all hazard undocumented (CONFIRMED)
This README was the only documentation of the subagent frontmatter schema, including the warning that OpenCode mode defaults to all — omitting mode: subagent exposes a role as a top-level primary agent with full permissions. No replacement exists in .agents/README.md or development/harness-usage.md.
| @@ -56,7 +60,7 @@ | |||
| "hooks": [ | |||
| { | |||
| "type": "command", | |||
There was a problem hiding this comment.
[code-review:xhigh] Stop hook runs make verify without CLAUDE_PROJECT_DIR anchor (PLAUSIBLE)
The hook anchors hook-input.sh at ${CLAUDE_PROJECT_DIR:-.} but still runs bare cwd-dependent make verify — if hook cwd is not the project root, verify fails with 'no makefile' and || exit 2 blocks the stop with an error unrelated to the code. make -C "${CLAUDE_PROJECT_DIR:-.}" verify would close it.
| PRs are **squash-merged**, so only the squash commit lands in history. | ||
| Put the Conventional Commits header in the **PR title**. Individual | ||
| branch commits during work can be freeform working notes. | ||
| Where the format applies depends on the repo's merge strategy: |
There was a problem hiding this comment.
[code-review:xhigh] Squash-merge policy lost to generic template text (CONFIRMED)
main's style.md/AGENTS.md stated the repo decision outright ('PRs are squash-merged... put the Conventional Commits header in the PR title'); with the pr_merge_strategy copier answer dropped, both now say 'check the repo's GitHub merge settings or ask a maintainer'. The decision survives only as a comment in pr-title.yml.
|
|
||
| - Don't edit anything under `*/generated/` — it's overwritten by your project's codegen pipeline. | ||
| - Don't add a runtime dependency without an ADR. | ||
| - Don't lock in a new dependency or datastore without checking whether it clears |
There was a problem hiding this comment.
[code-review:xhigh] Dependency-needs-ADR rule lost in template merge (CONFIRMED)
main's hard rule 'Don't add a runtime dependency without an ADR' became a generic ADR-bar check whose own example ('a library swap you could undo in an afternoon') flips the recorded practice — ADRs 0002-0008 all document dependency/vendoring choices under the old rule.
|
[code-review:xhigh] Findings that could not be attached inline (line outside the diff):
|
- hook-input.sh: probe jq by running it (broken jq now falls back to python3), reject concatenated JSON documents under both backends, and print compact JSON identically; document the residual numeric divergence. Mirror the functional probe in the SessionStart check. - Stop hook: anchor `make verify` at CLAUDE_PROJECT_DIR. - harness-usage.md: correct the Stop-hook claim — only exit 2 blocks; skip paths warn without blocking. - Restore the verify skill (updated for development/ layout and the DECISION-PENDING framework). - Resolve role-instruction contradictions: reviewer register-row scope contract now matches development/adr/README.md; test-skip escalation routed through DECISION-PENDING instead of a mandatory ADR; scratch.md may be created with Write when absent. - Restore lost repo policies: squash-merge → Conventional PR title (AGENTS.md, style.md); runtime dependencies require an ADR. - Document the subagent frontmatter schema and OpenCode mode:all hazard in .agents/README.md. - Fix remaining stale specs/ and docs/ paths in migrated work units. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Upstream issues filed on the template repo for the review findings that originate in v0.7.0 itself:
|
…-3 stack Evaluated the unowned competing stack (PRs #11/#13, branches ao/ulv-3/template-v0.6.0 and ao/ulv-3/template-v0.7.0) and ported its unique work: - tests/test_harness_config.py: hook-wiring pins, the hook-input.sh backend contract, block-destructive coverage, skill guard (all pass against this branch's implementations unchanged) - .agents/hooks/block-destructive.sh: quote-aware, command-position matcher with a divergence header for future copier updates - scripts/fmt-file.sh: real ruff --force-exclude implementation replacing the template no-op; opencode.jsonc runs it under bash - .claude/settings.json: PreToolUse matcher anchored to ^Bash$ - development/glossary.md: seeded domain vocabulary - development/adr/0010 + decision-register rows: relocation decision recorded (adapted to this PR's actual migration mechanics and to the retained dependency-needs-ADR rule); register marker scoped to report.md lines - src/ulv/model.py docstring no longer points into the unpublished development/ tree; README, tool-bootstrap, PR template refreshed; initial-project-description.md to development/ Rejected: their reverts of this branch's review fixes (hook-input.sh hardening, Stop-hook doc honesty, role-file contradiction fixes, verify skill) and their adoption of the softer dependency-ADR bar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Stop hook: when the stop payload is unreadable (no JSON parser, bad input), run `make verify` anyway and report a failure without blocking the stop (exit 1) - the gate always runs, and there is no stop-loop risk because nothing blocks without a readable .stop_hook_active. Only an unavailable uv still skips the gate. harness-usage.md and the hook-wiring test comment updated to match. - architect subagent: OpenCode `edit: ask` instead of `allow` - the plan phase cannot silently modify code where the platform can enforce it; Claude Code (which ignores the permission map) keeps the prose ban plus the reviewer scope check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Updates the
gh:grAItools/harness-copier-templatepin v0.5.0 → v0.7.0 viacopier update(answers file rewritten by copier itself).Template changes pulled in
docs/→development/,specs/→development/work/); three-criteria ADR bar + decision register; role playbooks,design-principlesskill, glossary scaffold.license,project_slug,mode,mcp, …), merged role files, one bounded hand-back convention; hooks no longer hard-requirejq(newhook-input.shwith python3 fallback); add-only Review checklist for the reviewer;.github/PULL_REQUEST_TEMPLATE.md.Conflict/migration resolution
No
.rejfiles or inline conflicts; the work was migrating local content into the new layout:initial-project-description.mdtodevelopment/work/, and local ADRs 0002–0009 todevelopment/adr/.docs/now holds user documentation only (docs/user/untouched).architecture.md, the "Browser automation (MCP)" section ofharness-usage.md, and the trimmedtool-bootstrap.mdintro.BSD-3-ClauseinAGENTS.md/README.md(template dropped its license question and left a fill-in marker)..mcp.json,.opencode/opencode.jsonc),Makefile,scripts/verify.sh(template unchanged upstream).Code-review follow-up (second commit)
An xhigh multi-agent review flagged 15 issues (posted as PR comments); the second commit addresses 12 of them, most notably:
hook-input.shnow probes jq by running it (a broken jq falls back to python3), rejects concatenated JSON documents under both backends, and prints compact JSON identically; the Stop hook anchorsmake verifyatCLAUDE_PROJECT_DIR; theverifyskill is restored; the reviewer/developer role contradictions around the decision register and test skips are resolved; the squash-merge and dependency-ADR repo policies the template merge had erased are restored; the subagent frontmatter schema (incl. the OpenCodemode: allhazard) is documented in.agents/README.md. Not addressed locally (upstream template design trade-offs, flagged in comments): the Stop hook's fail-open posture when no JSON parser exists, and the architect's regainedEditaccess (needed by the new scratch.md hand-back channel).Verification
make verifypasses after both commits: lint, fmt-check, 337 tests, docs-check, docs-build.🤖 Generated with Claude Code
ulv-3 stack triage (third commit)
The terminated ulv-3 session left a competing CI-green stack (#11 + #13, branches
ao/ulv-3/template-v0.6.0/ao/ulv-3/template-v0.7.0— left untouched for the humans to close). Its unique work was evaluated piece by piece:Ported —
tests/test_harness_config.pyextended with hook-wiring pins, thehook-input.shbackend contract,TestBlockDestructive, and a skill guard (all pass against this branch's implementations unchanged); the quote-aware, command-positionblock-destructive.shrewrite (upstream's substring grep denies commands that merely quote a pattern); the realfmt-file.sh(ruff--force-exclude, replacing the template's no-op) run underbashin opencode.jsonc (it silently dies undersh); the^Bash$PreToolUse matcher anchor (bareBashalso matchesBashOutput); the seededdevelopment/glossary.md; ADR 0010 + decision-register rows (adapted — see below); the register's marker-scoped-to-report.md clarification;model.py's docstring no longer pointing into the unpublisheddevelopment/tree; README/tool-bootstrap/PR-template refreshes;initial-project-description.mdatdevelopment/root.Adapted rather than copied — their ADR 0010 narrates a relocate-by-hand-then-update migration and embraces the template's softer dependency-ADR bar; this PR's version records the same durable decisions but describes the migration that actually happened here, and records the retained hard "runtime dependency ⇒ ADR" rule as an explicit register row (2026-08-template-v0.7.0.4) so the divergence is a reviewable decision, not a silent conflict. Their register row citing
TestBlockDestructiveis made true here by porting the tests alongside the script.Rejected — their reverts of this branch's review fixes (the hardened
hook-input.sh, the honest Stop-hook doc wording, the role-file contradiction fixes, the restoredverifyskill); their adoption of the softer dependency-ADR bar in AGENTS.md; their restoration of the historical/docs/wording in the frozen user-documentation spec.