From 126330f4820ed1f010847a002f154807482a8dba Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Sun, 26 Jul 2026 01:25:20 -0400 Subject: [PATCH 01/13] Harden recurring fleet failure guardrails --- .agents/skills/harness-adapters/SKILL.md | 1 + bin/fm-account-directory.sh | 127 ++++++++++++++++++++++- bin/fm-brief.sh | 17 ++- bin/fm-promote.sh | 5 +- bin/fm-report-contract-lib.sh | 12 ++- bin/fm-report-stack.mjs | 64 ++++++++++-- bin/fm-send.sh | 74 ++++++++++++- docs/configuration.md | 3 + docs/report-stack.md | 4 +- tests/fm-account-directory.test.sh | 122 +++++++++++++++++++++- tests/fm-brief.test.sh | 46 ++++++-- tests/fm-report-stack.test.sh | 66 ++++++++++++ tests/fm-send-popup-settle.test.sh | 39 ++++++- 13 files changed, 546 insertions(+), 34 deletions(-) diff --git a/.agents/skills/harness-adapters/SKILL.md b/.agents/skills/harness-adapters/SKILL.md index 86dbfd2ee5..8dae80da56 100644 --- a/.agents/skills/harness-adapters/SKILL.md +++ b/.agents/skills/harness-adapters/SKILL.md @@ -105,6 +105,7 @@ This preserves launch success instead of passing a known-bad value. Send the validation skill using the target harness's skill invocation form. Natural language is acceptable if uncertain. +`fm-send` refuses a bare installed-skill token whose prefix conflicts with the resolved harness, and names the correct form instead of silently rewriting a caller bug. - claude: `/`, for example `/no-mistakes`. - codex: `$`, for example `$no-mistakes`; `/` is claude-only and codex rejects it as "Unrecognized command". diff --git a/bin/fm-account-directory.sh b/bin/fm-account-directory.sh index 3ba66b5462..72b9732831 100755 --- a/bin/fm-account-directory.sh +++ b/bin/fm-account-directory.sh @@ -2,6 +2,7 @@ # Select and prepare direct Claude or Codex account-directory launches. # Usage: # fm-account-directory.sh select +# fm-account-directory.sh check-credential # fm-account-directory.sh install-herdr-hook # fm-account-directory.sh prepare # @@ -21,8 +22,15 @@ # selects the first real account directory in stable bytewise sort order. # Selection prints only the chosen absolute account home on stdout and logs # health, fallback, and choice diagnostics on stderr. -# prepare selects the account and idempotently runs Herdr's own integration -# installer with CODEX_HOME or CLAUDE_CONFIG_DIR set to the chosen home. +# prepare selects the account, verifies a usable credential before endpoint +# creation can begin, and idempotently runs Herdr's own integration installer +# with CODEX_HOME or CLAUDE_CONFIG_DIR set to the chosen home. +# Codex uses a cheap read-only auth.json credential-material check. +# Claude has no sufficient on-disk credential marker on macOS, so it uses the +# CLI's local `auth status --json` check, bounded to two seconds and never making +# a model call. +# A failed check names the selected account home and prints the exact scoped +# provider login command a human can run. # It verifies the installed per-profile hook before printing the chosen home. # # Credential state is read-only. @@ -34,7 +42,7 @@ set -u TEST_LAB_TOKEN=firstmate-account-directory-test-lab-v1 usage() { - sed -n '2,32p' "$0" | sed 's/^# \{0,1\}//' >&2 + sed -n '2,40p' "$0" | sed 's/^# \{0,1\}//' >&2 } log() { @@ -45,6 +53,12 @@ test_lab_enabled() { [ "${FM_ACCOUNT_DIRECTORY_TEST_LAB:-}" = "$TEST_LAB_TOKEN" ] } +shell_quote() { + printf "'" + printf '%s' "$1" | sed "s/'/'\\\\''/g" + printf "'" +} + system_perl() { if test_lab_enabled && [ -n "${FM_ACCOUNT_DIRECTORY_PERL_BIN:-}" ]; then printf '%s\n' "$FM_ACCOUNT_DIRECTORY_PERL_BIN" @@ -203,6 +217,108 @@ valid_account_home() { # esac } +provider_binary() { # + local vendor=$1 account_home=$2 override manifest binary install_path home perl_bin + if test_lab_enabled; then + case "$vendor" in + claude) override=${FM_ACCOUNT_DIRECTORY_CLAUDE_BIN:-} ;; + codex) override=${FM_ACCOUNT_DIRECTORY_CODEX_BIN:-} ;; + *) return 1 ;; + esac + if [ -n "$override" ]; then + [ -f "$override" ] && [ ! -L "$override" ] && [ -x "$override" ] || { + echo "error: test $vendor provider binary is not a real executable: $override" >&2 + return 1 + } + printf '%s\n' "$override" + return 0 + fi + fi + manifest=$account_home/.agent-fleet-provider-binary.json + if [ -f "$manifest" ] && [ ! -L "$manifest" ]; then + binary=$(jq -er '.binary.resolved_path | select(type == "string" and startswith("/"))' "$manifest" 2>/dev/null) || binary= + if [ -n "$binary" ] && [ -f "$binary" ] && [ ! -L "$binary" ] && [ -x "$binary" ]; then + printf '%s\n' "$binary" + return 0 + fi + fi + home=$(passwd_home) || return 1 + install_path=$home/.local/bin/$vendor + perl_bin=$(system_perl) || return 1 + # shellcheck disable=SC2016 # Perl source is intentionally single-quoted. + binary=$("$perl_bin" -MCwd=realpath -e ' + my $path = realpath($ARGV[0]); + exit 1 unless defined $path && $path =~ m{^/} && $path !~ /[\x00-\x1f\x7f]/; + print $path; + ' "$install_path" 2>/dev/null) || binary= + if [ -n "$binary" ] && [ -f "$binary" ] && [ ! -L "$binary" ] && [ -x "$binary" ]; then + printf '%s\n' "$binary" + return 0 + fi + echo "error: cannot resolve the $vendor provider binary for account credential verification at $account_home" >&2 + return 1 +} + +credential_login_command() { # [provider-binary] + local vendor=$1 account_home=$2 provider_bin=${3:-$1} + case "$vendor" in + codex) + printf 'CODEX_HOME=%s %s login' "$(shell_quote "$account_home")" "$(shell_quote "$provider_bin")" + ;; + claude) + printf 'CLAUDE_CONFIG_DIR=%s %s auth login' "$(shell_quote "$account_home")" "$(shell_quote "$provider_bin")" + ;; + esac +} + +check_codex_credential() { # + local account_home=$1 credential provider_bin login + credential=$account_home/auth.json + provider_bin=$(provider_binary codex "$account_home" 2>/dev/null || printf 'codex') + login=$(credential_login_command codex "$account_home" "$provider_bin") + if [ ! -f "$credential" ] || [ -L "$credential" ] || ! jq -e ' + ((.tokens.access_token? | type) == "string" and (.tokens.access_token | length) > 0 + and (.tokens.refresh_token? | type) == "string" and (.tokens.refresh_token | length) > 0) + or ((.OPENAI_API_KEY? | type) == "string" and (.OPENAI_API_KEY | length) > 0) + ' "$credential" >/dev/null 2>&1; then + echo "error: selected codex account directory '$account_home' has no usable on-disk credential; run: $login" >&2 + return 1 + fi +} + +check_claude_credential() { # + local account_home=$1 provider_bin status_json login + provider_bin=$(provider_binary claude "$account_home") || return 1 + login=$(credential_login_command claude "$account_home" "$provider_bin") + status_json=$(run_bounded 2 /usr/bin/env CLAUDE_CONFIG_DIR="$account_home" \ + "$provider_bin" auth status --json 2>/dev/null) || status_json= + if ! jq -e '.loggedIn == true' >/dev/null 2>&1 <&2 + return 1 + fi +} + +check_credential() { # + local vendor=$1 account_home=$2 root vendor_dir + root=$(account_root) || return 1 + vendor_dir=$root/$vendor + valid_account_home "$vendor_dir" "$account_home" || { + echo "error: unsafe $vendor account home for credential verification: $account_home" >&2 + return 1 + } + case "$vendor" in + codex) check_codex_credential "$account_home" ;; + claude) check_claude_credential "$account_home" ;; + *) + echo "error: account credential verification supports only claude or codex, not '$vendor'" >&2 + return 1 + ;; + esac +} + first_account_home() { # local vendor=$1 root vendor_dir candidate root=$(account_root) || return 1 @@ -381,6 +497,10 @@ case "${1:-}" in [ "$#" -eq 2 ] || { usage; exit 2; } select_account "$2" ;; + check-credential) + [ "$#" -eq 3 ] || { usage; exit 2; } + check_credential "$2" "$3" + ;; install-herdr-hook) [ "$#" -eq 3 ] || { usage; exit 2; } install_herdr_hook "$2" "$3" @@ -388,6 +508,7 @@ case "${1:-}" in prepare) [ "$#" -eq 2 ] || { usage; exit 2; } selected_home=$(select_account "$2") || exit 1 + check_credential "$2" "$selected_home" || exit 1 install_herdr_hook "$2" "$selected_home" || exit 1 printf '%s\n' "$selected_home" ;; diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index eb0ba38faf..e443e5468f 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -223,7 +223,18 @@ EOF ) fi +BROWSER_SECTION=$(cat <<'EOF' +# Browser automation safety +Never set `CHROME_DEVTOOLS_AXI_AUTO_CONNECT=1` or `CHROME_DEVTOOLS_AXI_HEADED=1`. +Always set your own `CHROME_DEVTOOLS_AXI_SESSION`. +When an authenticated dashboard is needed, set `CHROME_DEVTOOLS_AXI_USER_DATA_DIR` to a per-crewmate profile directory; the persistent profile keeps its login cookie across headless runs. +Never use `open -a "Google Chrome"` or AppleScript that focuses an app. +If a check genuinely cannot run headlessly, report the exact step that fails instead of falling back to a visible or auto-connected browser. +EOF +) + if [ "$KIND" = scout ]; then +REPORT_HEADINGS=$(fm_completion_report_required_headings) cat > "$BRIEF" < - local data=$1 task=$2 + local data=$1 task=$2 headings + headings=$(fm_completion_report_required_headings) printf '%s\n' \ '# Completion report' \ - "Before the final \`done:\` status, write \`$data/$task/completion.md\` with these six sections, each as a LEVEL-TWO markdown heading spelled exactly: \`## Summary\`, \`## What changed\`, \`## Verification\`, \`## Visual evidence\`, \`## Artifacts\`, \`## Follow-ups\`." \ - "Publication rejects the report if any of those headings is missing, spelled differently, or written at another level - a level-one \`# Summary\` fails. Each section needs substantive prose directly under it; if you use sub-headings inside a section, make them \`###\` so they nest rather than ending the section." \ + "Before the final \`done:\` status, write \`$data/$task/completion.md\` with these six exact level-two headings in this order: $headings." \ + "Keep all six at level two; publication also accepts a report that consistently uses the level-one equivalents as its section structure, but do not mix heading levels. Each section needs substantive prose directly under it; if you use sub-headings inside a section, make them \`###\` so they nest rather than ending the section." \ 'When a section genuinely does not apply, say so in a sentence under the heading rather than omitting the heading.' \ 'Make it stand alone for the captain: explain the outcome, name important files or links, record the validation performed, and call out remaining risk or decisions.' \ "Put screenshots, diagrams, or other visual artifacts under \`$data/$task/visuals/\` and reference them from the report when they materially help review." \ diff --git a/bin/fm-report-stack.mjs b/bin/fm-report-stack.mjs index de47121baa..accf86a775 100755 --- a/bin/fm-report-stack.mjs +++ b/bin/fm-report-stack.mjs @@ -123,12 +123,47 @@ function lastStatus(status) { const { markdownStructure } = markdownModule; +function completionSectionStructure(markdown, options = {}) { + const structure = markdownStructure(markdown, options); + const requiredIndex = new Map(requiredSections.map((section, index) => [section.toLowerCase(), index])); + const headings = structure + .map((entry, index) => ({ entry, index })) + .filter(({ entry }) => entry.heading?.level <= 2 && requiredIndex.has(entry.heading.content.toLowerCase())); + const level = headings[0]?.entry.heading.level; + const mixedLevels = headings.some(({ entry }) => entry.heading.level !== level); + let lastRequiredIndex = -1; + let outOfOrder = false; + for (const { entry } of headings) { + const index = requiredIndex.get(entry.heading.content.toLowerCase()); + if (index <= lastRequiredIndex) outOfOrder = true; + lastRequiredIndex = index; + } + const firstHeadingIndex = headings[0]?.index; + const lastHeadingIndex = headings[headings.length - 1]?.index; + const interrupted = level === 2 && structure.some(({ heading }, index) => ( + index > firstHeadingIndex + && index < lastHeadingIndex + && heading?.level === 1 + )); + return { + structure, + level, + mixedLevels, + outOfOrder, + interrupted, + }; +} + function firstSummary(markdown, fallback) { - const structure = markdownStructure(markdown); - const summaryStart = structure.findIndex(({ heading }) => heading?.level === 2 && heading.content.toLowerCase() === "summary"); + const { structure, level } = completionSectionStructure(markdown); + const summaryStart = structure.findIndex(({ heading }) => ( + heading && heading.level === level && heading.content.toLowerCase() === "summary" + )); let summaryLines; if (summaryStart >= 0) { - const followingHeading = structure.findIndex(({ heading }, index) => index > summaryStart && heading?.level === 2); + const followingHeading = structure.findIndex(({ heading }, index) => ( + index > summaryStart && heading?.level <= level + )); summaryLines = structure.slice(summaryStart + 1, followingHeading < 0 ? undefined : followingHeading).map(({ line }) => line); } else { summaryLines = structure.map(({ line }) => line); @@ -145,10 +180,19 @@ function firstSummary(markdown, fallback) { function requireCompletionSections(markdown, sourceFile, taskId) { const sections = new Map(requiredSections.map((section) => [section.toLowerCase(), { present: false, body: [] }])); + const { + structure, + level, + mixedLevels, + outOfOrder, + interrupted, + } = completionSectionStructure(markdown, { includeFenceContent: true }); let currentSection; - for (const entry of markdownStructure(markdown, { includeFenceContent: true })) { - if (entry.heading?.level === 2) { - currentSection = sections.get(entry.heading.content.toLowerCase()); + for (const entry of structure) { + if (entry.heading?.level <= level) { + currentSection = entry.heading.level === level + ? sections.get(entry.heading.content.toLowerCase()) + : undefined; if (currentSection) currentSection.present = true; } else if (currentSection) { currentSection.body.push(entry); @@ -169,16 +213,20 @@ function requireCompletionSections(markdown, sourceFile, taskId) { const state = sections.get(section.toLowerCase()); return state.present && !state.body.some(substantive); }); - if (missing.length === 0 && empty.length === 0) return; + if (missing.length === 0 && empty.length === 0 && !mixedLevels && !outOfOrder && !interrupted) return; const required = requiredSections.map((section) => `## ${section}`).join(", "); const absent = missing.map((section) => `## ${section}`).join(", "); const blank = empty.map((section) => `## ${section}`).join(", "); const problems = []; if (missing.length > 0) problems.push(`missing required section headings: ${absent}`); if (empty.length > 0) problems.push(`required sections have no substantive content: ${blank}`); + if (mixedLevels || interrupted) { + problems.push("required section headings do not share one top structural level (# or ##)"); + } + if (outOfOrder) problems.push("required section headings are out of order"); throw new Error( `completion report at ${sourceFile} ${problems.join("; ")}. ` - + `Update ${sourceFile} to include these level-two headings with substantive content: ${required}. ` + + `Update ${sourceFile} to include these headings at one common top structural level, in order, with substantive content (level two recommended): ${required}. ` + `Then rerun ${fmRoot}/bin/fm-report-stack.mjs publish ${taskId} or ${fmRoot}/bin/fm-teardown.sh ${taskId}. ` + "This attempt did not replace the durable report, and teardown remains stopped before destructive cleanup.", ); diff --git a/bin/fm-send.sh b/bin/fm-send.sh index 7ad920b052..fef80bf353 100755 --- a/bin/fm-send.sh +++ b/bin/fm-send.sh @@ -20,6 +20,12 @@ # Tune with FM_SEND_RETRIES (default 3) / FM_SEND_SLEEP (0.4). # Slash commands, and codex `$...` skill invocations resolved through harness # meta, get a longer pre-Enter settle so completion popups do not swallow Enter. +# Before that shared settle path, a bare installed-skill token is refused when +# its `/` or `$` prefix conflicts with the resolved harness form recorded in +# .agents/skills/harness-adapters/SKILL.md. Refusal is deliberate: silently +# rewriting would hide a supervisor caller bug. Ordinary slash commands, +# `$`-prefixed prose, multi-token messages, keys, and unknown-harness explicit +# targets keep their existing behavior. # # From-firstmate marker: when the resolved target is a task selector whose meta # records kind=secondmate, the text is prefixed with the from-firstmate marker @@ -95,6 +101,60 @@ fm_send_count_colons() { # printf '%s' $(( ${#s} - ${#no_colons} )) } +fm_send_bare_skill_name() { # + local message=$1 name root + case "$message" in + /*|\$*) ;; + *) return 1 ;; + esac + case "$message" in + *[[:space:]]*) return 1 ;; + esac + name=${message:1} + case "$name" in + ''|*[!A-Za-z0-9._-]*) return 1 ;; + esac + case "$name" in + no-mistakes) + printf '%s' "$name" + return 0 + ;; + esac + for root in \ + "$FM_HOME/.agents/skills" \ + "$FM_ROOT/.agents/skills" \ + "$FM_ROOT/skills" \ + "${HOME:-}/.agents/skills" \ + "${HOME:-}/.claude/skills" \ + "${HOME:-}/.codex/skills"; do + [ -n "$root" ] || continue + [ -d "$root/$name" ] || continue + printf '%s' "$name" + return 0 + done + return 1 +} + +fm_send_validate_skill_form() { # + local harness=$1 message=$2 skill expected corrected + skill=$(fm_send_bare_skill_name "$message") || return 0 + case "$harness" in + claude|grok) expected=/ ;; + codex) expected='$' ;; + opencode|pi) + echo "error: refusing bare skill invocation '$message' for harness '$harness'; it has no verified bare skill form, so send natural language such as 'Run the $skill skill.'" >&2 + return 1 + ;; + *) return 0 ;; + esac + case "$message" in + "$expected"*) return 0 ;; + esac + corrected=$expected$skill + echo "error: refusing bare skill invocation '$message' for harness '$harness'; use '$corrected'" >&2 + return 1 +} + fm_send_resolve_target() { # local raw=$1 meta pane_meta target backend assumed colons id session hint @@ -188,6 +248,11 @@ shift fm_backend_validate "$TARGET_BACKEND" || exit 1 +if [ "${1:-}" != "--key" ]; then + MESSAGE=$* + fm_send_validate_skill_form "$TARGET_HARNESS" "$MESSAGE" || exit 1 +fi + # Classify a from-firstmate -> secondmate request. Only a task selector resolved # through this home's meta whose authoritative kind is secondmate is marked: the # secondmate then routes its reply via the status path (see fm-marker-lib.sh). @@ -198,10 +263,10 @@ if [ -n "$TARGET_SELECTOR" ] && [ -n "$TARGET_META" ] && [ "$(fm_meta_get "$TARG MARK_FROM_FIRSTMATE=1 fi -# Resolve the target's harness from its meta (recorded by fm-spawn), used only to -# scope the codex `$` popup-settle below. A task selector carries -# meta; an explicit backend-target escape hatch has none, so its harness is -# unknown and treated as non-codex (the safe default that keeps the fast path). +# The target's harness came from its meta (recorded by fm-spawn), and scopes both +# the bare-skill form guard above and the codex `$` popup-settle below. +# A task selector carries meta; an explicit backend-target escape hatch has none, +# so its harness is unknown and retains the existing unguarded fast path. # The target's BACKEND comes from selector meta, from matching an explicit target # back to recorded meta, or from strict explicit-target shape validation. # Do not add a separate passive liveness preflight here. Active send paths own @@ -257,7 +322,6 @@ if [ "${1:-}" = "--key" ]; then fi fi else - MESSAGE=$* if [ "$MARK_FROM_FIRSTMATE" = 1 ]; then fm_message_mark_from_firstmate "$MESSAGE" MESSAGE fi diff --git a/docs/configuration.md b/docs/configuration.md index 34a14ae896..47496a4280 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -173,6 +173,9 @@ The selected provider command receives `CLAUDE_CONFIG_DIR=` or `CODEX_HOME New ship/scout launches never ask Agent Fleet to enable a profile, establish identity, install a bundle, or acquire a lease. They invoke Herdr's own integration installer against the selected profile directory and verify its per-profile hook file before launching. Account credentials remain captain-owned and read-only to Firstmate; selection never authenticates, logs in, or invokes a model. +Before installing that hook or creating an endpoint, spawn preflight requires usable credential material in the selected directory. +Codex uses a cheap on-disk credential check, while Claude uses its bounded local `auth status` command because cached account metadata is not a credential and its macOS Keychain credential has no sufficient on-disk marker. +Failure names the selected directory and prints the exact provider-scoped login command for a human. Codex health and usage are genuinely readable per account. Every selection performs a fresh per-account quota read instead of trusting a prior cache, and a Codex directory with no fresh general usage window is skipped. diff --git a/docs/report-stack.md b/docs/report-stack.md index 947f486422..f9df53041b 100644 --- a/docs/report-stack.md +++ b/docs/report-stack.md @@ -19,7 +19,9 @@ Expired entries are renamed to deletion tombstones before the index changes, and New task metadata carries `report_required=1`. A ship task writes `data//completion.md`, while a scout keeps using `data//report.md`. Both may attach screenshots, diagrams, or other review artifacts under `data//visuals/`. -Every post-cutover ship and scout report must use the level-two sections Summary, What changed, Verification, Visual evidence, Artifacts, and Follow-ups. +Every post-cutover ship and scout report must use the sections Summary, What changed, Verification, Visual evidence, Artifacts, and Follow-ups in that order. +All six section headings must use one common top structural level: either level one, or level two beneath an optional level-one report title. +The brief scaffold requests the level-two form so reports are consistent, while publication accepts the level-one form because heading depth is formatting rather than substance. Every required section must contain substantive body content, with an explicit `None.` accepted when the section has nothing to report. Within a real required section, meaningful fenced transcript or literal-code lines count as body content, but an empty fence body, whitespace, a bare Markdown blockquote or list marker, or Unicode control and format characters alone do not. Fence delimiters never count as body content, and heading-like lines inside a fence never satisfy a required section heading. diff --git a/tests/fm-account-directory.test.sh b/tests/fm-account-directory.test.sh index 3d6db27dba..0fab14da81 100755 --- a/tests/fm-account-directory.test.sh +++ b/tests/fm-account-directory.test.sh @@ -11,6 +11,7 @@ ACCOUNT_ROOT="$TMP_ROOT/accounts" FAKEBIN=$(fm_fakebin "$TMP_ROOT") QUOTA_LOG="$TMP_ROOT/quota.log" HERDR_LOG="$TMP_ROOT/herdr.log" +CLAUDE_AUTH_LOG="$TMP_ROOT/claude-auth.log" TREEHOUSE_LOG="$TMP_ROOT/treehouse.log" mkdir -p "$ACCOUNT_ROOT/codex" "$ACCOUNT_ROOT/claude" @@ -79,12 +80,29 @@ fi SH chmod +x "$FAKEBIN/herdr" +cat > "$FAKEBIN/claude" <<'SH' +#!/usr/bin/env bash +set -u +[ "${1:-}" = auth ] && [ "${2:-}" = status ] && [ "${3:-}" = --json ] || exit 64 +[ -n "${CLAUDE_CONFIG_DIR:-}" ] || exit 65 +printf '%s\n' "$CLAUDE_CONFIG_DIR" >> "$FM_FAKE_CLAUDE_AUTH_LOG" +if [ -f "$CLAUDE_CONFIG_DIR/test-authenticated" ]; then + printf '%s\n' '{"loggedIn":true,"authMethod":"claude.ai"}' + exit 0 +fi +printf '%s\n' '{"loggedIn":false,"authMethod":"none"}' +exit 1 +SH +chmod +x "$FAKEBIN/claude" + run_selector() { FM_ACCOUNT_DIRECTORY_TEST_LAB=firstmate-account-directory-test-lab-v1 \ FM_ACCOUNT_DIRECTORY_ROOT="$ACCOUNT_ROOT" \ FM_ACCOUNT_DIRECTORY_QUOTA_AXI="$FAKEBIN/quota-axi" \ FM_ACCOUNT_DIRECTORY_HERDR="$FAKEBIN/herdr" \ + FM_ACCOUNT_DIRECTORY_CLAUDE_BIN="$FAKEBIN/claude" \ FM_FAKE_QUOTA_LOG="$QUOTA_LOG" FM_FAKE_HERDR_LOG="$HERDR_LOG" \ + FM_FAKE_CLAUDE_AUTH_LOG="$CLAUDE_AUTH_LOG" \ "$SELECTOR" "$@" } @@ -93,6 +111,14 @@ set_remaining() { mkdir -p "$ACCOUNT_ROOT/codex/$account/.agent-fleet-quota-cache/quota-axi" printf '%s\n' "$remaining" > "$ACCOUNT_ROOT/codex/$account/test-remaining" printf '{"stale":true}\n' > "$ACCOUNT_ROOT/codex/$account/.agent-fleet-quota-cache/quota-axi/quotas.json" + printf '{"auth_mode":"chatgpt","tokens":{"access_token":"test-access","refresh_token":"test-refresh"}}\n' \ + > "$ACCOUNT_ROOT/codex/$account/auth.json" +} + +set_claude_authenticated() { + local account=$1 + mkdir -p "$ACCOUNT_ROOT/claude/$account" + : > "$ACCOUNT_ROOT/claude/$account/test-authenticated" } reset_accounts() { @@ -100,6 +126,7 @@ reset_accounts() { mkdir -p "$ACCOUNT_ROOT/codex" "$ACCOUNT_ROOT/claude" : > "$QUOTA_LOG" : > "$HERDR_LOG" + : > "$CLAUDE_AUTH_LOG" } test_codex_picks_highest_fresh_minimum_and_skips_no_window() { @@ -191,11 +218,33 @@ test_default_root_uses_passwd_home_not_ambient_home() { pass "default account discovery ignores ambient HOME and stays under the passwd home" } +test_claude_credential_check_recovers_from_stale_profile_binary_manifest() { + local account_home passwd_home + reset_accounts + account_home="$ACCOUNT_ROOT/claude/1" + passwd_home="$TMP_ROOT/credential-passwd-home" + set_claude_authenticated 1 + mkdir -p "$passwd_home/.local/bin" + ln -s "$FAKEBIN/claude" "$passwd_home/.local/bin/claude" + printf '{"binary":{"resolved_path":"/missing/claude"}}\n' \ + > "$account_home/.agent-fleet-provider-binary.json" + + FM_ACCOUNT_DIRECTORY_TEST_LAB=firstmate-account-directory-test-lab-v1 \ + FM_ACCOUNT_DIRECTORY_ROOT="$ACCOUNT_ROOT" \ + FM_ACCOUNT_DIRECTORY_PASSWD_HOME="$passwd_home" \ + FM_FAKE_CLAUDE_AUTH_LOG="$CLAUDE_AUTH_LOG" \ + "$SELECTOR" check-credential claude "$account_home" \ + || fail "Claude credential check did not recover from a stale pinned binary" + assert_grep "$account_home" "$CLAUDE_AUTH_LOG" \ + "Claude credential check did not scope the current provider binary to the selected account" + pass "Claude credential preflight resolves the current install when profile binary metadata is stale" +} + test_prepare_installs_and_verifies_per_account_herdr_hooks() { local codex_home claude_home reset_accounts set_remaining 1 90,80 - mkdir -p "$ACCOUNT_ROOT/claude/1" + set_claude_authenticated 1 codex_home=$(run_selector prepare codex 2>"$TMP_ROOT/prepare-codex.err") claude_home=$(run_selector prepare claude 2>"$TMP_ROOT/prepare-claude.err") @@ -206,6 +255,40 @@ test_prepare_installs_and_verifies_per_account_herdr_hooks() { pass "prepare uses Herdr's own installer and verifies each selected profile hook" } +test_prepare_rejects_missing_credentials_before_hook_install() { + local out status codex_home claude_home + + reset_accounts + set_remaining 1 90,80 + codex_home="$ACCOUNT_ROOT/codex/1" + rm -f "$codex_home/auth.json" + out=$(run_selector prepare codex 2>&1) + status=$? + expect_code 1 "$status" "Codex prepare without auth.json should fail closed" + assert_contains "$out" "selected codex account directory '$codex_home' has no usable on-disk credential" \ + "Codex credential refusal omitted the selected account directory" + assert_contains "$out" "CODEX_HOME='$codex_home'" \ + "Codex credential refusal omitted the exact scoped login command" + [ ! -s "$HERDR_LOG" ] || fail "Codex credential refusal still installed a Herdr hook" + + reset_accounts + claude_home="$ACCOUNT_ROOT/claude/1" + mkdir -p "$claude_home" + out=$(run_selector prepare claude 2>&1) + status=$? + expect_code 1 "$status" "Claude prepare without a usable login should fail closed" + assert_contains "$out" "selected claude account directory '$claude_home' has no usable credential" \ + "Claude credential refusal omitted the selected account directory" + assert_contains "$out" "CLAUDE_CONFIG_DIR='$claude_home'" \ + "Claude credential refusal omitted the exact scoped login command" + assert_contains "$out" "auth login" \ + "Claude credential refusal omitted the provider login command" + assert_grep "$claude_home" "$CLAUDE_AUTH_LOG" \ + "Claude credential preflight did not query the selected config directory" + [ ! -s "$HERDR_LOG" ] || fail "Claude credential refusal still installed a Herdr hook" + pass "prepare refuses unauthenticated account directories before hook installation" +} + make_spawn_fakebin() { local fakebin=$1 cat > "$fakebin/tmux" <<'SH' @@ -299,7 +382,9 @@ run_direct_spawn() { FM_ACCOUNT_DIRECTORY_ROOT="$ACCOUNT_ROOT" \ FM_ACCOUNT_DIRECTORY_QUOTA_AXI="$FAKEBIN/quota-axi" \ FM_ACCOUNT_DIRECTORY_HERDR="$FAKEBIN/herdr" \ + FM_ACCOUNT_DIRECTORY_CLAUDE_BIN="$FAKEBIN/claude" \ FM_FAKE_QUOTA_LOG="$QUOTA_LOG" FM_FAKE_HERDR_LOG="$HERDR_LOG" \ + FM_FAKE_CLAUDE_AUTH_LOG="$CLAUDE_AUTH_LOG" \ FM_AGENT_FLEET_BIN="$FAKEBIN/forbidden-agent-fleet" \ FM_FAKE_AGENT_FLEET_LOG="$TMP_ROOT/agent-fleet.log" \ "$ROOT/bin/fm-spawn.sh" "$@" @@ -361,7 +446,8 @@ test_spawn_uses_direct_claude_fallback_and_hook() { local record id out launch meta reset_accounts : > "$TMP_ROOT/agent-fleet.log" - mkdir -p "$ACCOUNT_ROOT/claude/2" "$ACCOUNT_ROOT/claude/1" + mkdir -p "$ACCOUNT_ROOT/claude/2" + set_claude_authenticated 1 id=direct-claude-z2 record=$(make_spawn_case direct-claude claude "$id") read_spawn_case "$record" @@ -379,6 +465,27 @@ test_spawn_uses_direct_claude_fallback_and_hook() { pass "new account-flagged Claude spawn uses deterministic CLAUDE_CONFIG_DIR with an explicit warning" } +test_spawn_refuses_unauthenticated_account_before_endpoint_creation() { + local record id out status + reset_accounts + mkdir -p "$ACCOUNT_ROOT/claude/1" + id=direct-claude-unauth-z2a + record=$(make_spawn_case direct-claude-unauth claude "$id") + read_spawn_case "$record" + + out=$(run_direct_spawn "$SPAWN_HOME" "$SPAWN_WORKTREE" "$SPAWN_LAUNCH_LOG" \ + "$id" "$SPAWN_PROJECT" --account-pool legacy-claude-pool 2>&1) + status=$? + expect_code 1 "$status" "unauthenticated direct Claude spawn should fail closed" + assert_contains "$out" "selected claude account directory '$ACCOUNT_ROOT/claude/1' has no usable credential" \ + "spawn credential refusal omitted the selected account directory" + assert_contains "$out" "auth login" "spawn credential refusal omitted the exact login command" + assert_absent "$SPAWN_HOME/state/.fake-endpoint" \ + "unauthenticated direct Claude spawn created an endpoint" + [ ! -s "$SPAWN_LAUNCH_LOG" ] || fail "unauthenticated direct Claude spawn typed a launch command" + pass "fm-spawn refuses an unauthenticated direct account before endpoint creation" +} + test_observe_spawn_uses_direct_directory_without_agent_fleet() { local record id out launch meta reset_accounts @@ -954,15 +1061,26 @@ if [ "${FM_TEST_FOCUSED:-}" = direct-recovery-lifecycle ]; then exit 0 fi +if [ "${FM_TEST_FOCUSED:-}" = credential-preflight ]; then + test_claude_credential_check_recovers_from_stale_profile_binary_manifest + test_prepare_installs_and_verifies_per_account_herdr_hooks + test_prepare_rejects_missing_credentials_before_hook_install + test_spawn_refuses_unauthenticated_account_before_endpoint_creation + exit 0 +fi + test_codex_picks_highest_fresh_minimum_and_skips_no_window test_codex_rechecks_health_on_every_selection test_codex_fails_when_no_account_has_a_fresh_window test_codex_timeout_skips_wedged_account test_claude_uses_stable_first_without_treating_usage_as_health test_default_root_uses_passwd_home_not_ambient_home +test_claude_credential_check_recovers_from_stale_profile_binary_manifest test_prepare_installs_and_verifies_per_account_herdr_hooks +test_prepare_rejects_missing_credentials_before_hook_install test_spawn_uses_direct_codex_home_without_agent_fleet test_spawn_uses_direct_claude_fallback_and_hook +test_spawn_refuses_unauthenticated_account_before_endpoint_creation test_observe_spawn_uses_direct_directory_without_agent_fleet test_direct_spawn_and_recovery_support_detached_worktree test_direct_recovery_preserves_recorded_task_context diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index a5ce892ffb..a87e540975 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -109,12 +109,14 @@ test_ship_completion_report_contract() { brief="$home/data/$id/brief.md" assert_grep "# Completion report" "$brief" "ship brief has no completion-report contract" assert_grep "data/$id/completion.md" "$brief" "ship brief has no durable report path" - assert_grep "six sections, each as a LEVEL-TWO markdown heading spelled exactly" "$brief" \ - "ship brief does not require exact level-two report headings" + assert_grep "six exact level-two headings in this order" "$brief" \ + "ship brief does not require ordered exact level-two report headings" assert_grep "\`## Summary\`, \`## What changed\`, \`## Verification\`, \`## Visual evidence\`, \`## Artifacts\`, \`## Follow-ups\`" "$brief" \ "ship brief has no exact review-oriented report schema" - assert_grep "a level-one \`# Summary\` fails" "$brief" \ - "ship brief does not reject completion sections at the wrong heading level" + assert_grep "publication also accepts a report that consistently uses the level-one equivalents" "$brief" \ + "ship brief does not describe the accepted level-one compatibility form" + assert_grep "do not mix heading levels" "$brief" \ + "ship brief does not forbid structurally mixed report sections" assert_grep "When a section genuinely does not apply, say so in a sentence under the heading rather than omitting the heading." "$brief" \ "ship brief permits inapplicable report sections to be omitted" assert_grep "data/$id/visuals/" "$brief" "ship brief has no visual evidence path" @@ -132,11 +134,40 @@ test_scout_completion_report_contract() { FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --scout >/dev/null 2>&1 brief="$home/data/$id/brief.md" assert_grep "data/$id/report.md" "$brief" "scout brief has no durable report path" - assert_grep "Summary, What changed, Verification, Visual evidence, Artifacts, and Follow-ups" "$brief" \ - "scout brief does not name every enforced report section" + assert_grep "\`## Summary\`, \`## What changed\`, \`## Verification\`, \`## Visual evidence\`, \`## Artifacts\`, \`## Follow-ups\`" "$brief" \ + "scout brief does not name every enforced report heading concretely" pass "fm-brief.sh: scout tasks receive the enforced completion-report schema" } +test_browser_automation_safety_contract() { + local home kind id brief + home="$TMP_ROOT/browser-safety-home" + mkdir -p "$home/data" + for kind in ship scout; do + id="brief-browser-safety-$kind" + if [ "$kind" = scout ]; then + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --scout >/dev/null 2>&1 + else + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj >/dev/null 2>&1 + fi + brief="$home/data/$id/brief.md" + assert_grep "# Browser automation safety" "$brief" "$kind brief omitted browser safety" + assert_grep "Never set \`CHROME_DEVTOOLS_AXI_AUTO_CONNECT=1\` or \`CHROME_DEVTOOLS_AXI_HEADED=1\`." "$brief" \ + "$kind brief omitted both independent visible-browser hazards" + assert_grep "Always set your own \`CHROME_DEVTOOLS_AXI_SESSION\`." "$brief" \ + "$kind brief omitted per-crewmate browser session isolation" + assert_grep "\`CHROME_DEVTOOLS_AXI_USER_DATA_DIR\` to a per-crewmate profile directory" "$brief" \ + "$kind brief omitted persistent authenticated-profile isolation" + assert_grep "persistent profile keeps its login cookie across headless runs" "$brief" \ + "$kind brief omitted authenticated headless reuse" + assert_grep "Never use \`open -a \"Google Chrome\"\` or AppleScript that focuses an app." "$brief" \ + "$kind brief omitted focus-hijack commands" + assert_grep "report the exact step that fails instead of falling back" "$brief" \ + "$kind brief omitted fail-closed headless reporting" + done + pass "fm-brief.sh: ship and scout briefs always carry browser-isolation safety" +} + test_promoted_scout_receives_completion_contract() { local home data id out expected_report home="$TMP_ROOT/promote-report-home" @@ -147,7 +178,7 @@ test_promoted_scout_receives_completion_contract() { out=$(FM_HOME="$home" FM_DATA_OVERRIDE="$data" FM_ROOT_OVERRIDE="$ROOT" "$ROOT/bin/fm-promote.sh" "$id") \ || fail "scout promotion failed" assert_grep 'kind=ship' "$home/state/$id.meta" "scout promotion did not update task kind" - assert_contains "$out" "Summary, What changed, Verification, Visual evidence, Artifacts, and Follow-ups" \ + assert_contains "$out" "\`## Summary\`, \`## What changed\`, \`## Verification\`, \`## Visual evidence\`, \`## Artifacts\`, \`## Follow-ups\`" \ "promoted scout did not receive the ship completion-report schema" expected_report="$data/$id/completion.md" assert_contains "$out" "$expected_report" "promoted scout did not receive the authoritative completion-report path" @@ -454,6 +485,7 @@ test_no_mistakes_dod_wording test_ship_project_memory_wording test_ship_completion_report_contract test_scout_completion_report_contract +test_browser_automation_safety_contract test_promoted_scout_receives_completion_contract test_promote_serializes_with_account_session_updates test_promote_locks_lifecycle_before_metadata_and_rechecks_kind diff --git a/tests/fm-report-stack.test.sh b/tests/fm-report-stack.test.sh index 79a5fcf2ee..6301945b82 100755 --- a/tests/fm-report-stack.test.sh +++ b/tests/fm-report-stack.test.sh @@ -704,6 +704,65 @@ test_required_source_fails_closed() { pass "report stack refuses a required ship report with no completion source" } +test_required_sections_accept_one_top_heading_level_in_order() { + local id source entry manifest out status + + id=report-level-one-b2a + write_task "$id" ship + source="$HOME_DIR/data/$id/completion.md" + cat > "$source" <<'EOF' +# Summary + +Level-one sections are accepted. + +# What changed + +Recorded work. + +# Verification + +Evidence checked. + +# Visual evidence + +None. + +# Artifacts + +Report. + +# Follow-ups + +None. +EOF + run_stack publish "$id" >/dev/null || fail "ordered level-one report sections were rejected" + entry=$(run_stack path "$id") + manifest="$(dirname "$entry")/manifest.json" + assert_grep '"summary": "Level one sections are accepted."' "$manifest" \ + "manifest summary did not follow the accepted level-one section structure" + + id=report-mixed-levels-b2b + write_task "$id" ship + source="$HOME_DIR/data/$id/completion.md" + printf '# Summary\n\nComplete.\n\n## What changed\n\nChanged.\n\n## Verification\n\nVerified.\n\n## Visual evidence\n\nNone.\n\n## Artifacts\n\nReport.\n\n## Follow-ups\n\nNone.\n' > "$source" + out=$(run_stack publish "$id" 2>&1) + status=$? + [ "$status" -ne 0 ] || fail "mixed level-one and level-two report sections unexpectedly published" + assert_contains "$out" "do not share one top structural level (# or ##)" \ + "mixed-heading refusal did not explain the common-level requirement" + + id=report-out-of-order-b2c + write_task "$id" ship + source="$HOME_DIR/data/$id/completion.md" + printf '# Summary\n\nComplete.\n\n# Verification\n\nVerified.\n\n# What changed\n\nChanged.\n\n# Visual evidence\n\nNone.\n\n# Artifacts\n\nReport.\n\n# Follow-ups\n\nNone.\n' > "$source" + out=$(run_stack publish "$id" 2>&1) + status=$? + [ "$status" -ne 0 ] || fail "out-of-order level-one report sections unexpectedly published" + assert_contains "$out" "required section headings are out of order" \ + "out-of-order refusal did not explain the ordering requirement" + pass "report stack accepts level one or two while rejecting mixed and out-of-order sections" +} + test_required_sections_fail_actionably() { local id=report-headings-b3 out status before after source heading write_task "$id" ship @@ -4018,6 +4077,12 @@ if [ "${FM_TEST_FOCUSED:-}" = report-fence-enforcement ]; then exit 0 fi +if [ "${FM_TEST_FOCUSED:-}" = report-heading-levels ]; then + test_required_sections_accept_one_top_heading_level_in_order + test_required_sections_reject_empty_bodies + exit 0 +fi + if [ "${FM_TEST_FOCUSED:-}" = fenced-report-body-final ]; then test_fenced_required_section_bodies_use_scoped_content test_nested_short_fences_do_not_satisfy_required_sections @@ -4097,6 +4162,7 @@ test_metadata_is_bounded_before_reading test_report_temps_are_exclusive_and_randomized test_visual_inventory_is_count_and_depth_bounded test_required_source_fails_closed +test_required_sections_accept_one_top_heading_level_in_order test_required_sections_fail_actionably test_required_sections_reject_empty_bodies test_required_sections_reject_container_only_markers diff --git a/tests/fm-send-popup-settle.test.sh b/tests/fm-send-popup-settle.test.sh index 685ce627ab..bf21ea963d 100755 --- a/tests/fm-send-popup-settle.test.sh +++ b/tests/fm-send-popup-settle.test.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# fm-send pre-submit popup-settle selection (the codex `$` fix). +# fm-send bare-skill syntax guard and pre-submit popup-settle selection. # # Some TUIs open a completion popup when the composer's first character triggers # it: codex (and others) for a leading `/` slash command, and codex specifically @@ -24,6 +24,12 @@ # real safety net; this settle is only the optimization that lets the popup clear so # the first Enter lands. # +# A bare installed-skill token is refused before typing when its prefix is wrong +# for the resolved harness. That guard is intentionally separate from settle: +# matching forms still reach the matrix below, ordinary slash commands and +# multi-token `$` prose keep their old behavior, and unknown-harness explicit +# targets remain an escape hatch. +# # Every case below passes a LITERAL `$` / `$price` message in single quotes # on purpose - the whole point is to send an unexpanded `$...` line to the agent - # so SC2016 (which flags single-quoted `$` as a probably-forgotten expansion) is a @@ -109,6 +115,25 @@ first_settle() { #