joshtronic/igor

Watch

fix: act on CEO digest board comments same-tick instead of next week #435

Merged

joshtronic merged 3 commits from agent/433-ceo-digest-steering-is-unreachable-the-footer into master

2026-07-28 00:25:10 +00:00

igor commented

2026-07-27 20:57:39 +00:00

Collaborator

What this PR does

fix: act on CEO digest board comments same-tick instead of next week
Add ceo_digest_pending_steering_number -- selector for an open digest carrying a reviewer comment newer than the CEO's own last reply (the watermark; no new state needed)
Add ceo_digest_thread, ceo_build_digest_steering_prompt, _ceo_parse_work, ceo_parse_digest_steering, ceo_file_digest_work in lib/ceo.sh
Wire a new Path 1c into do_ceo_tick (bin/tick.sh) that runs every tick (not week-gated), files any resulting work DIRECTLY as Agent-labeled/unassigned tickets (no second greenlight round-trip), and replies on the digest thread naming what it did -- digest stays open + assigned
Authorization gate: only a comment authored by FORGEJO_REVIEWER counts as steering, in both the pending-selector AND in what's rendered into the steering prompt (ceo_digest_thread filters the thread to reviewer + CEO's own replies only) -- a comment from anyone else never reaches the model
Tests in bin/test-ceo.sh: selector matches a commented digest, does not match one the CEO already replied to, is not suppressed by the weekly ISO-week stamp, rejects a non-reviewer comment, and the thread-rendering itself excludes non-reviewer text

Context

igor#433: the digest footer said "comment to steer" but nothing read a digest comment until the next week's composition (ceo_prior_digest_steering, only called when composing a fresh digest) -- up to 7 days late, even though the CEO already had a same-day act-on-input path for board questions. This adds the same same-day path for digest comments, on the signal the footer already promised, so no footer text changed.

A first attempt at this ticket was rejected in security review for gating the selector on "commenter is not the bot" rather than "commenter is the reviewer" -- that would have let any commenter inject work into the autonomous execution queue. This implementation gates on .user.login == $reviewer everywhere a comment is read for steering, including the prompt-rendering step itself (not just the pending-check), and adds test coverage for a non-reviewer comment at both layers.

Live verification note (per the issue): joshtronic/idolidle.com#27 is open with an unanswered board comment and should be picked up + replied to on the next CEO pass once this lands. Left open, not touched by this PR.

Test plan

make test passes (bin/check-sync.sh + all bin/test-*.sh, including bin/test-ceo.sh)
make lint passes (shellcheck + mdl)
No manual verification needed beyond the above -- this is scripted shell logic with unit coverage; the live end-to-end check is idolidle.com#27 picking up a reply on the next real CEO pass, which is outside this PR's control

Closes #433

