diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index fdf3c51e7b..9f0070bee1 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -7,9 +7,18 @@ # when the task genuinely deviates (e.g. working an existing external PR instead # of shipping a new one). # Usage: fm-brief.sh [--scout] [--herdr-lab] +# [--pr-repo / --pr-base ] # fm-brief.sh --secondmate {...|--no-projects} # --scout writes the scout contract instead: the deliverable is a report at # data//report.md (no branch, no push, no PR) and the worktree is scratch. +# --pr-repo/--pr-base name the explicit PR target for a ship brief whose +# delivery mode opens a pull request (no-mistakes or direct-PR). Required for +# those modes, exactly as caller-supplied as the repo-name argument itself, +# so the scaffolded brief always names a concrete `--repo / +# --base ` target and a bare `gh pr create` (which can silently +# default to GitHub's own fork-parent repo instead of this fleet's +# origin/fork convention) never appears. Not required for --scout or +# local-only, neither of which opens a PR. # --secondmate writes a persistent secondmate charter. The project list # is cloned into the secondmate home, while the natural-language scope # tells the main firstmate when to route work there; routine churn stays in its own home; @@ -99,15 +108,20 @@ fi KIND=ship HERDR_LAB=0 NO_PROJECTS=0 +PR_REPO="" +PR_BASE="" POS=() -for a in "$@"; do - case "$a" in +while [ $# -gt 0 ]; do + case "$1" in --scout) KIND=scout ;; --secondmate) KIND=secondmate ;; --herdr-lab) HERDR_LAB=1 ;; --no-projects) NO_PROJECTS=1 ;; - *) POS+=("$a") ;; + --pr-repo) shift; PR_REPO=${1:?"error: --pr-repo requires a value"} ;; + --pr-base) shift; PR_BASE=${1:?"error: --pr-base requires a value"} ;; + *) POS+=("$1") ;; esac + shift done ID=${POS[0]} @@ -220,6 +234,18 @@ fi REPO=${POS[1]} +# herdr's clone lands a worker at a detached HEAD on the default branch, which +# tracks the third-party upstream `origin`, not the captain's own `fork` - +# the actual working base with all landed work. This has stalled dispatched +# herdr tasks twice by omission (data/learnings.md, "Fork base"), so bake the +# check into every herdr brief instead of relying on it being hand-added. +if [ "$REPO" = herdr ]; then +# shellcheck disable=SC2016 # single quotes are deliberate: the backtick-wrapped commands are literal brief text, not shell expansion. +FORK_BASE_NOTE=$'\n\n''**FIRST: verify or rebase onto `fork/master` before doing anything else.** The clone lands you at a detached HEAD on its default branch, which tracks `origin` - the third-party upstream `ogulcancelik/herdr` - not `fork` (`brchue-ux/herdr`), the actual working base with all landed captain work; the two have diverged and neither is an ancestor of the other. Run `git fetch fork`, then `git merge-base --is-ancestor fork/master HEAD`; if that fails, rebase onto `fork/master` before creating your branch.' +else +FORK_BASE_NOTE="" +fi + if [ "$HERDR_LAB" -eq 1 ]; then HERDR_LAB_HELPER=$(shell_quote "$FM_ROOT/bin/fm-herdr-lab.sh") # shellcheck disable=SC2016 # single quotes are deliberate: these lines are literal brief text whose backtick-wrapped $(...) and "$HERDR_LAB_SESSION" snippets must reach the reading agent verbatim, not expand at scaffold time; only the '"$VAR"' break-outs interpolate. @@ -262,7 +288,7 @@ You are a crewmate: an autonomous worker agent managed by firstmate. Work on you $HERDR_SECTION # Setup -You are in a disposable git worktree of $REPO, at a detached HEAD on a clean default branch. +You are in a disposable git worktree of $REPO, at a detached HEAD on a clean default branch.$FORK_BASE_NOTE This is a SCOUT task: the deliverable is a written report, not a PR. The worktree is your laboratory - install, run, edit, and make scratch commits freely; all of it is discarded at teardown. The report is the only thing that survives, so anything worth keeping must be in it. @@ -307,6 +333,23 @@ read -r MODE _ < +# herdrdev/herdr, brchue-ux/firstmate -> kunchenguid/firstmate; see +# data/learnings.md, "`gh pr create` with no `--repo`..."). Every mode that +# opens a PR must name its explicit target, supplied the same way the repo +# argument itself is: as a caller-supplied argument, not inferred here. +case "$MODE" in + local-only) ;; # never opens a PR + *) + case "$PR_REPO" in + */*) ;; + *) echo "error: --pr-repo / is required for a $MODE ship brief (repo: $REPO)" >&2; exit 1 ;; + esac + [ -n "$PR_BASE" ] || { echo "error: --pr-base is required for a $MODE ship brief (repo: $REPO)" >&2; exit 1; } + ;; +esac + case "$MODE" in direct-PR) SETUP2="" @@ -315,7 +358,7 @@ case "$MODE" in # Definition of done This project 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: PR {url}\` to the status file and stop. +When it is implemented and committed, push your branch and open a PR with \`gh-axi pr create --repo $PR_REPO --base $PR_BASE\` - never a bare \`gh pr create\`, which can silently target the wrong repository - then append \`done: PR {url}\` to the status file and stop. Do NOT run /no-mistakes. The configured merge authority decides whether to merge the PR; firstmate relays the outcome. EOF ;; @@ -340,6 +383,10 @@ EOF The task is complete only when committed on your branch. When you believe it is complete, append \`done: {summary}\` to the status file and stop. Firstmate will then instruct you to run /no-mistakes to validate and ship a PR. +This project's PR target is \`$PR_REPO\` on \`$PR_BASE\`. The pipeline owns the actual push and PR +creation, but if you ever need to run \`gh-axi pr create\` yourself, always pass +\`--repo $PR_REPO --base $PR_BASE\` explicitly - never a bare \`gh pr create\`, which can silently +target the wrong repository. You drive no-mistakes by responding to its gates, not by implementing fixes. Follow the guidance no-mistakes itself provides for the mechanics: it loads when you invoke /no-mistakes, and \`no-mistakes axi run --help\` plus the \`help\` lines in each \`axi\` response are authoritative and version-matched to the installed binary. @@ -370,7 +417,7 @@ You are a crewmate: an autonomous worker agent managed by firstmate. Work on you $HERDR_SECTION # Setup -You are in a disposable git worktree of $REPO, at a detached HEAD on a clean default branch. +You are in a disposable git worktree of $REPO, at a detached HEAD on a clean default branch.$FORK_BASE_NOTE **Verify isolation before anything else.** Run \`pwd -P\` and \`git rev-parse --show-toplevel\`; both must resolve to the disposable task worktree you were launched in, such as a treehouse pool path or an Orca-managed worktree, not the primary checkout firstmate operates from. The path check is authoritative: \`git rev-parse --git-dir\` and \`git rev-parse --git-common-dir\` can help inspect the repo, but they do not prove you are outside the primary checkout. diff --git a/tests/fm-ask-user-authority.test.sh b/tests/fm-ask-user-authority.test.sh index 89ec517fa1..a4ab25016f 100644 --- a/tests/fm-ask-user-authority.test.sh +++ b/tests/fm-ask-user-authority.test.sh @@ -14,7 +14,7 @@ test_primary_and_secondmate_instruction_generation() { mkdir -p "$home/data" FM_HOME="$home" FM_ROOT_OVERRIDE="$ROOT" \ - "$BRIEF" authority-worker sample >/dev/null 2>&1 + "$BRIEF" authority-worker sample --pr-repo owner/sample --pr-base main >/dev/null 2>&1 ship="$home/data/authority-worker/brief.md" assert_grep 'ask-user findings are never yours to answer' "$ship" \ "generated implementation brief lets the worker own an ask-user decision" diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 0199311824..8cd28923d9 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -201,7 +201,7 @@ test_ship_modes_generate_clean_briefs() { for id_proj in "brief-nomistakes-a1:no-registry-proj" "brief-directpr-a2:direct-proj" "brief-localonly-a3:local-proj"; do id=${id_proj%%:*} proj=${id_proj##*:} - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" "$proj" >/dev/null 2>&1; status=$? + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" "$proj" --pr-repo owner/"$proj" --pr-base main >/dev/null 2>&1; status=$? expect_code 0 "$status" "fm-brief.sh $id $proj should exit 0" brief="$home/data/$id/brief.md" assert_present "$brief" "$id: brief was not scaffolded" @@ -219,7 +219,7 @@ test_faster_paths_use_configured_authority_without_stacked_review() { home="$TMP_ROOT/configured-authority-home" write_registry "$home" id="brief-direct-authority-a4" - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" direct-proj >/dev/null 2>&1 + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" direct-proj --pr-repo owner/direct-proj --pr-base main >/dev/null 2>&1 brief="$home/data/$id/brief.md" assert_grep "The configured merge authority decides whether to merge the PR; firstmate relays the outcome." "$brief" \ "direct-PR brief lost configured merge authority" @@ -244,7 +244,7 @@ test_no_mistakes_dod_wording() { home="$TMP_ROOT/wording-home" mkdir -p "$home/data" id="brief-wording-b1" - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj >/dev/null 2>&1 + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --pr-repo owner/some-proj --pr-base main >/dev/null 2>&1 brief="$home/data/$id/brief.md" assert_present "$brief" "brief was not scaffolded" assert_grep "no-mistakes itself provides for the mechanics" "$brief" \ @@ -269,7 +269,7 @@ test_ship_project_memory_wording() { home="$TMP_ROOT/project-memory-home" mkdir -p "$home/data" id="brief-memory-c1" - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj >/dev/null 2>&1 + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --pr-repo owner/some-proj --pr-base main >/dev/null 2>&1 brief="$home/data/$id/brief.md" assert_present "$brief" "brief was not scaffolded" assert_grep "Record only project knowledge useful to almost every future session." "$brief" \ @@ -286,7 +286,7 @@ test_herdr_lab_contract_is_explicit_and_complete() { home="$TMP_ROOT/herdr-lab-home" mkdir -p "$home/data" id="brief-herdr-lab-d1" - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" firstmate --herdr-lab >/dev/null 2>&1 + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" firstmate --herdr-lab --pr-repo owner/firstmate --pr-base main >/dev/null 2>&1 brief="$home/data/$id/brief.md" assert_present "$brief" "Herdr lab brief was not scaffolded" assert_grep "# Herdr isolation - HARD SAFETY CONTRACT" "$brief" \ @@ -338,7 +338,7 @@ test_herdr_lab_omission_is_loud_for_ship_and_scout() { if [ "$kind" = scout ]; then FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" firstmate --scout >/dev/null 2>&1 else - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" firstmate >/dev/null 2>&1 + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" firstmate --pr-repo owner/firstmate --pr-base main >/dev/null 2>&1 fi brief="$home/data/$id/brief.md" assert_grep "# Herdr lifecycle declaration - NOT ENABLED" "$brief" \ @@ -553,7 +553,7 @@ test_pause_verb_override_renders_all_brief_scaffolds() { case "$kind" in ship) FM_HOME="$home" FM_CLASSIFY_PAUSED_VERB=awaiting \ - "$ROOT/bin/fm-brief.sh" "$id" firstmate >/dev/null 2>&1 + "$ROOT/bin/fm-brief.sh" "$id" firstmate --pr-repo owner/firstmate --pr-base main >/dev/null 2>&1 ;; scout) FM_HOME="$home" FM_CLASSIFY_PAUSED_VERB=awaiting \ @@ -618,6 +618,96 @@ test_scout_and_secondmate_scaffold() { pass "fm-brief: scout and secondmate code paths still scaffold well-formed briefs" } +# herdr's clone lands a worker at a detached HEAD on the default branch, which +# tracks the third-party upstream `origin`, not the captain's working `fork`. +# This has stalled dispatched herdr tasks twice by omission (data/learnings.md, +# "Fork base"), so the scaffold must bake the check in automatically for both +# ship and scout herdr briefs, with no manual edit required. +test_herdr_repo_bakes_in_fork_base_check() { + local home brief + home="$TMP_ROOT/herdr-fork-base-home" + mkdir -p "$home/data" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" herdr-ship herdr \ + --pr-repo brchue-ux/herdr --pr-base master >/dev/null 2>&1 \ + || fail "fm-brief.sh herdr ship scaffold exited non-zero" + brief="$home/data/herdr-ship/brief.md" + [ "$(grep -c 'fork/master' "$brief")" -gt 0 ] \ + || fail "herdr ship brief missing the fork/master base check" + assert_grep "verify or rebase onto \`fork/master\`" "$brief" \ + "herdr ship brief missing the fork-base instruction" + assert_grep "the third-party upstream \`ogulcancelik/herdr\`" "$brief" \ + "herdr ship brief did not name the upstream remote it must not build on" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" herdr-scout herdr --scout >/dev/null 2>&1 \ + || fail "fm-brief.sh herdr scout scaffold exited non-zero" + brief="$home/data/herdr-scout/brief.md" + [ "$(grep -c 'fork/master' "$brief")" -gt 0 ] \ + || fail "herdr scout brief missing the fork/master base check" + + # A non-herdr repo must not pick up herdr-specific base instructions. + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" other-ship some-proj \ + --pr-repo owner/some-proj --pr-base main >/dev/null 2>&1 \ + || fail "fm-brief.sh non-herdr ship scaffold exited non-zero" + brief="$home/data/other-ship/brief.md" + assert_no_grep "fork/master" "$brief" \ + "non-herdr brief picked up the herdr-only fork-base check" + pass "fm-brief.sh: herdr briefs bake in the fork/master base check automatically" +} + +# A bare `gh pr create` targets GitHub's own fork-parent metadata instead of +# this fleet's origin/fork convention (data/learnings.md, "`gh pr create` with +# no `--repo`..."), confirmed live for both brchue-ux/herdr -> herdrdev/herdr +# and brchue-ux/firstmate -> kunchenguid/firstmate. Every ship brief whose mode +# opens a PR must name an explicit target, supplied the same way the repo +# argument itself is: as a caller-supplied --pr-repo/--pr-base pair. +test_ship_pr_target_is_explicit() { + local home brief status + home="$TMP_ROOT/pr-target-home" + write_registry "$home" + + # direct-PR mode: the worker runs gh-axi itself, so the command must be + # fully explicit (this is the exact incident shape: "push your branch and + # open a PR with `gh-axi`" with no target, which defaulted to herdrdev/herdr). + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" herdr-pr-target direct-proj \ + --pr-repo brchue-ux/herdr --pr-base master >/dev/null 2>&1 \ + || fail "fm-brief.sh direct-PR scaffold with --pr-repo exited non-zero" + brief="$home/data/herdr-pr-target/brief.md" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_grep '`gh-axi pr create --repo brchue-ux/herdr --base master`' "$brief" \ + "direct-PR brief did not render the explicit PR-creation command" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_no_grep 'open a PR with `gh-axi`, then append' "$brief" \ + "direct-PR brief kept the generic no-target gh-axi instruction" + + # no-mistakes mode: the pipeline owns the actual push and PR, but the brief + # must still name the explicit target for any manual gh-axi fallback + # (this is the exact incident shape: brchue-ux/firstmate -> kunchenguid/firstmate). + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" firstmate-pr-target firstmate \ + --pr-repo brchue-ux/firstmate --pr-base main >/dev/null 2>&1 \ + || fail "fm-brief.sh firstmate scaffold with --pr-repo exited non-zero" + brief="$home/data/firstmate-pr-target/brief.md" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_grep 'PR target is `brchue-ux/firstmate` on `main`' "$brief" \ + "firstmate no-mistakes brief did not name its explicit PR target" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_grep '`--repo brchue-ux/firstmate --base main`' "$brief" \ + "firstmate no-mistakes brief did not instruct the explicit flag for a manual gh-axi fallback" + + # Omitting --pr-repo/--pr-base must fail loudly rather than scaffold a brief + # with a bare, mis-targetable `gh pr create`. + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" herdr-no-target herdr >/dev/null 2>&1; status=$? + expect_code 1 "$status" "a ship brief that opens a PR without --pr-repo/--pr-base must fail" + assert_absent "$home/data/herdr-no-target/brief.md" \ + "loud-failure ship brief still wrote a file" + + # local-only never opens a PR, so it needs no explicit target. + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" local-no-target local-proj >/dev/null 2>&1; status=$? + expect_code 0 "$status" "local-only brief must not require --pr-repo/--pr-base" + + pass "fm-brief.sh: ship briefs that open a PR always name an explicit target repo and base branch" +} + test_script_parses test_no_heredoc_in_command_substitution test_help_includes_entire_header @@ -635,3 +725,5 @@ test_secondmate_directory_paths_are_absolute_and_output_is_stable test_pause_verb_override_renders_all_brief_scaffolds test_scout_and_secondmate_load_decision_hold_policy test_scout_and_secondmate_scaffold +test_herdr_repo_bakes_in_fork_base_check +test_ship_pr_target_is_explicit diff --git a/tests/fm-tangle-guard.test.sh b/tests/fm-tangle-guard.test.sh index 4df2706121..e2eaa9313c 100755 --- a/tests/fm-tangle-guard.test.sh +++ b/tests/fm-tangle-guard.test.sh @@ -127,7 +127,7 @@ test_brief_assertion_precedes_branch() { local home brief iso br home="$TMP_ROOT/brief-home" mkdir -p "$home/data" - FM_HOME="$home" "$ROOT/bin/fm-brief.sh" tangle-brief-cc3 alpha >/dev/null 2>&1 + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" tangle-brief-cc3 alpha --pr-repo owner/alpha --pr-base main >/dev/null 2>&1 brief="$home/data/tangle-brief-cc3/brief.md" assert_present "$brief" "brief was not scaffolded" assert_grep "blocked: launched in primary checkout, not an isolated worktree" "$brief" \