chore(harness): tools/ + /make-tool + doc registry + TESTING.md (#461 A1+A3+A4) - #514
Conversation
…461 A1+A3+A4) A1: stand up a tracked tools/ directory with README + CONVENTIONS, an agent-review .sh/.ps1 wrapper over second-opinion.mjs, the two reusable merge-sweep scripts promoted out of the gitignored night-run dir, a /make-tool skill, and a CLAUDE.md pointer. A3: a greppable "At a glance" header on the 7 system docs plus a Docs registry in CLAUDE.md. A4: a root TESTING.md suite catalog, linked from CLAUDE.md, /audit-tests, /pr-review. Docs, one skill, and thin shell wrappers only; no product code, so no parity or i18n. The merge-sweep scripts were copied verbatim because the source night-run dir is gitignored and untracked (git mv was not possible); their pre-existing comment em dashes are left as-is (retrofit is a non-goal). A tools/*.sh eol=lf gitattributes rule keeps the bash scripts LF on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Code Review: PR #514
Scope: PR #514 — chore(harness): tools/ dir + /make-tool + doc registry + TESTING.md (#461 A1+A3+A4)
Recommendation: APPROVE
Summary
Pure harness change: a new tools/ dir (2 promoted merge-sweep scripts + a new agent-review wrapper + its contract doc), a new TESTING.md, and > **At a glance** headers + a CLAUDE.md docs registry across 7 existing docs. No product code, so parity/i18n/contract/security/design gates correctly all N/A. Two Medium findings, nothing Critical/High.
Findings
Medium
1. Docs registry lists WORKFLOW.md but it has no "At a glance" header
- location:
orbit-ui-mobile/CLAUDE.md(new "## Docs registry" section) - issue: The registry's own instruction is "Grep a doc's
At a glanceheader before loading the whole file," butWORKFLOW.md(untouched by this PR) has none — verified via grep. The other 6 registry rows all do. - risk: An agent following the registry's own instruction greps
WORKFLOW.md, finds nothing, and the grep-first savings the registry exists to deliver breaks on its first table. - fix: Add a
> **At a glance**header toWORKFLOW.mdin this PR, or note in the row that it's pending. - reference: CLAUDE.md "Docs registry" section (added by this diff).
2. merge-sweep scripts promoted into a catalogued tool without closing the reviewDecision race the same diff documents as still open
- location:
tools/merge-sweep-cov.sh:29-55,tools/merge-sweep.sh:9-25 - issue: This PR's own edit to
.claude/pending-lessons.mddocuments a real past incident (orbit-api #403) where a sweep script merged past a re-triggered CHANGES_REQUESTED review, and still lists the fix as "Proposed home" / not yet promoted to code. The promoted scripts re-pollreviewDecisionevery 20s afterupdate-branch(an improvement over a single stale read), but neither explicitly waits for the async re-review check/workflow to reach a terminal state before trustingmergeStateStatus=CLEAN. - risk:
mainis protected and both scripts callgh pr merge --admin/--squashunattended — a recurrence merges a PR whose fresh review flagged something Critical, same as the documented incident, straight tomainwith no human in the loop. - fix: Either close the gap (poll the named review check-run to a terminal conclusion, not just
reviewDecision), or land with an explicit caveat intools/README.md/tools/CONVENTIONS.mdflagging the residual race. - reference:
.claude/pending-lessons.md"2026-07-14 — sweep-merge can race..." (edited by this same diff); CLAUDE.md rule 1.
Subagents
All 5 (parity-checker, i18n-syncer, contract-aligner, security-reviewer, design-reviewer) — N/A, none of their gating surfaces are touched by this diff.
Validation
N/A for lint/type-check/tests/build — no TS/C#/test-bearing source changed (Build/Unit Tests/SonarCloud run as separate required CI checks for this PR). Spot-verified instead: every TESTING.md command resolves to a real package.json script; every CI workflow it names exists under .github/workflows/; FEATURES.md's numeric at-a-glance claims match its own body; no stale .claude/night-run/merge-sweep* references remain outside one intentionally-historical mention.
Deferred
- Dimension 4 (comment policy): no TS/C# changed, so the mechanized gate never fires; new
.shfiles carry narration comments outside any lint gate's scope — not raised as a rubric-4 finding. - Dimensions 6, 7, 8, 9, 10, 11, 13, 14: N/A, confirmed by the 20-file changed list (all
.claude/,tools/, or root docs). - Prose accuracy of the four
.claude/research/*.mdat-a-glance summaries was spot-checked, not exhaustively verified against their full bodies. - Cross-repo dimensions (contract-aligner, orbit-api side of backward-compat guard): not verifiable in this CI job — orbit-api is not checked out.
What's good
- Re-poll-every-20s design in the merge-sweep scripts is a real improvement over a stale pre-update snapshot.
TESTING.mdis fully accurate against real scripts/CI files.- PR body honestly scopes out parity/i18n rather than force-fitting one.
tools/CONVENTIONS.mdandmake-tool/SKILL.mdstay consistent with each other.
Recommendation
Approve as-is; land the two Medium follow-ups (one doc header, one script hardening/caveat) same-day or as a tracked follow-up — neither blocks this PR.
|



Part of #461, bundle 1 (A1 + A3 + A4). Closes three cheap, high-value harness gaps entirely inside
orbit-ui-mobile. Docs, one skill, and thin shell wrappers only.What landed
A1 - a real
tools/directory +/make-tooltools/withREADME.md(catalog + what belongs here) andCONVENTIONS.md(the agent-callable-script contract: single purpose,--help, meaningful exit codes, non-interactive, cwd-safe, stdin for big payloads, no secrets in argv, POSIX.shbaseline +.ps1only when it must run in the user's shell).tools/agent-review.sh+tools/agent-review.ps1: thin wrappers over.claude/skills/second-opinion/second-opinion.mjs(GLM-5.2 via opencode). They resolve the repo root from the script location, support--help/-h, forward stdin plus--claim/--model/--timeout, and pass the helper's exit code through.tools/merge-sweep.sh+tools/merge-sweep-cov.sh: the two reusable merge-sweep scripts promoted out of the gitignored.claude/night-run/into trackedtools/..claude/skills/make-tool/SKILL.md: the "run the same incantation twice, build a tool" skill, plus aCLAUDE.mdpointer.A3 - greppable doc summaries + a registry
> **At a glance**header on the 7 system docs:DESIGN.md,FEATURES.md,research.md, and the 4.claude/research/*.mddossiers.## Docs registrysection inCLAUDE.md(table of docs + one-line purpose) with a self-healing instruction to update the header and the registry together.A4 -
TESTING.mdTESTING.md: how to write a test here, the anti-patterns/audit-testsencodes, the six-suite catalog with exact commands and what each proves, the CI mapping, and an orbit-api pointer. Linked fromCLAUDE.md,/audit-tests, and/pr-review.Scope notes
git mv-d: the source.claude/night-run/dir is gitignored and untracked, so there was no tracked history to move. End state is identical (the scripts are now tracked undertools/). Their pre-existing comment em dashes are left as-is; em-dash retrofit of existing content is a deliberate non-goal, and all newly authored copy is em-dash-free.tools/*.sh text eol=lfto.gitattributesso the bash scripts stay LF on Windows checkout (same guard the repo already applies to.mjs); shell scripts committed as100755.TESTING.mdstates honestly that the Playwright "e2e" suite and the post-deploy "smoke" suite are one and the same; there is no separate PR-time Playwright suite.Validation
agent-review --helpprints usage and exits 0 in both shells (bash tools/agent-review.sh --help,pwsh -File tools/agent-review.ps1 --help); unknown flag and truly-empty input exit 1.node second-opinion.mjs-> single-line JSON with astatusfield -> exit code passthrough) without spending a model call, since the helper short-circuits whitespace-only input before invoking opencode.At a glance;CLAUDE.mdhas the registry, thetools/pointer, and theTESTING.mdlink;/audit-testsand/pr-reviewlinkTESTING.md; no remainingnight-run/merge-sweepreference anywhere; newly authored copy is em-dash-free.🤖 Generated with Claude Code