feat(harness): opencode enforcement parity — port CC hooks via a shared logic core (7h) - #441
Conversation
…ed logic core (7h) Stage 7h of the harness-upgrade plan, paired with agentic-dev-workflow PR #2 (the generic dual-target hook engine). Gives Orbit opencode enforcement parity on this machine using the same shared-logic-core + adapter pattern the pack standardizes: every rule lives once, both tools enforce it, no twin drift. Shared logic core — .claude/hooks/_lib/ (pure, both tools call it): - rules-git.mjs checkGitCommand (protected-ref push, bypass flags, commit -n) + checkNpmExpoPin (Expo SDK 57 pin guard) - rules-content.mjs checkEmDashes + checkBrandColors (scan only the added text) - rules-source.mjs checkTsAntipatterns, checkNewTodos, checkCsharpAuthz, checkCsharpTimezone, checkCsharpFluentConfig (whole-file) - rules-parity.mjs classifyScope + parityMessages (cross-platform nudge) - io.mjs payload normalizers for both tools Claude Code side: all 10 hooks refactored into thin adapters over _lib (byte-faithful — the exact regexes and stderr messages are preserved; settings.json wiring unchanged since the filenames are unchanged). opencode side: .opencode/plugin/orbit-guardrails.js — one plugin wiring the SAME _lib. tool.execute.before runs the git/Expo-pin + added-text content rules (block by throw); tool.execute.after runs the whole-file rules + parity nudge (a throw surfaces the violation the way the CC PostToolUse exit-2 does); event/session.idle emits a best-effort proactivity nudge. opencode auto-loads it from .opencode/plugin/. Mapping note: opencode has no PostToolUse, so whole-file rules run in tool.execute.after (file written) and added-text rules in tool.execute.before (can block); the proactivity Stop-gate degrades to an idle nudge because opencode cannot rewind a finished turn. Proof (.claude/hooks/test-hooks.mjs, 42 assertions): _lib unit checks + the REAL Claude Code hooks (regression guard, confirming the refactor preserved behavior) + the REAL opencode plugin — the same rule yields the same verdict in both tools. Cross-links agentic-dev-workflow PR #2. 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
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #441
Scope: PR #441 in thomasluizon/orbit-ui-mobile
Recommendation: APPROVE
Summary
The diff extracts Orbit's 10 Claude Code hooks into a shared, pure _lib/ core, turns the hooks into thin adapters over it, and ports the same rules to opencode via a new .opencode/plugin/orbit-guardrails.js plugin plus a hand-rolled proof script (test-hooks.mjs). It touches only .claude/hooks/** and .opencode/plugin/** — no apps/*, orbit-api, or packages/shared — so this is pure dev-tooling, not a user-facing change. The extraction itself is byte-faithful: every regex and stderr message diffed against the pre-PR originals matches exactly, with no unused imports or dead code introduced. Two Medium-severity gaps are worth tracking; neither blocks merge.
Findings
Critical / High
None.
Medium
[MEDIUM] The parity proof (test-hooks.mjs) is never run automatically
.claude/hooks/test-hooks.mjs:1-137- It's a hand-rolled assertion runner (not Vitest), living outside the npm
workspacesglob (apps/*,packages/*).npx turbo run test(CI'sunit-testsjob) never touches it — no husky/pre-commit/CI wiring exists anywhere in the repo for this file. - Risk: the PR's "same rule, same verdict, both tools" proof is real only when someone remembers to run it by hand. A future
_lib/adapter/plugin edit can silently desync Claude Code and opencode enforcement with nothing in CI catching it — the exact "twin drift" this PR exists to prevent. - Fix: add
"test:hooks": "node .claude/hooks/test-hooks.mjs"and wire it into theunit-testsCI job (or a dedicated step gated on.claude/hooks/**/.opencode/plugin/**). Ref: CLAUDE.md "Testing: Vitest unit tests only; every feature needs behavior tests."
[MEDIUM] The test never imports the shipped .opencode/plugin/orbit-guardrails.js as-is
test-hooks.mjs:1345-1353doescpSync(pluginSrc, pluginMjs)— copying the plugin to a*.probe.mjsfile beforeimport()-ing it. That rename is necessary because plain Node parses a.jsas CommonJS unless the nearestpackage.jsondeclares"type": "module"(root doesn't; none exists under.opencode/), and the shipped file uses ESM syntax that would throw aSyntaxErrorif Node loaded the.jsdirectly. The test's own workaround is the tell.- Risk: if opencode's plugin loader ever resolves modules the way plain Node does, the shipped plugin fails to load entirely and opencode runs with zero Orbit guardrails, silently — untested by this suite since it always tests the renamed copy. Not verifiable in this CI job whether opencode's actual loader is Bun-based (which would sidestep this) — could not execute a live import test.
- Fix (cheap, removes the ambiguity regardless of runtime): ship as
.opencode/plugin/orbit-guardrails.mjs, or add a one-line.opencode/plugin/package.jsonwith{"type":"module"}, and point the test at the real path with no copy step. Ref: CLAUDE.md rule 1 (root cause over workarounds).
Low / Info
None (signal gate).
Subagents
All five gated subagents (parity-checker, i18n-syncer, contract-aligner, security-reviewer, design-reviewer) are N/A — the diff never touches apps/web, apps/mobile, packages/shared, or orbit-api.
Validation
Skipped per CI adaptation (Build / Unit Tests / SonarCloud run as separate required checks). Changed files also sit outside every linted/typed/tested npm workspace regardless.
Deferred
Dimensions covering DESIGN.md, cross-platform parity, i18n, contract drift, orbit-api security/backend rules, FEATURES.md — all N/A, diff never touches their surfaces. The orbit-api side of the backward-compat guard is not verifiable in this job (sibling repo not checked out) but is not implicated since this PR touches no contract/DTO surface.
What's good
- The
_libextraction is genuinely byte-faithful — regexes and messages match pre-PR originals exactly. - No unused imports/dead code in any of the 10 slimmed adapters.
test-hooks.mjs's self-avoidance tricks ("TO" + "DO","--no-" + "verify") so the fixture doesn't trip the hooks it's testing.- Sensible, well-reasoned mapping of Claude Code's PreToolUse/PostToolUse split onto opencode's
tool.execute.before/after.
Recommendation
Approve as-is. Wiring test-hooks.mjs into CI and removing the .js/.mjs module-format ambiguity in the opencode plugin are cheap, contained fast follows — neither is a functional regression today.
1 participant
Stage 7h — opencode enforcement parity (paired Orbit deliverable)
Paired with agentic-dev-workflow PR #2 (the generic dual-target hook engine). Gives Orbit opencode enforcement parity on this machine using the same shared-logic-core + adapter pattern the pack standardizes: each rule lives once, both tools enforce it, no twin drift.
Shared logic core —
.claude/hooks/_lib/(pure; both tools call it)rules-git.mjscommit -n) + Expo-SDK-57 pin guardrules-content.mjsrules-source.mjsrules-parity.mjsio.mjsClaude Code side
All 10 hooks refactored into thin adapters over
_lib— byte-faithful: the exact regexes and stderr messages are preserved, andsettings.jsonwiring is unchanged (filenames unchanged). The parity test runs the real hook files to confirm no behavior regressed.opencode side —
.opencode/plugin/orbit-guardrails.jsOne plugin wiring the same
_lib:tool.execute.before→ git + Expo-pin + em-dash/brand-color on the added text (block bythrow).tool.execute.after→ whole-file rules + parity nudge (a throw surfaces the violation the way the CC PostToolUse exit-2 does).event/session.idle→ best-effort proactivity nudge.opencode auto-loads it from
.opencode/plugin/(no wiring). Mapping note: opencode has no PostToolUse, so whole-file rules run intool.execute.after(file written) and added-text rules intool.execute.before(can block); the proactivity Stop-gate degrades to an idle nudge because opencode cannot rewind a finished turn.Proof —
node .claude/hooks/test-hooks.mjs(42 assertions)_libunit checks + the real Claude Code hooks (regression guard) + the real opencode plugin — the same rule yields the same verdict in both tools.Cross-links agentic-dev-workflow#2.
🤖 Generated with Claude Code