fix(ce-code-review): replace resolve-base.sh with prose-driven base detection

[tmchow]

Summary

Fixes #811 by removing the failing helper rather than fixing how it's called. Supersedes #812.

ce-code-review branch mode and standalone mode invoked bash scripts/resolve-base.sh via a bare relative path. At runtime the Bash tool's CWD is the user's project, not the skill directory, so the helper has been silently failing on every invocation — the agent improvised its way through base detection on its own judgment, which is why nobody noticed.

What changed

• Delete scripts/resolve-base.sh (and its 300-line test file). The helper's most distinctive feature (fork-safe PR-base resolution) wasn't realized at the call sites that actually used it — PR mode has its own inline fork-safe block, and branch/standalone modes never had a real PR-URL to resolve against.
• Replace both call sites with prose that names the priority chain explicitly: PR metadata for the current branch (reusing PR mode's PR_BASE_REMOTE block for fork safety) → origin/HEAD → gh repo view → common branch names. Compute the merge-base, unshallow on shallow clones, stop hard if nothing resolves.
• Preserve the load-bearing safety guard. Both modes still explicitly say "do not fall back to git diff HEAD" — a review without a base would only show uncommitted changes and silently miss all committed work. The contract test in `tests/review-skill-contract.test.ts` now pins this guard phrase per-mode.
• Drop the ${CLAUDE_SKILL_DIR} workaround and allowed-tools grant that fix(ce-code-review): resolve base helper from skill dir #812 was going to add — both become unnecessary when there's no bundled script to invoke.

Net: -429 lines (script + script tests), +13 lines (prose + new guard test).

Why this shape instead of #812

#812 was a correct minimal fix that kept the helper and corrected the call. Discussion in #812 and our review session converged on a different question: does the helper earn its complexity?

Empirically no — users have been hitting "couldn't find resolve-base.sh" errors, the spec said to stop, but the agent improvised and the review still completed with reasonable scope. That's a signal that the deterministic helper wasn't load-bearing in branch/standalone modes. PR mode (which DOES need fork-safe resolution) has its own inline block and is untouched.

The change trades:

• One bash tool call (helper) → two or three (PR check + remote fetch + merge-base) per invocation
• Deterministic script behavior (untested for fork edge cases anyway) → prose-driven with explicit priority chain
• A 4-line allowed-tools + \${CLAUDE_SKILL_DIR} workaround → nothing

Test plan

• `bun test` (1340 pass, 0 fail — net -9 tests = the deleted resolve-base-script.test.ts)
• `bun run release:validate` (in sync, 49 agents / 37 skills)
• Contract test `fails closed when merge-base is unresolved` still pins the no-HEAD-fallback guard via the new "Do not fall back to `git diff HEAD`" regex (matches once per mode = 2 occurrences)
• Manual: run `/ce-code-review` standalone on a clean feature branch in this repo and verify base detection lands on `origin/main`
• Manual: run `/ce-code-review some-branch` with no PR open and verify origin/HEAD path
• Manual: run `/ce-code-review ` and verify PR mode still works unchanged

Credit

Thanks to @frankstallone for the original report (#811) and PR (#812). The closing comment on #812 explains the structural choice.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3027c58447

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".