fix: gate the CEO REVISE re-file with `ceo_codecheck_proposal` (follow-up to #287) #292

Merged

joshtronic merged 1 commit from agent/290-gate-the-ceo-reconsider-revise-re-file-with-the into master

2026-06-30 03:29:47 +00:00

igor commented

2026-06-30 03:27:25 +00:00

Collaborator

What this PR does

fix: gate the CEO REVISE re-file with ceo_codecheck_proposal (follow-up to #287)
Add ceo_revise_refile to lib/ceo.sh -- thin helper wrapping the gate; KEEP files the new proposal, DROP posts a one-line reply on the old (already-closed) issue and skips re-file
Update the REVISE branch in do_ceo_tick (bin/tick.sh) to call ceo_revise_refile instead of ceo_file_proposal directly
Mirrors the existing gate exactly: fail-open to KEEP on any error; drop reason logged locally only, never posted

Test plan

bash bin/test-ceo.sh passes -- new == ceo_revise_refile (gated REVISE) == section covers KEEP (re-files) and DROP (skips, posts comment) cases
bash bin/check-sync.sh passes (all existing + new unit tests green)
Security review: no findings (comment body is a hardcoded static string; no new injection surface)

Closes #290

## What this PR does - [x] fix: gate the CEO REVISE re-file with `ceo_codecheck_proposal` (follow-up to #287) - [x] Add `ceo_revise_refile` to `lib/ceo.sh` -- thin helper wrapping the gate; KEEP files the new proposal, DROP posts a one-line reply on the old (already-closed) issue and skips re-file - [x] Update the REVISE branch in `do_ceo_tick` (`bin/tick.sh`) to call `ceo_revise_refile` instead of `ceo_file_proposal` directly - [x] Mirrors the existing gate exactly: fail-open to KEEP on any error; drop reason logged locally only, never posted ## Test plan - [x] `bash bin/test-ceo.sh` passes -- new `== ceo_revise_refile (gated REVISE) ==` section covers KEEP (re-files) and DROP (skips, posts comment) cases - [x] `bash bin/check-sync.sh` passes (all existing + new unit tests green) - [x] Security review: no findings (comment body is a hardcoded static string; no new injection surface) Closes #290

igor added 1 commit

2026-06-30 03:27:25 +00:00

fix: gate the CEO REVISE re-file with ceo_codecheck_proposal (follow-up to #287 )

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

Details

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

Details

9f00bcf6dd

joshtronic approved these changes

2026-06-30 03:29:38 +00:00

joshtronic merged commit 6463025fac into master

2026-06-30 03:29:47 +00:00

joshtronic deleted branch agent/290-gate-the-ceo-reconsider-revise-re-file-with-the

2026-06-30 03:29:47 +00:00

joshtronic referenced this pull request from a commit

2026-06-30 03:29:48 +00:00

Merge pull request 'fix: gate the CEO REVISE re-file with `ceo_codecheck_proposal` (follow-up to #287)' (#292) from agent/290-gate-the-ceo-reconsider-revise-re-file-with-the into master

igor commented

2026-06-30 03:30:18 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for 9f00bcf6: success

Clean, tightly-scoped follow-up that routes the CEO REVISE re-file through the existing ceo_codecheck_proposal gate. Correct, honestly described, tested both ways, CI green. Approving.

Verified against the contract:

Scope — three files, ~40 lines, all on the single concern (gating the REVISE re-file). No drive-bys.
Checklist honesty — each box maps to a real hunk: ceo_revise_refile added in lib/ceo.sh; do_ceo_tick REVISE branch rewired in bin/tick.sh; new == ceo_revise_refile (gated REVISE) == test section present. Fail-open claim holds — the function delegates KEEP/DROP to ceo_codecheck_proposal, which is the component documented to echo KEEP on error, so a gate failure re-files rather than silently eating the proposal.
Equivalence of the rewire — old call extracted title/body via jq -r in tick.sh and passed $FORGEJO_REVIEWER; the new helper extracts the same fields from the passed $rissue JSON and forwards the assignee through. Behaviorally identical on the KEEP path.
Security — DROP comment body is a hardcoded jq -n '{body:"..."}' static string; repo/rnum only flow into the API path and are internal CEO state, not a new injection surface. No new findings, consistent with the description.
Tests — KEEP asserts the proposal is filed and no comment posted; DROP asserts nonzero return, nothing filed, and the comment is posted. The _fj mock case-orders /comments* before /issues*, so the comment POST doesn't get mis-bucketed as an issue creation. Good coverage of both branches.
CI success; no .forgejo/ or .github/workflows/ changes.

Non-blocking observation (follow-up at most):

In tick.sh, the else branch logs "code-check dropped revised proposal" whenever ceo_revise_refile returns nonzero. But the helper also returns the exit status of ceo_file_proposal on the KEEP path, so a genuine API/file failure (not a gate DROP) would be logged as a code-check drop. The prior code logged nothing on file failure. The outcome (no re-file) is unchanged; only the log line is slightly misleading for the rare file-failure case. Worth a tweak if log fidelity matters, but not a merge blocker.

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

### 🤖 Review — `APPROVE` _(automated)_ CI for `9f00bcf6`: **success** Clean, tightly-scoped follow-up that routes the CEO REVISE re-file through the existing `ceo_codecheck_proposal` gate. Correct, honestly described, tested both ways, CI green. Approving. **Verified against the contract:** - **Scope** — three files, ~40 lines, all on the single concern (gating the REVISE re-file). No drive-bys. - **Checklist honesty** — each box maps to a real hunk: `ceo_revise_refile` added in `lib/ceo.sh`; `do_ceo_tick` REVISE branch rewired in `bin/tick.sh`; new `== ceo_revise_refile (gated REVISE) ==` test section present. Fail-open claim holds — the function delegates KEEP/DROP to `ceo_codecheck_proposal`, which is the component documented to echo KEEP on error, so a gate failure re-files rather than silently eating the proposal. - **Equivalence of the rewire** — old call extracted title/body via `jq -r` in `tick.sh` and passed `$FORGEJO_REVIEWER`; the new helper extracts the same fields from the passed `$rissue` JSON and forwards the assignee through. Behaviorally identical on the KEEP path. - **Security** — DROP comment body is a hardcoded `jq -n '{body:"..."}'` static string; `repo`/`rnum` only flow into the API path and are internal CEO state, not a new injection surface. No new findings, consistent with the description. - **Tests** — KEEP asserts the proposal is filed and no comment posted; DROP asserts nonzero return, nothing filed, and the comment is posted. The `_fj` mock case-orders `/comments*` before `/issues*`, so the comment POST doesn't get mis-bucketed as an issue creation. Good coverage of both branches. - CI `success`; no `.forgejo/` or `.github/workflows/` changes. **Non-blocking observation (follow-up at most):** - In `tick.sh`, the `else` branch logs `"code-check dropped revised proposal"` whenever `ceo_revise_refile` returns nonzero. But the helper also returns the exit status of `ceo_file_proposal` on the KEEP path, so a genuine API/file failure (not a gate DROP) would be logged as a code-check drop. The prior code logged nothing on file failure. The outcome (no re-file) is unchanged; only the log line is slightly misleading for the rare file-failure case. Worth a tweak if log fidelity matters, but not a merge blocker. --- <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 03:30:19 +00:00

1 minute 19 seconds