feat(bin): add DO IT/DECIDE/REVIEW park filter, ChatGPT return transport, and packet-scoped merge grant - #4315
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a9768137c
ℹ️ 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".
| my $rp = Cwd::realpath($candidate); | ||
| $resolved = defined $rp ? $rp : $candidate; | ||
| } else { |
There was a problem hiding this comment.
When $HOME/inbox/FIRST_MATE_TO_CHATGPT.md does not yet exist, as in a fresh home or CI environment, Cwd::realpath returns undefined for a symlink targeting it and this fallback preserves the symlink pathname instead of its target. Consequently test_crewmate_equivalent_path_spellings_refuse reaches the write, and the newly added tests/fm-chatgpt-return.test.sh exits nonzero; this was reproduced locally. Canonicalize the dangling symlink target before comparing it with the default path.
Useful? React with 👍 / 👎.
| if [ "$park" = 0 ]; then | ||
| occurrence=$(( $(resolution_record_count "$(show_field "$show" body)") + 1 )) | ||
| [ -n "$(body_hold_set_timestamp "$(show_field_value "$show" body)")" ] \ | ||
| || fail "task $id lost its hold-set stamp while being held" | ||
| publish_parent_hold "$id" "$occurrence" needs-decision "$reason" |
There was a problem hiding this comment.
When --park reclassifies an existing captain-held task in a secondmate home, the earlier live hold has already emitted a keyed needs-decision event to the parent, but this branch skips parent publication entirely. The task therefore becomes locally non-actionable while the parent's durable fold continues presenting the old decision as open; emit a matching resolution when transitioning an existing live hold to parked state.
AGENTS.md reference: AGENTS.md:L504-L506
Useful? React with 👍 / 👎.
| | grep -oE '(/pull/[0-9]+|[Pp][Rr][[:space:]]*#[0-9]+|#[0-9]+)' \ | ||
| | grep -oE '[0-9]+' \ | ||
| | LC_ALL=C sort -u |
There was a problem hiding this comment.
When a return lists PRs from different repositories that share a number, such as acme/one#7 and acme/two#7, the collector strips both identifiers to 7 and sort -u counts only one. The verifier then rejects a correct two-PR return, preventing the primary from publishing it; deduplicate by repository plus PR number, or count the distinct listed PR entries instead.
AGENTS.md reference: AGENTS.md:L517-L518
Useful? React with 👍 / 👎.
Parked or standby work now uses a structured parked hold so it is not a current human decision without a live ask. The primary writes ChatGPT-bound returns atomically with PR count/list QA, and an execution packet may grant bounded merge authority that expires with the packet.
…d with new park test coverage
8a97681 to
b9d8b97
Compare
brentwarnes-repo
changed the title
1 participant
Intent
Authorized follow-on packet (Career Pipeline Follow-On Control Packet, rebased 2026-09-12) is in a safe continuation state. Execute the fleet-ops slice only: objectives 1, 4, 6, and 7 as one coordinated First Mate change. Packet file: ~/inbox/CAREER_PIPELINE_FOLLOW_ON_CONTROL_PACKET_REBASED.md. YOLO stays off. Bounded packet-scoped merge is firstmate's job after your PR is green, independently verified, and in-scope — you never merge. Do not publish, send consequential external mail, spend, change permissions, or take destructive action. Objective 13 (phone notify) is PARKED until 2026-10-12; do not dispatch or install anything for it. Objectives 2, 3, 5, 8-12 are not this slice.
Objective 1: add the smallest durable First Mate-local control so Captain-facing escalation uses DO IT / DECIDE / REVIEW. PARK/HOLD is state, not automatically a current Captain decision. A held item must not appear in CURRENT HUMAN DECISIONS merely because Captain authorization would eventually be required to resume it. Treat unnecessary escalation as observable friction. Reuse the existing decision procedure. Add regression coverage that prevents a parked/held/standby item from being classified as a current human decision without a live ask.
Objective 4: whenever the primary First Mate produces a Captain-facing return explicitly intended to be carried back to ChatGPT, automatically write the complete return to ~/inbox/FIRST_MATE_TO_CHATGPT.md BEFORE presenting it in chat. The Captain must not need to request this manually. Scope includes tiny reports as well as large completion reports. Do not write every internal agent message or routine status update. The primary First Mate is the sole/consolidating writer so Second Mates cannot overwrite that file. Transport contents should include, where applicable: generated timestamp; originating task/packet; result/status; concise return; referenced artifact paths; blockers/genuine decisions; Clear Safety footer. Use an atomic write/replace. Preserve the inbox as a transient transport surface, not canonical durable storage. Test with a small ChatGPT-bound return.
Objective 6: update the First Mate execution contract so an execution packet may explicitly grant bounded packet-scoped merge authority that expires with the packet. When granted: conforming implementation PRs can merge without another Captain merge word; verification/review remain mandatory; scope expansion or reserved human gates still stop and escalate; YOLO stays off; do not create standing global autonomous-merge authority.
Objective 7: the completed packet report said four PRs landed but enumerated only PR #32, #33, and #34 in the governance-repo list; later transport also named PR #6 in ai-forward-work-proof (front-door repair). Retrieve actual merge history and reconcile whether a fourth PR was omitted, the count was wrong, or another process created the inconsistency. Repair the smallest reporting/QA so enumerated counts agree with listed items. Do not escalate unless the missing PR reveals a material unauthorized change.
What Changed
bin/fm-captain-hold.shadds a--parkflag that records parked/standby items viatasks-axi --kind parkedinstead of--kind captain, so they get no hold-set stamp, no parentneeds-decisionevent, and nocaptain_actionablestatus;docs/captain-hold-lifecycle.mdandtests/fm-fleet-snapshot-view.test.sh/tests/fm-captain-hold-lifecycle.test.share updated to document and cover the new parked bucket (carries nohold_bucket,captain_actionable: false).bin/fm-chatgpt-return.sh, a new script (withtests/fm-chatgpt-return.test.sh) that assembles a Captain-facing return intended for ChatGPT — timestamp, task/packet, status, return text, artifact paths, blockers, and a Clear Safety footer — verifies enumerated counts against listed items, and atomically writes/replaces~/inbox/FIRST_MATE_TO_CHATGPT.md; it refuses to run from a secondmate home and canonicalizes its guard path to prevent a task-worker equivalence bypass (fixed across two follow-up commits).AGENTS.mdis updated to define bounded packet-scoped merge authority (an execution packet may explicitly grant merge authority for conforming in-scope PRs that expires when the packet closes, without enabling standingyoloor bypassing verification/review/escalation gates), and to route Captain-facing escalation through the DO IT/DECIDE/REVIEW filter so parked/held/standby items aren't treated as current human decisions absent a live ask..agents/skills/ask-user-authority/SKILL.md,.agents/skills/captain-hold-lifecycle/SKILL.md,bin/fm-fleet-snapshot.sh,bin/fm-test-run.sh, anddocs/scripts.mdto wire the new script/flag into existing docs and tooling.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The branch delivers four bounded, well-tested changes (parked/held state filter, atomic ChatGPT-return transport with a live-path bypass that was already caught and fixed in two follow-up review commits within this branch, packet-scoped merge authority documented as a bounded contract change, and a PR-count reconciliation QA regression test reproducing the actual four-vs-three discrepancy); all tests execute real scripts and assert observable behavior rather than source content, and the guard logic (secondmate marker + canonicalized live-path equality for FM_TASK_ID workers) is sound against symlink/
./../double-slash bypass attempts.Testing
Ran the three targeted, pre-existing behavior test suites that map to objectives 1, 4, and 7 (fm-captain-hold-lifecycle, fm-fleet-snapshot-view, fm-chatgpt-return); all passed by invoking the real scripts and asserting on exit codes, stderr messages, and generated file contents (e.g. atomic write of FIRST_MATE_TO_CHATGPT.md, refusal messages, and the four-vs-three PR count/list disagreement detection), not by grepping source. Objective 6 is an AGENTS.md contract/prose change (packet-scoped merge authority) with no executable surface to test; I reviewed the diff and confirmed it states the required bounds (expires with packet, verification/review still mandatory, no standing yolo) without contradiction elsewhere in the file.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-chatgpt-return.test.sh— exercises bin/fm-chatgpt-return.sh write/verify: primary atomic write+replace, secondmate refusal, task-worker live-path refusal (direct, explicit, and symlink/./..-equivalent spellings), and the count/list QA that rejects a 'claimed 4 PRs but listed 3 items' report (objectives 4 and 7)bash tests/fm-captain-hold-lifecycle.test.sh— exercises bin/fm-captain-hold.sh lifecycle including 'parked or standby state is not a current human decision without a live ask' (objective 1)bash tests/fm-fleet-snapshot-view.test.sh— exercises bin/fm-fleet-snapshot.sh JSON output including 'captain-hold buckets are total, mutually exclusive, and never decided by prose' with parked/standby rows carrying null hold_bucket and captain_actionable=false (objective 1)git status --porcelain— confirmed clean worktree with no transient test artifacts left behind✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: Install actionlint locally to fix lint tooling gap
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.