Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ The worker reports the PR when CI first becomes green rather than waiting for me

### PR ready, landing, and teardown

For PR-based ship tasks, the ready signal depends on mode: `no-mistakes` reports `done [at=<epoch>]: PR <url> checks green` after CI is green, while `direct-PR` reports `done [at=<epoch>]: PR <url>` after opening the PR.
For PR-based ship tasks, the ready signal depends on mode: `no-mistakes` reports `done [at=<epoch>]: PR <url> checks green` after CI is green, while `direct-PR` reports `done [at=<epoch>]: PR <url>` after opening the PR, each only for a non-draft PR; a lane that deliberately holds a draft declares a wait instead, and `bin/fm-pr-check.sh` refuses to arm merge monitoring on a draft.
Run `bin/fm-pr-check.sh <id> <PR url>` with the URL copied from that ready signal - it records `pr=` and the forge's `pr_head=` when available in the task's meta and arms the watcher's merge poll.
Tell the captain the PR's full `https://...` URL copied from the worker's ready line or the task's `pr=` metadata, a concise outcome summary, and the no-mistakes risk level when applicable.
A captain instruction to merge is explicit authority; `yolo` is the only standing routine merge authority.
Expand Down
15 changes: 13 additions & 2 deletions bin/fm-dod-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
# mode is refused rather than silently rendered as the pipeline contract.
# The block opens with the fixed machine-readable "Delivery contract: mode=<mode>"
# line that bin/fm-spawn.sh checks a ship brief against.
# The two PR-based blocks require a non-draft pull request before the done
# report, read back from the forge; a lane that deliberately holds a draft
# declares a paused wait instead. bin/fm-pr-check.sh refuses to arm merge
# monitoring on a draft through the same reading bin/fm-pr-merge.sh uses.
# This file is the one owner of the no-mistakes `--intent` contract: only the
# brief's `## Captain's intent` subsection plus later captain words, never
# `## Firstmate spec` and never the worker's own tradeoffs.
Expand Down Expand Up @@ -249,7 +253,11 @@ fm_dod_block() { # <mode> <task-id>
Delivery contract: mode=direct-PR
This task ships **direct-PR**: you raise the PR yourself, without the no-mistakes pipeline.
The task is complete only when committed on your branch.
When it is implemented and committed, push your branch and open a PR with \`gh-axi\`, then append \`done [at=<epoch>]: PR {url}\` to the status file and stop.
When it is implemented and committed, push your branch and open a PR with \`gh-axi\` that is ready for review, not a draft.
Before you report done, read the PR back from the forge and confirm it is not a draft (\`gh pr view <url> --json isDraft\` must print false); if it is a draft, mark it ready with \`gh-axi pr ready\`.
A draft cannot be merged, so a done report on one leaves the merge unasked.
Then append \`done [at=<epoch>]: PR {url}\` to the status file and stop.
If you deliberately keep the PR a draft, append \`paused [at=<epoch>]: {why the draft is held}\` instead of done.
Do NOT run /no-mistakes. The configured merge authority decides whether to merge the PR; firstmate relays the outcome.
EOF
;;
Expand Down Expand Up @@ -297,7 +305,10 @@ Two firstmate-specific rules layer on top of that guidance:
- NEVER pass \`--yes\` (or \`-y\`) to \`no-mistakes axi run\` or \`no-mistakes axi respond\`. It is banned fleet-wide.
It auto-resolves every gate including ask-user findings with no escalation, and answering your own ask-user finding is a hard rule violation.

After /no-mistakes reports CI green (the CI-ready return point - do not wait for it to keep monitoring in the background until merge), append \`done [at=<epoch>]: PR {url} checks green\` and stop. You are finished.
After /no-mistakes reports CI green (the CI-ready return point - do not wait for it to keep monitoring in the background until merge), read the PR back from the forge and confirm it is not a draft (\`gh pr view <url> --json isDraft\` must print false); if it is a draft, mark it ready with \`gh-axi pr ready\`.
A draft cannot be merged, so a done report on one leaves the merge unasked.
Then append \`done [at=<epoch>]: PR {url} checks green\` and stop. You are finished.
If you deliberately keep the PR a draft, append \`paused [at=<epoch>]: {why the draft is held}\` instead of done.
EOF
;;
*)
Expand Down
18 changes: 18 additions & 0 deletions bin/fm-pr-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
# live only in a private sidecar and are never interpolated into shell source.
# A GitHub pull request URL and a GitLab merge request URL are both accepted,
# including a merge request on a self-hosted GitLab instance.
# A GitHub pull request the forge reports as a draft is refused, naming the draft
# state and recording and arming nothing: a draft cannot be merged, so a poll armed on it
# would wait for an event that cannot occur while nobody is asked to act.
# Mark the pull request ready for review, then arm again; a lane that keeps a
# draft on purpose declares a wait instead of reporting done. An unreadable
# draft state does not refuse, matching how the head read below is optional.
# bin/fm-pr-merge.sh records through this script with FM_PR_CHECK_MERGE=1 and
# skips this refusal, because its own merge-time draft refusal is authoritative.
# Usage: fm-pr-check.sh <task-id> <pr-url>
set -eu

Expand Down Expand Up @@ -60,6 +68,16 @@ if [ "$PROVIDER" = gitlab ] && ! command -v glab >/dev/null 2>&1; then
exit 1
fi

# The draft state is read before anything is recorded or armed. Only a positive
# draft reading refuses, because an unreadable one must not block arming.
if [ "$PROVIDER" = github ] && [ "${FM_PR_CHECK_MERGE:-}" != 1 ] && command -v gh >/dev/null 2>&1 && command -v jq >/dev/null 2>&1; then
DRAFT_JSON=$(gh pr view "$URL" --json isDraft 2>/dev/null || true)
if [ "$(fm_pr_json_draft_state "$DRAFT_JSON")" = true ]; then
echo "error: $URL is a draft pull request; a draft cannot be merged, so merge monitoring would wait for an event that cannot occur - mark it ready for review and arm again, or declare a wait instead of done if the draft is deliberate" >&2
exit 1
fi
fi

"$FM_ROOT/bin/fm-guard.sh" || true

# pr_head is recorded only when the forge's CLI can supply it. gh exposes the
Expand Down
11 changes: 11 additions & 0 deletions bin/fm-pr-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,17 @@ fm_pr_head_valid() {
[[ "$head" =~ ^[0-9a-f]{40}$|^[0-9a-f]{64}$ ]]
}

# The one reading of a GitHub pull request's draft state. Prints "true" or
# "false" for a boolean isDraft and nothing for anything else, so a caller can
# tell a positive draft from an unreadable payload. bin/fm-pr-merge.sh refuses
# a merge unless this prints "false"; bin/fm-pr-check.sh refuses to arm a merge
# poll only when it prints "true".
fm_pr_json_draft_state() { # <pull-request-json>
printf '%s' "${1-}" | jq -r '
if type == "object" and (.isDraft | type) == "boolean" then (.isDraft | tostring) else "" end
' 2>/dev/null || true
}

fm_pr_file_mode() {
if [ "$(uname)" = Darwin ]; then
/usr/bin/stat -f %Lp "$1" 2>/dev/null
Expand Down
7 changes: 3 additions & 4 deletions bin/fm-pr-merge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,6 @@ github_verify_mergeable() {
if ! fields=$(printf '%s' "$json" | jq -r '
if type == "object" then
"state=" + ((.state // "") | tostring),
"draft=" + (if (.isDraft | type) == "boolean" then (.isDraft | tostring) else "" end),
"mergeable=" + ((.mergeable // "") | tostring),
"merge_state=" + ((.mergeStateStatus // "") | tostring),
"head=" + ((.headRefOid // "") | tostring),
Expand All @@ -599,7 +598,6 @@ github_verify_mergeable() {
total=$((total + 1))
case "$line" in
state=*) state=${line#state=} ;;
draft=*) draft=${line#draft=} ;;
mergeable=*) mergeable=${line#mergeable=} ;;
merge_state=*) merge_state=${line#merge_state=} ;;
head=*) live_head=${line#head=} ;;
Expand All @@ -610,11 +608,12 @@ github_verify_mergeable() {
done <<FIELDS
$fields
FIELDS
if [ "$named" -ne 6 ] || [ "$total" -ne 6 ] || [ -z "$base" ]; then
if [ "$named" -ne 5 ] || [ "$total" -ne 5 ] || [ -z "$base" ]; then
echo "error: could not read the GitHub pull request state before merging" >&2
return 1
fi

draft=$(fm_pr_json_draft_state "$json")
if ! fm_pr_head_valid "$live_head"; then
echo "error: could not read the GitHub pull request head commit before merging" >&2
return 1
Expand Down Expand Up @@ -864,7 +863,7 @@ METHODS
}

record_pr_metadata() {
if ! "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL"; then
if ! FM_PR_CHECK_MERGE=1 "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL"; then
return 1
fi
grep -qxF "pr=$URL" "$META" || {
Expand Down
2 changes: 1 addition & 1 deletion docs/scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize
| `fm-pr-lib.sh` | Own canonical task and PR validation plus private atomic PR-poll publication, merge-notification identity, and retirement |
| `fm-pr-poll.sh` | Provide the byte-static watcher program for validated PR/MR-poll sidecars |
| `fm-contributions.sh` | Observe owned publications, retain exact-head judgments, measure required actors, and wake on maintainer signals |
| `fm-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll |
| `fm-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll; refuses a GitHub draft |
| `fm-pr-merge.sh` | Record PR metadata, merge a task's canonical full GitHub or GitLab URL, then refuse an outcome it cannot prove landed or queued |
| `fm-pr-state.sh` | Read-only: print one line per GitHub pull-request blocker it can see, reporting on checks that have reported rather than verdicting merge-readiness |
| `fm-pr-reviewers.sh` | Read-only: suggest reviewers from GitHub's own author mapping of recent commits on a pull request's changed files, never requesting one |
Expand Down
29 changes: 29 additions & 0 deletions tests/fm-brief.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,34 @@ test_faster_paths_use_configured_authority_without_stacked_review() {
pass "fm-brief.sh: faster paths use configured authority without stacked review"
}

# A PR-based ship must not report done on a draft, which cannot be merged; a
# lane that deliberately holds a draft declares a wait instead. local-only opens
# no PR, so it must not carry the requirement.
test_pr_based_dod_requires_non_draft() {
local home mode id brief
home="$TMP_ROOT/draft-dod-home"
mkdir -p "$home/data"
for mode in no-mistakes direct-PR local-only; do
id="brief-draft-$mode"
FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --mode "$mode" >/dev/null 2>&1
brief="$home/data/$id/brief.md"
assert_present "$brief" "$mode: brief was not scaffolded"
if [ "$mode" = local-only ]; then
assert_no_grep "isDraft" "$brief" "$mode: a branch-only delivery must not require a non-draft PR"
continue
fi
# shellcheck disable=SC2016 # single quotes are deliberate: the backticks must stay literal
assert_grep 'confirm it is not a draft (`gh pr view <url> --json isDraft` must print false)' "$brief" \
"$mode: done must require reading the PR back from the forge as non-draft"
# shellcheck disable=SC2016 # single quotes are deliberate: the backticks must stay literal
assert_grep 'mark it ready with `gh-axi pr ready`' "$brief" \
"$mode: a draft must be marked ready before done"
assert_grep "If you deliberately keep the PR a draft, append \`paused" "$brief" \
"$mode: a deliberate draft must declare a wait instead of done"
done
pass "fm-brief.sh: PR-based done requires a non-draft PR; a deliberate draft declares a wait"
}

# Pin the specific line the bug lived on: the no-mistakes DOD's no-mistakes
# reference must render as plain prose with no dangling apostrophe artifact.
test_no_mistakes_dod_wording() {
Expand Down Expand Up @@ -1042,6 +1070,7 @@ test_ship_mode_is_explicit_not_registry
test_delivery_flags_are_refused_where_they_do_not_apply
test_faster_paths_use_configured_authority_without_stacked_review
test_no_mistakes_dod_wording
test_pr_based_dod_requires_non_draft
test_ask_user_escalation_format
test_ship_project_memory_wording
test_herdr_lab_contract_is_explicit_and_complete
Expand Down
41 changes: 41 additions & 0 deletions tests/fm-pr-check-security.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ case "${1:-} ${2:-}" in
printf '%s\n' "{\"state\":\"OPEN\",\"isDraft\":false,\"mergeable\":\"MERGEABLE\",\"mergeStateStatus\":\"CLEAN\",\"headRefOid\":\"${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}\",\"baseRefName\":\"main\",\"statusCheckRollup\":[{\"__typename\":\"CheckRun\",\"name\":\"ci\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\"}]}"
exit 0
;;
*" --json isDraft "*)
printf '%s\n' "{\"isDraft\":${FM_TEST_GH_DRAFT:-false}}"
exit 0
;;
*headRefOid,reviewDecision*)
printf '%s\n' "{\"headRefOid\":\"${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}\",\"reviewDecision\":\"APPROVED\"}"
exit 0
Expand Down Expand Up @@ -518,6 +522,42 @@ test_invalid_entrypoints_have_zero_side_effects() {
pass "PR and teardown entrypoints reject invalid arguments before every side effect"
}

# A draft cannot be merged, so arming a merge poll on one would wait for an event
# that cannot occur. Only a positive draft reading refuses, and it refuses before
# anything is recorded or armed; a ready or unreadable one arms as before.
test_draft_pull_request_is_not_armed() {
local dir rc
dir=$(make_case draft-refused)
write_task_meta "$dir"
cp "$dir/home/state/task-a.meta" "$dir/meta.before"
set +e
FM_TEST_GH_DRAFT=true run_check_entry "$dir" task-a https://github.com/o/r/pull/9 \
> "$dir/stdout" 2> "$dir/stderr"; rc=$?
set -e
[ "$rc" -ne 0 ] || fail "arming accepted a draft pull request"
grep -qi 'draft' "$dir/stderr" || fail "the refusal did not name the draft state"
grep -qF 'https://github.com/o/r/pull/9' "$dir/stderr" || fail "the refusal did not name the pull request"
cmp -s "$dir/meta.before" "$dir/home/state/task-a.meta" || fail "a refused draft changed the task metadata"
[ ! -e "$dir/home/state/task-a.check.sh" ] || fail "a refused draft armed a poll"
[ ! -e "$dir/home/state/task-a.pr-poll" ] || fail "a refused draft wrote a poll sidecar"
[ ! -s "$dir/guard.log" ] || fail "a refused draft reached the guard"

dir=$(make_case draft-cleared)
write_task_meta "$dir"
FM_TEST_GH_DRAFT=false run_check_entry "$dir" task-a https://github.com/o/r/pull/9 \
> "$dir/stdout" 2> "$dir/stderr" || fail "arming refused a pull request that is not a draft"
grep -qxF 'pr=https://github.com/o/r/pull/9' "$dir/home/state/task-a.meta" \
|| fail "a non-draft pull request was not recorded"
[ -f "$dir/home/state/task-a.check.sh" ] || fail "a non-draft pull request was not armed"

dir=$(make_case draft-unreadable)
write_task_meta "$dir"
FM_TEST_GH_DRAFT=null run_check_entry "$dir" task-a https://github.com/o/r/pull/9 \
> "$dir/stdout" 2> "$dir/stderr" || fail "an unreadable draft state blocked arming"
[ -f "$dir/home/state/task-a.check.sh" ] || fail "an unreadable draft state was not armed"
pass "arming refuses a draft pull request, naming it, and arms a ready or unreadable one"
}

test_valid_recording_and_merge_derivation() {
local dir expected sidecar count rc
dir=$(make_case valid-recording)
Expand Down Expand Up @@ -2774,6 +2814,7 @@ test_retirement_refuses_replacement_and_nonterminal_results
test_retirement_queue_failure_and_receipt_tampering
test_gitlab_merged_poll_retires
test_invalid_entrypoints_have_zero_side_effects
test_draft_pull_request_is_not_armed
test_valid_recording_and_merge_derivation
test_rejected_metacharacter_bytes_are_inert
test_static_poll_contract
Expand Down
39 changes: 39 additions & 0 deletions tests/fm-pr-merge.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ case "${1:-} ${2:-}" in
cat "$FM_TEST_GH_HEAD"
exit 0
;;
*isDraft*)
cat "$FM_TEST_GH_VIEW_JSON"
exit 0
;;
esac
;;
"pr merge")
Expand Down Expand Up @@ -2404,6 +2408,40 @@ test_github_red_checks_refuse_and_allow_red_waives_named() {
pass "fm-pr-merge refuses red GitHub checks and waives only a named --allow-red check"
}

# A draft cannot be merged, and neither can a pull request whose draft state the
# forge did not report as a boolean; both refuse before any merge call.
test_github_draft_or_unreadable_draft_state_refuses() {
local case_dir rc head label filter
head=dddddddddddddddddddddddddddddddddddddddd
for label in draft unreadable; do
case_dir=$(make_case "github-$label")
mkdir -p "$case_dir/wt"
add_gh_mocks "$case_dir" "$head"
case "$label" in
draft) filter='.isDraft = true' ;;
*) filter='del(.isDraft)' ;;
esac
jq -c "$filter" "$case_dir/github-view.json" > "$case_dir/github-view.tmp"
mv "$case_dir/github-view.tmp" "$case_dir/github-view.json"

set +e
run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/82 \
> "$case_dir/stdout" 2> "$case_dir/stderr"
rc=$?
set -e
expect_code 1 "$rc" "github-$label: a pull request not read as non-draft must refuse"
assert_grep "the pull request is a draft" "$case_dir/stderr" \
"github-$label: the draft state was not named"
assert_no_grep 'pr merge' "$case_dir/gh.log" \
"github-$label: gh pr merge ran without a non-draft reading"
assert_no_grep 'declare a wait instead of done' "$case_dir/stderr" \
"github-$label: the arm-time draft refusal preempted the merge refusal"
grep -qxF 'pr=https://github.com/example/repo/pull/82' "$case_dir/state/task-x1.meta" \
|| fail "github-$label: pr= was not recorded before the merge refusal"
done
pass "fm-pr-merge refuses a draft pull request and one with no boolean draft state"
}

# When the base branch advances, GitHub cancels a pull request's in-flight run
# and re-triggers it, leaving the cancelled run in the rollup beside the passing
# re-run while reporting the pull request itself CLEAN. The merge must follow the
Expand Down Expand Up @@ -3196,6 +3234,7 @@ test_untraversable_user_backend_config_directory_refuses_the_merge
test_absent_user_backend_config_directory_and_backlog_still_merge
test_backend_override_bypasses_unreadable_user_config
test_github_red_checks_refuse_and_allow_red_waives_named
test_github_draft_or_unreadable_draft_state_refuses
test_superseded_failed_check_run_no_longer_refuses
test_check_runs_never_supersede_status_contexts
test_current_failed_check_run_still_refuses
Expand Down
Loading