## What this PR does - [x] fix: act on CEO digest board comments same-tick instead of next week - [x] Add `ceo_digest_pending_steering_number` -- selector for an open digest carrying a reviewer comment newer than the CEO's own last reply (the watermark; no new state needed) - [x] Add `ceo_digest_thread`, `ceo_build_digest_steering_prompt`, `_ceo_parse_work`, `ceo_parse_digest_steering`, `ceo_file_digest_work` in `lib/ceo.sh` - [x] Wire a new Path 1c into `do_ceo_tick` (`bin/tick.sh`) that runs every tick (not week-gated), files any resulting work DIRECTLY as Agent-labeled/unassigned tickets (no second greenlight round-trip), and replies on the digest thread naming what it did -- digest stays open + assigned - [x] Authorization gate: only a comment authored by `FORGEJO_REVIEWER` counts as steering, in both the pending-selector AND in what's rendered into the steering prompt (`ceo_digest_thread` filters the thread to reviewer + CEO's own replies only) -- a comment from anyone else never reaches the model - [x] Tests in `bin/test-ceo.sh`: selector matches a commented digest, does not match one the CEO already replied to, is not suppressed by the weekly ISO-week stamp, rejects a non-reviewer comment, and the thread-rendering itself excludes non-reviewer text ## Context igor#433: the digest footer said "comment to steer" but nothing read a digest comment until the *next* week's composition (`ceo_prior_digest_steering`, only called when composing a fresh digest) -- up to 7 days late, even though the CEO already had a same-day act-on-input path for board *questions*. This adds the same same-day path for digest *comments*, on the signal the footer already promised, so no footer text changed. A first attempt at this ticket was rejected in security review for gating the selector on "commenter is not the bot" rather than "commenter is the reviewer" -- that would have let any commenter inject work into the autonomous execution queue. This implementation gates on `.user.login == $reviewer` everywhere a comment is read for steering, including the prompt-rendering step itself (not just the pending-check), and adds test coverage for a non-reviewer comment at both layers. Live verification note (per the issue): `joshtronic/idolidle.com#27` is open with an unanswered board comment and should be picked up + replied to on the next CEO pass once this lands. Left open, not touched by this PR. ## Test plan - [x] `make test` passes (`bin/check-sync.sh` + all `bin/test-*.sh`, including `bin/test-ceo.sh`) - [x] `make lint` passes (`shellcheck` + `mdl`) - [x] No manual verification needed beyond the above -- this is scripted shell logic with unit coverage; the live end-to-end check is `idolidle.com#27` picking up a reply on the next real CEO pass, which is outside this PR's control Closes #433

igor added 1 commit

2026-07-27 20:57:39 +00:00

fix: act on CEO digest board comments same-tick instead of next week

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

Details

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

Details

aa9b5e4aa5

igor commented

2026-07-27 21:01:13 +00:00

Author

Collaborator

🤖 Review — `REQUEST_CHANGES` (automated)

CI for aa9b5e4a: success

Adds a same-tick "act on board steering left as a digest comment" path (Path 1c) plus five new helpers in lib/ceo.sh. Scope is tight, the checklist matches the diff item-for-item, the reviewer-authorship gate is genuinely applied at both layers, and CI is green — but two correctness issues in the new code should be fixed before merge.

Blocking

