feat(ceo): code-check gate -- vet proposals against the real code before filing #287

Merged

joshtronic merged 1 commit from feat/ceo-codecheck into master

2026-06-30 02:25:01 +00:00

igor commented

2026-06-30 01:44:44 +00:00

Collaborator

The CEO drafts proposals blind to the codebase — it sees the mandate, metrics, and tracker activity, never the actual code — so it proposed already-done work (porksicle #101: per-game SEO meta that was already unique). This gate is the sharp dev going "you know we already do that, right?"

ceo_codecheck_proposal, run before ceo_file_proposal:

derive 4–6 distinctive keywords (tool-free claude_call)
git grep origin/master for them (reads the ref, not a stale checkout; 8KB cap)
verdict (tool-free claude_call): KEEP / DROP

Fail-open: DROP only on a confident "already implemented"; KEEP on any error or ambiguity (no clone, call fails, no terms, unparseable). A broken gate never wrongly kills a real proposal — the 5 new tests cover exactly these paths.

No-exfil by construction: both calls are tool-free claude_call; the drop reason is logged locally only, never posted/emailed/returned. The code goes in, one bit (file-or-skip) comes out. That closes the #241 "read-only ≠ safe" concern structurally rather than by trust.

Gates the digest + answer-path proposals (where the #101 class lives). The REVISE re-file in the reconsider path is left ungated and flagged for follow-up (rare + feedback-informed → low risk).

Verified: bash -n clean, test-ceo all pass, check-sync green. Reviewed by hand independently of the build. No reviewer.

The CEO drafts proposals **blind to the codebase** — it sees the mandate, metrics, and tracker activity, never the actual code — so it proposed already-done work (porksicle #101: per-game SEO meta that was already unique). This gate is the sharp dev going *"you know we already do that, right?"* **`ceo_codecheck_proposal`**, run before `ceo_file_proposal`: 1. derive 4–6 distinctive keywords (tool-free `claude_call`) 2. `git grep origin/master` for them (reads the ref, not a stale checkout; 8KB cap) 3. verdict (tool-free `claude_call`): KEEP / DROP **Fail-open**: DROP only on a confident "already implemented"; KEEP on *any* error or ambiguity (no clone, call fails, no terms, unparseable). A broken gate never wrongly kills a real proposal — the 5 new tests cover exactly these paths. **No-exfil by construction**: both calls are tool-free `claude_call`; the drop reason is logged **locally only**, never posted/emailed/returned. The code goes in, one bit (file-or-skip) comes out. That closes the #241 "read-only ≠ safe" concern structurally rather than by trust. Gates the digest + answer-path proposals (where the #101 class lives). The REVISE re-file in the reconsider path is left ungated and flagged for follow-up (rare + feedback-informed → low risk). Verified: `bash -n` clean, `test-ceo` all pass, check-sync green. Reviewed by hand independently of the build. No reviewer.

igor added 1 commit

2026-06-30 01:44:44 +00:00

feat(ceo): code-check gate -- vet proposals against the real code before filing

Lint / check-sync (pull_request) Successful in 5s

Details

Lint / check-sync (push) Successful in 4s

Details

8b5eac6bb3

The CEO drafts proposals BLIND to the codebase (mandate + metrics + tracker
activity, never the code), so it proposed already-done work (porksicle #101:
per-game SEO meta that was already unique -> withdrawn). The gate is the sharp dev
going 'you know we already do that, right?'

ceo_codecheck_proposal <repo> <title> <body>, run before ceo_file_proposal in
_ceo_file_outputs:
  1. derive 4-6 distinctive keywords (tool-free claude_call)
  2. git grep origin/master for them (reads the ref, not a stale checkout; 8KB cap)
  3. verdict (tool-free claude_call): KEEP|DROP
DROP only on a confident 'already implemented'; FAIL-OPEN to KEEP on any error or
ambiguity (no clone, call fails, no terms, unparseable) -- a broken gate never
wrongly kills a real proposal.

Non-agentic by design (both calls are tool-free claude_call) so there is no exfil
channel -- the #241 'read-only != safe' concern is closed by construction. The drop
reason is logged LOCALLY only, never posted/emailed/returned; the gate's sole
outward effect is the file-or-skip bit.

Gates the digest + answer-path proposals (the #101 class). The REVISE re-file in the
reconsider path is a noted follow-up (rare + feedback-informed, low risk).

bash -n clean; bin/test-ceo.sh all pass (5 new fail-open checks); check-sync green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011KXPafSYJPY6XkWHHk1Lfs

igor commented

2026-06-30 01:46:28 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 8b5eac6b: success

Adds a fail-open "code-check" gate (ceo_codecheck_proposal) that derives keywords via a tool-free claude_call, greps origin/master, and asks the model KEEP/DROP before filing a CEO proposal; wired into _ceo_file_outputs and covered by 5 tests. In scope (~85 lines), CI green, no CI-config touches, and the security story (tool-free calls, reason logged locally, only a one-bit decision leaves the function) holds up. My reservations are functional/unverifiable, not safety — every error path correctly fails open to KEEP, so a real proposal is never silently eaten.

Reservations I'd want a human to weigh:

log output stream is unverifiable from the diff (lib/ceo.sh, DROP branch). The function's stdout contract is exactly KEEP/DROP — tick.sh does [ "$(ceo_codecheck_proposal ...)" = "DROP" ]. The DROP branch calls log "..." before echo DROP. If log writes to stdout (rather than stderr/a file), the captured value becomes "<log line>\nDROP", the = "DROP" test fails, and the gate silently never drops anything in production. The test at test-ceo.sh stubs log() { :; }, so this exact production interaction is never exercised. Please confirm log writes to stderr/file (it almost certainly does, but I can't see its definition). If there's any doubt, route the log call's output to stderr explicitly or move it after the decision is captured.
Hardcoded origin/master (lib/ceo.sh, git grep ... origin/master). Repos whose default branch is main will have git grep origin/master error → 2>/dev/null → empty grepout → KEEP. Fail-open is fine, but the gate becomes a silent no-op for any non-master repo. Worth deriving the default ref (e.g. git symbolic-ref refs/remotes/origin/HEAD) or at least documenting the master assumption.

Smaller notes (non-blocking):

Model-derived terms are fed to git grep -e "$t" — safe from option/command injection because they're passed as -e pattern args inside a quoted array and pre-filtered to [[:alnum:]_./-]. Good. The . in keywords does act as a regex wildcard (the prompt asks for "no regex" but doesn't enforce it); harmless, just slightly noisier matches.
Per-proposal git fetch -q origin means N fetches for N proposals — minor, acceptable.
Test coverage is solid on the gate's own fail-open branches (no clone, terms-call failure, unparseable verdict, KEEP, DROP), but nothing exercises the tick.sh continue integration or the real log path — which is exactly where the stdout-purity concern above lives. Consider one test that drives the gate with the real log to lock the stdout contract.

The REVISE re-file path being left ungated is acknowledged in the description and is reasonable to defer. Nothing here rises to a blocking defect, but the log stdout question is a genuine gap between the tests and production behavior that I can't resolve from the diff alone — hence COMMENT rather than APPROVE.

_{Independent review by the harness on claude-opus-4-8. The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `COMMENT` _(automated)_ CI for `8b5eac6b`: **success** Adds a fail-open "code-check" gate (`ceo_codecheck_proposal`) that derives keywords via a tool-free `claude_call`, greps `origin/master`, and asks the model KEEP/DROP before filing a CEO proposal; wired into `_ceo_file_outputs` and covered by 5 tests. In scope (~85 lines), CI green, no CI-config touches, and the security story (tool-free calls, reason logged locally, only a one-bit decision leaves the function) holds up. My reservations are functional/unverifiable, not safety — every error path correctly fails open to KEEP, so a real proposal is never silently eaten. **Reservations I'd want a human to weigh:** - **`log` output stream is unverifiable from the diff (lib/ceo.sh, DROP branch).** The function's stdout contract is exactly `KEEP`/`DROP` — tick.sh does `[ "$(ceo_codecheck_proposal ...)" = "DROP" ]`. The DROP branch calls `log "..."` *before* `echo DROP`. If `log` writes to **stdout** (rather than stderr/a file), the captured value becomes `"<log line>\nDROP"`, the `= "DROP"` test fails, and the gate silently never drops anything in production. The test at `test-ceo.sh` stubs `log() { :; }`, so this exact production interaction is never exercised. Please confirm `log` writes to stderr/file (it almost certainly does, but I can't see its definition). If there's any doubt, route the log call's output to stderr explicitly or move it after the decision is captured. - **Hardcoded `origin/master` (lib/ceo.sh, `git grep ... origin/master`).** Repos whose default branch is `main` will have `git grep origin/master` error → `2>/dev/null` → empty grepout → KEEP. Fail-open is fine, but the gate becomes a silent no-op for any non-`master` repo. Worth deriving the default ref (e.g. `git symbolic-ref refs/remotes/origin/HEAD`) or at least documenting the `master` assumption. **Smaller notes (non-blocking):** - Model-derived terms are fed to `git grep -e "$t"` — safe from option/command injection because they're passed as `-e` pattern args inside a quoted array and pre-filtered to `[[:alnum:]_./-]`. Good. The `.` in keywords does act as a regex wildcard (the prompt asks for "no regex" but doesn't enforce it); harmless, just slightly noisier matches. - Per-proposal `git fetch -q origin` means N fetches for N proposals — minor, acceptable. - Test coverage is solid on the gate's own fail-open branches (no clone, terms-call failure, unparseable verdict, KEEP, DROP), but nothing exercises the tick.sh `continue` integration or the real `log` path — which is exactly where the stdout-purity concern above lives. Consider one test that drives the gate with the real `log` to lock the stdout contract. The REVISE re-file path being left ungated is acknowledged in the description and is reasonable to defer. Nothing here rises to a blocking defect, but the `log` stdout question is a genuine gap between the tests and production behavior that I can't resolve from the diff alone — hence COMMENT rather than APPROVE. --- <sub>Independent review by the harness on `claude-opus-4-8`. The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-06-30 01:46:29 +00:00

1 minute 31 seconds

igor requested review from joshtronic

2026-06-30 01:46:29 +00:00