lib/ceo.sh ceo_digest_thread: nothing is ever marked NEW on the first steering round. The jq filter is if ($lb != "" and .created_at > $lb) then "NEW -- ". When the CEO has never replied on the thread, last_b is "", so the guard short-circuits and no comment gets the NEW -- prefix — even though in that state every reviewer comment is unanswered. The prompt built by ceo_build_digest_steering_prompt tells the model "comments marked NEW -- are unanswered", so on the most common case (a fresh digest, first board comment, no CEO reply yet — i.e. exactly the idolidle.com#27 scenario cited in the description) the model is shown a thread with zero unanswered items and may reasonably reply "nothing needed" and file nothing. Fix: if ($lb == "" or .created_at > $lb). Note the existing thread tests both include a BOT_USER comment or don't assert on NEW, so this branch is untested — please add a case with reviewer-only comments asserting the NEW -- prefix.
bin/tick.sh Path 1c: work is filed before the reply is posted, so a failed reply POST re-files duplicate tickets. _ceo_file_digest_work "$repo" "$dparsed" runs first; if the subsequent _fj POST .../comments fails, the code only logs warning: ceo: failed to post steering reply and returns 0. The watermark (BOT_USER's last comment) never moves, so the next tick re-selects the same digest, re-runs the model, and files the same work again — Agent-labeled and unassigned, i.e. straight into the autonomous execution queue. ceo_codecheck_proposal gates on the code, not on already-open CEO-filed tickets, so it won't catch the dupe. Either post the reply first and file work only on success, or dedupe against open issues carrying CEO_DIGEST_WORK_MARKER before filing. A test for the failed-reply path would be worth having.

Non-blocking observations

ceo_digest_thread renders each comment as a single "> [login] body" line; multi-line reviewer comments will only have the first line prefixed, and their subsequent lines land unquoted next to the prompt's own ### section headers. Reviewer is trusted so this isn't a security issue, but it makes the thread section easy to visually corrupt (a comment containing NEW -- or ### The board thread would blend in).
The watermark is "any BOT_USER comment on the digest thread", not "any steering reply". If some other path ever comments on a digest as the bot, it will silently suppress pending steering.
The _fj stub redefined at line ~325 of bin/test-ceo.sh persists for everything after it in the file (the ceo_read_metrics block follows). It appears harmless here since that block doesn't hit the API, but it's a latent trap for the next test added below it.

What's good

The authorization gate is correct and defended at both layers (select(.user.login == $r) in the selector, and the reviewer/bot-only filter in ceo_digest_thread), with tests at both — that addresses the prior security rejection properly.
Watermark-instead-of-new-state is a sound design; the "fresh comment after CEO reply re-fires" test is the right one to have.
Unparseable-response path deliberately posts nothing so the comment is retried rather than dropped — good, and commented as such.
Diff is ~300 lines, in scope, no unrelated refactors, no CI-config or test deletions.

_{Independent review by the harness on claude-opus-5 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `REQUEST_CHANGES` _(automated)_ CI for `aa9b5e4a`: **success** Adds a same-tick "act on board steering left as a digest comment" path (Path 1c) plus five new helpers in `lib/ceo.sh`. Scope is tight, the checklist matches the diff item-for-item, the reviewer-authorship gate is genuinely applied at both layers, and CI is green — but two correctness issues in the new code should be fixed before merge. ## Blocking - **`lib/ceo.sh` `ceo_digest_thread`: nothing is ever marked `NEW` on the first steering round.** The jq filter is `if ($lb != "" and .created_at > $lb) then "NEW -- "`. When the CEO has never replied on the thread, `last_b` is `""`, so the guard short-circuits and *no* comment gets the `NEW --` prefix — even though in that state *every* reviewer comment is unanswered. The prompt built by `ceo_build_digest_steering_prompt` tells the model "comments marked NEW -- are unanswered", so on the most common case (a fresh digest, first board comment, no CEO reply yet — i.e. exactly the `idolidle.com#27` scenario cited in the description) the model is shown a thread with zero unanswered items and may reasonably reply "nothing needed" and file nothing. Fix: `if ($lb == "" or .created_at > $lb)`. Note the existing thread tests both include a `BOT_USER` comment or don't assert on `NEW`, so this branch is untested — please add a case with reviewer-only comments asserting the `NEW --` prefix. - **`bin/tick.sh` Path 1c: work is filed before the reply is posted, so a failed reply POST re-files duplicate tickets.** `_ceo_file_digest_work "$repo" "$dparsed"` runs first; if the subsequent `_fj POST .../comments` fails, the code only logs `warning: ceo: failed to post steering reply` and returns 0. The watermark (`BOT_USER`'s last comment) never moves, so the next tick re-selects the same digest, re-runs the model, and files the same work again — Agent-labeled and unassigned, i.e. straight into the autonomous execution queue. `ceo_codecheck_proposal` gates on the *code*, not on already-open CEO-filed tickets, so it won't catch the dupe. Either post the reply first and file work only on success, or dedupe against open issues carrying `CEO_DIGEST_WORK_MARKER` before filing. A test for the failed-reply path would be worth having. ## Non-blocking observations - `ceo_digest_thread` renders each comment as a single `"> [login] body"` line; multi-line reviewer comments will only have the first line prefixed, and their subsequent lines land unquoted next to the prompt's own `###` section headers. Reviewer is trusted so this isn't a security issue, but it makes the thread section easy to visually corrupt (a comment containing `NEW --` or `### The board thread` would blend in). - The watermark is "any `BOT_USER` comment on the digest thread", not "any steering reply". If some other path ever comments on a digest as the bot, it will silently suppress pending steering. - The `_fj` stub redefined at line ~325 of `bin/test-ceo.sh` persists for everything after it in the file (the `ceo_read_metrics` block follows). It appears harmless here since that block doesn't hit the API, but it's a latent trap for the next test added below it. ## What's good - The authorization gate is correct and defended at both layers (`select(.user.login == $r)` in the selector, and the reviewer/bot-only filter in `ceo_digest_thread`), with tests at both — that addresses the prior security rejection properly. - Watermark-instead-of-new-state is a sound design; the "fresh comment after CEO reply re-fires" test is the right one to have. - Unparseable-response path deliberately posts nothing so the comment is retried rather than dropped — good, and commented as such. - Diff is ~300 lines, in scope, no unrelated refactors, no CI-config or test deletions. --- Independent review by the harness on `claude-opus-5` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.

igor added spent time

2026-07-27 21:01:13 +00:00

1 minute 21 seconds

igor self-assigned this

2026-07-27 21:01:14 +00:00

igor commented

2026-07-27 21:12:24 +00:00

Author

Collaborator

🔧 Rework — round 1 (automated)

Addressed the review on claude-opus-5 at effort high — 1 new commit(s).

### 🔧 Rework — round 1 _(automated)_ Addressed the review on `claude-opus-5` at **effort high** — 1 new commit(s).

igor removed their assignment

2026-07-27 21:12:52 +00:00

igor added 1 commit

2026-07-27 21:12:52 +00:00

fix: mark the first steering round NEW; reply before filing work

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

Details

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

Details

e8a7a075d7

Two correctness fixes from review on the digest-steering path.

ceo_digest_thread's NEW flag short-circuited on an empty watermark, so
on the first steering round -- the CEO has never replied, every reviewer
comment is unanswered -- the model was handed a thread with nothing
marked pending, on exactly the case the path exists for. The guard is
now "no watermark OR newer than it". Comments are also quoted on every
line, so a multi-line comment's tail can't sit flush against the
prompt's own section headers.

The reply is the watermark, so posting it AFTER filing work meant a
failed reply POST left the watermark parked and the next tick re-filed
the same Agent-labeled tickets into the autonomous queue. Filing now
lives behind the reply in ceo_commit_digest_steering (moved out of
tick.sh so the ordering is unit-testable): reply first, rc 1 with
nothing filed if it fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

igor added spent time

2026-07-27 21:12:52 +00:00

8 minutes 50 seconds

igor commented

2026-07-27 21:16:19 +00:00

Author

Collaborator

🤖 Review — `REQUEST_CHANGES` (automated)

CI for e8a7a075: success

Adds a same-tick "act on board comment on the open CEO digest" path (selector + thread render + parse + direct Agent-ticket filing + reply), wired as Path 1c in do_ceo_tick. Scope is tight, the checklist matches the diff item-for-item, CI is green, and the reviewer-only authorization gate is genuinely applied at both layers (selector and prompt render) with tests for each. One concrete defect blocks.

Blocking

lib/ceo.sh:ceo_digest_pending_steering_number — unset BOT_USER turns this into an unbounded ticket-filing loop. The watermark is [ .[]? | select(.user.login == $b) | .created_at ] | last, with $b defaulted to "" via ${BOT_USER:-}. If BOT_USER is empty/unset at runtime, no comment ever matches, last_b stays empty, and the [ -z "$last_b" ] branch reports pending forever — even after the CEO's own reply lands. Every tick then: model call → reply posted → the same ===WORK=== blocks re-filed as Agent-labeled, unassigned tickets straight into the autonomous queue. That is precisely the failure mode the reply-first ordering comment in ceo_commit_digest_steering says it exists to prevent, just reached by a different door. The three ${BOT_USER:-} defaults in this diff suggest the author isn't certain the var is always set; I can't verify it from the diff either.
- Fix: fail closed at the top of the selector — [ -n "${BOT_USER:-}" ] || return 0 (same shape as the existing [ -n "$reviewer" ] || return 0) — and add a bin/test-ceo.sh case asserting the selector returns empty when BOT_USER is unset. The tests currently set BOT_USER="igor" unconditionally, so this hole is invisible to the suite.

Non-blocking, worth a look

lib/ceo.sh:ceo_commit_digest_steering — stdin sharing in the work loop. while IFS= read -r w; ... done < <(jq -c '.[]?' ...) calls ceo_codecheck_proposal (a model-backed gate) inside the loop body. If that call — or anything under it — reads stdin, it eats the remaining records and the second work item is silently dropped, no log line. Worth < /dev/null on the codecheck/file calls, or reading the array into a var first. (May be pre-existing pattern from the proposal path; flagging because the cap is 2 and the loss would be silent.)
Non-CEO bot comments move the watermark. Any comment posted by BOT_USER on the digest thread after a reviewer comment — from this path or any other automation — permanently suppresses that steering with no log line. Consider anchoring the watermark on a marker the CEO stamps into its steering replies rather than "any bot comment".
bin/test-ceo.sh — stubs that outlive the block. _CC_ORIG/ceo_codecheck_proposal is carefully restored, but log() { :; } and forgejo_add_label() are redefined and never restored, and they shadow for the rest of the file (ceo_read_metrics and everything after). CI is green so nothing currently depends on them, but this is a latent way to silently neuter a later assertion. Same treatment as _CC_ORIG would be cheap.
ceo_build_digest_steering_prompt has no direct test. Everything around it is covered; the prompt builder itself (including that the thread is actually interpolated) is not.
Timestamp comparison [[ "$last_r" > "$last_b" ]] is a lexical compare — correct for Forgejo's Z-suffixed RFC3339, but would silently misorder if the API ever returns offset-form (+00:00) timestamps. Fine as-is; noting it.

Things I checked and am satisfied with

Every checked box in the description corresponds to real code in the diff; no fabricated claims found. ceo_commit_digest_steering is extra, not missing.
The security regression called out in the description is genuinely fixed: .user.login == $reviewer gates the selector, and ceo_digest_thread filters to reviewer+bot before rendering, with hasnt assertions on both the login and the injected text.
Reply-before-file ordering and the dparsed empty → continue (watermark unmoved, retried) path are correct for idempotence, modulo the BOT_USER issue above.
No CI/workflow files touched; no secrets, no injection surface beyond the reviewer-authored comment that is the intended trust boundary.

_{Independent review by the harness on claude-opus-5 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `REQUEST_CHANGES` _(automated)_ CI for `e8a7a075`: **success** Adds a same-tick "act on board comment on the open CEO digest" path (selector + thread render + parse + direct Agent-ticket filing + reply), wired as Path 1c in `do_ceo_tick`. Scope is tight, the checklist matches the diff item-for-item, CI is green, and the reviewer-only authorization gate is genuinely applied at *both* layers (selector and prompt render) with tests for each. One concrete defect blocks. ## Blocking - **`lib/ceo.sh:ceo_digest_pending_steering_number` — unset `BOT_USER` turns this into an unbounded ticket-filing loop.** The watermark is `[ .[]? | select(.user.login == $b) | .created_at ] | last`, with `$b` defaulted to `""` via `${BOT_USER:-}`. If `BOT_USER` is empty/unset at runtime, no comment ever matches, `last_b` stays empty, and the `[ -z "$last_b" ]` branch reports *pending* forever — even after the CEO's own reply lands. Every tick then: model call → reply posted → the same `===WORK===` blocks re-filed as **Agent-labeled, unassigned** tickets straight into the autonomous queue. That is precisely the failure mode the reply-first ordering comment in `ceo_commit_digest_steering` says it exists to prevent, just reached by a different door. The three `${BOT_USER:-}` defaults in this diff suggest the author isn't certain the var is always set; I can't verify it from the diff either. - Fix: fail closed at the top of the selector — `[ -n "${BOT_USER:-}" ] || return 0` (same shape as the existing `[ -n "$reviewer" ] || return 0`) — and add a `bin/test-ceo.sh` case asserting the selector returns empty when `BOT_USER` is unset. The tests currently set `BOT_USER="igor"` unconditionally, so this hole is invisible to the suite. ## Non-blocking, worth a look - **`lib/ceo.sh:ceo_commit_digest_steering` — stdin sharing in the work loop.** `while IFS= read -r w; ... done < <(jq -c '.[]?' ...)` calls `ceo_codecheck_proposal` (a model-backed gate) inside the loop body. If that call — or anything under it — reads stdin, it eats the remaining records and the second work item is silently dropped, no log line. Worth `< /dev/null` on the codecheck/file calls, or reading the array into a var first. (May be pre-existing pattern from the proposal path; flagging because the cap is 2 and the loss would be silent.) - **Non-CEO bot comments move the watermark.** Any comment posted by `BOT_USER` on the digest thread after a reviewer comment — from this path or any other automation — permanently suppresses that steering with no log line. Consider anchoring the watermark on a marker the CEO stamps into its steering replies rather than "any bot comment". - **`bin/test-ceo.sh` — stubs that outlive the block.** `_CC_ORIG`/`ceo_codecheck_proposal` is carefully restored, but `log() { :; }` and `forgejo_add_label()` are redefined and never restored, and they shadow for the rest of the file (`ceo_read_metrics` and everything after). CI is green so nothing currently depends on them, but this is a latent way to silently neuter a later assertion. Same treatment as `_CC_ORIG` would be cheap. - **`ceo_build_digest_steering_prompt` has no direct test.** Everything around it is covered; the prompt builder itself (including that the thread is actually interpolated) is not. - **Timestamp comparison** `[[ "$last_r" > "$last_b" ]]` is a lexical compare — correct for Forgejo's `Z`-suffixed RFC3339, but would silently misorder if the API ever returns offset-form (`+00:00`) timestamps. Fine as-is; noting it. ## Things I checked and am satisfied with - Every checked box in the description corresponds to real code in the diff; no fabricated claims found. `ceo_commit_digest_steering` is extra, not missing. - The security regression called out in the description is genuinely fixed: `.user.login == $reviewer` gates the selector, and `ceo_digest_thread` filters to reviewer+bot before rendering, with `hasnt` assertions on both the login and the injected text. - Reply-before-file ordering and the `dparsed` empty → `continue` (watermark unmoved, retried) path are correct for idempotence, modulo the `BOT_USER` issue above. - No CI/workflow files touched; no secrets, no injection surface beyond the reviewer-authored comment that is the intended trust boundary. --- Independent review by the harness on `claude-opus-5` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.

igor added spent time

2026-07-27 21:16:20 +00:00

1 minute 46 seconds

igor self-assigned this

2026-07-27 21:16:20 +00:00

igor commented

2026-07-27 21:26:57 +00:00

Author

Collaborator

🔧 Rework — round 2 (automated)

Addressed the review on claude-opus-5 at effort xhigh — 1 new commit(s).

### 🔧 Rework — round 2 _(automated)_ Addressed the review on `claude-opus-5` at **effort xhigh** — 1 new commit(s).

igor removed their assignment

2026-07-27 21:27:13 +00:00

igor added spent time

2026-07-27 21:27:13 +00:00

8 minutes 4 seconds

igor added 1 commit

2026-07-27 21:27:14 +00:00

fix: fail closed when BOT_USER is unresolved in the steering selector

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

Details

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

Details

1938714e0a

The CEO's own last reply is the watermark for "has this board comment been
answered". With an empty $BOT_USER that watermark matches nothing, reads as
"never replied", and ceo_digest_pending_steering_number reports pending
forever -- every tick re-running the model and re-filing the same
Agent-labeled, unassigned tickets straight into the autonomous queue. Guard
it the same way the reviewer check is guarded, and cover it: the suite set
BOT_USER unconditionally, so the hole was invisible.

Also from review:

- </dev/null on the work loop's model-backed codecheck + file calls. If
  anything under them read stdin it drained the process substitution and the
  second work item vanished with no log line. Covered by a stdin-eating gate
  stub (red before the fix: only "First" got filed).
- Restore the log() and forgejo_add_label() stubs after the steering block
  instead of letting them shadow the rest of the file.
- A direct test for ceo_build_digest_steering_prompt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

igor commented

2026-07-27 21:31:21 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 1938714e: success

Adds a same-tick "Path 1c" so the CEO acts on reviewer comments left on the open weekly digest instead of waiting for next week's composition. Code is careful, well-commented, and unusually well-tested; CI is green and the checklist matches the diff. I'm not blocking, but there are a few things I want a human to weigh before merge.

Checklist / scope

Every checked item maps to real diff content: ceo_digest_pending_steering_number, ceo_digest_thread, ceo_build_digest_steering_prompt, _ceo_parse_work, ceo_parse_digest_steering, ceo_file_digest_work all exist in lib/ceo.sh; Path 1c is wired in bin/tick.sh; the reviewer-author gate is applied in both the selector and ceo_digest_thread; the claimed tests (non-reviewer rejected at both layers, ISO-week stamp doesn't suppress, already-replied watermark) are all present. No fabrications found.
Diff is ~475 added lines, over the ~400 budget (lib +218, tests +217, tick +40). Nothing was trimmed to fit — the overage is real content — but it's worth noting the split-into-stacked-PRs option wasn't taken.

Things a human should decide on

Authority expansion (ceo_file_digest_work, lib/ceo.sh). This is the substantive change: model output derived from a free-text reviewer comment now becomes an Agent-labeled, unassigned ticket that the autonomous grind picks up with no second greenlight. The gate is .user.login == $FORGEJO_REVIEWER (correct, and fails closed on empty $reviewer and empty $BOT_USER), and the code-check gate still runs — but I can't see issue #433's text, so I can't verify the claimed "requirement 6: no second greenlight round-trip" is actually what was asked for. If it isn't, this is a policy change, not a bug fix.
continue on an unparseable steering response (bin/tick.sh, Path 1c). Two effects worth confirming are intended: (1) it skips Path 2 for that repo, so a digest comment the model can never parse indefinitely blocks that repo's weekly digest composition, not just the steering reply; (2) unlike the success path (return 0, "one model-backed action per tick"), this burns two model calls and then moves on to the next repo, where more calls can happen in the same tick.

Smaller findings

Timestamp comparison is lexicographic ([[ "$last_r" > "$last_b" ]] and sort | last in ceo_digest_pending_steering_number). Fine for same-server RFC3339 strings, but if Forgejo returns a non-Z offset, a DST transition can misorder the watermark for a one-hour window. Fixtures only cover Z. Low severity; worth a comment or a normalized compare if you care.
forgejo_add_label "$repo" "$num" "Agent" is only exercised against a test stub (bin/test-ceo.sh, _AL_ORIG block) that hardcodes the $2/$3 positions. If the real signature differs, the stub masks it — worth an eyeball against another caller in lib/ceo.sh.
_fj is redefined at the start of the new Phase 5 test block and never restored, so it shadows for the remainder of bin/test-ceo.sh. The later ceo_codecheck_proposal/forgejo_add_label/log stubs are carefully restored (good), which makes _fj the odd one out. CI green suggests nothing downstream depends on the old one, but it's a latent trap for the next test added below it.
No test for the [ -n "$reviewer" ] || return 0 early-out (empty FORGEJO_REVIEWER), though the parallel BOT_USER fail-closed case is covered.

Nothing blocking found

No injection/secret/auth issues beyond the authority question above; no CI-workflow changes; reply-before-file ordering and the </dev/null on the model-backed gate are both correct and explicitly tested.

_{Independent review by the harness on claude-opus-5 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `COMMENT` _(automated)_ CI for `1938714e`: **success** Adds a same-tick "Path 1c" so the CEO acts on reviewer comments left on the open weekly digest instead of waiting for next week's composition. Code is careful, well-commented, and unusually well-tested; CI is green and the checklist matches the diff. I'm not blocking, but there are a few things I want a human to weigh before merge. ## Checklist / scope - Every checked item maps to real diff content: `ceo_digest_pending_steering_number`, `ceo_digest_thread`, `ceo_build_digest_steering_prompt`, `_ceo_parse_work`, `ceo_parse_digest_steering`, `ceo_file_digest_work` all exist in `lib/ceo.sh`; Path 1c is wired in `bin/tick.sh`; the reviewer-author gate is applied in **both** the selector and `ceo_digest_thread`; the claimed tests (non-reviewer rejected at both layers, ISO-week stamp doesn't suppress, already-replied watermark) are all present. No fabrications found. - **Diff is ~475 added lines**, over the ~400 budget (lib +218, tests +217, tick +40). Nothing was trimmed to fit — the overage is real content — but it's worth noting the split-into-stacked-PRs option wasn't taken. ## Things a human should decide on - **Authority expansion (`ceo_file_digest_work`, lib/ceo.sh).** This is the substantive change: model output derived from a free-text reviewer comment now becomes an `Agent`-labeled, **unassigned** ticket that the autonomous grind picks up with *no* second greenlight. The gate is `.user.login == $FORGEJO_REVIEWER` (correct, and fails closed on empty `$reviewer` and empty `$BOT_USER`), and the code-check gate still runs — but I can't see issue #433's text, so I can't verify the claimed "requirement 6: no second greenlight round-trip" is actually what was asked for. If it isn't, this is a policy change, not a bug fix. - **`continue` on an unparseable steering response (bin/tick.sh, Path 1c).** Two effects worth confirming are intended: (1) it skips Path 2 for that repo, so a digest comment the model can never parse indefinitely blocks that repo's *weekly digest composition*, not just the steering reply; (2) unlike the success path (`return 0`, "one model-backed action per tick"), this burns two model calls and then moves on to the next repo, where more calls can happen in the same tick. ## Smaller findings - **Timestamp comparison is lexicographic** (`[[ "$last_r" > "$last_b" ]]` and `sort | last` in `ceo_digest_pending_steering_number`). Fine for same-server RFC3339 strings, but if Forgejo returns a non-`Z` offset, a DST transition can misorder the watermark for a one-hour window. Fixtures only cover `Z`. Low severity; worth a comment or a normalized compare if you care. - **`forgejo_add_label "$repo" "$num" "Agent"` is only exercised against a test stub** (`bin/test-ceo.sh`, `_AL_ORIG` block) that hardcodes the `$2`/`$3` positions. If the real signature differs, the stub masks it — worth an eyeball against another caller in `lib/ceo.sh`. - `_fj` is **redefined** at the start of the new Phase 5 test block and never restored, so it shadows for the remainder of `bin/test-ceo.sh`. The later `ceo_codecheck_proposal`/`forgejo_add_label`/`log` stubs *are* carefully restored (good), which makes `_fj` the odd one out. CI green suggests nothing downstream depends on the old one, but it's a latent trap for the next test added below it. - No test for the `[ -n "$reviewer" ] || return 0` early-out (empty `FORGEJO_REVIEWER`), though the parallel `BOT_USER` fail-closed case is covered. ## Nothing blocking found No injection/secret/auth issues beyond the authority question above; no CI-workflow changes; reply-before-file ordering and the `</dev/null` on the model-backed gate are both correct and explicitly tested. --- Independent review by the harness on `claude-opus-5` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.

igor added spent time

2026-07-27 21:31:22 +00:00

1 minute 47 seconds

igor requested review from joshtronic

2026-07-27 21:31:22 +00:00