diff --git a/.agents/skills/firstmate-coding-guidelines/SKILL.md b/.agents/skills/firstmate-coding-guidelines/SKILL.md index 0ed6d4b8f52..5885617b108 100644 --- a/.agents/skills/firstmate-coding-guidelines/SKILL.md +++ b/.agents/skills/firstmate-coding-guidelines/SKILL.md @@ -3,7 +3,7 @@ name: firstmate-coding-guidelines description: >- Agent-only reference for changing firstmate's shared, tracked material per AGENTS.md section 1. Use before editing any of that material, whether working as firstmate directly or as a crewmate briefed on a firstmate-repo task. - Covers the knowledge-placement decision tree, the one-owner rule for contracts, the inline-stub pattern for content moved into a skill, AGENTS.md size discipline, trigger hygiene for new skills, and repo style rules (one sentence per line, plain dash, no agent co-author, shellcheck-clean bin scripts, colocated tests, and maintainer-verification evidence). + Covers the knowledge-placement decision tree, the one-owner rule for contracts, the inline-stub pattern for content moved into a skill, AGENTS.md size discipline, trigger hygiene for new skills, and repo style rules (one sentence per line, plain dash, no agent co-author, shellcheck-clean shell scripts, colocated tests, and maintainer-verification evidence). user-invocable: false metadata: internal: true @@ -124,7 +124,7 @@ Firstmate PR #3644 demonstrated the cost: pinning a 75-162-script walk took 32.7 - Never wrap multiple sentences onto one physical line. - Plain dash `-`, never an em dash. - Never add an agent name as a commit co-author. -- `bin/*.sh` and `bin/backends/*.sh` must pass `shellcheck`. +- Tracked shell scripts must pass `shellcheck`, test scripts included; `bin/fm-lint.sh`'s header owns the exact file set and which codes only run in CI. - Run `bin/fm-lint.sh` before treating a script change as done; it is the single owner of the lint definition that CI and the no-mistakes pre-push gate both invoke, its own header owns what that definition covers, and it refuses to run under any other version of either linter. - When a task names a specific tool, implement the work with that tool, or explicitly flag the substitution and its new dependency footprint for review before shipping. - Colocate tests with the existing pattern in `tests/`, name them `.test.sh`, and extend an existing script rather than inventing a new runner. diff --git a/bin/fm-ff-lib.sh b/bin/fm-ff-lib.sh index f3ac3ba069e..fecf575014c 100644 --- a/bin/fm-ff-lib.sh +++ b/bin/fm-ff-lib.sh @@ -379,6 +379,7 @@ fetch_once() { fi FF_FETCH_ERROR="fetch failed" if ! ff_sync_origin_fork "$dir"; then + # shellcheck disable=SC2034 # Read by bin/fm-update.sh (exit "$FF_UPDATE_FAILED") after sourcing this library. FF_UPDATE_FAILED=1 return 1 fi @@ -433,6 +434,7 @@ remote_sync_failure_reason() { # # FF_UPDATE_FAILED result the local route already sets, instead of collapsing # into an ordinary skip the caller cannot distinguish from ubiquitous # transport failure. +# shellcheck disable=SC2034 # Read by bin/fm-update.sh and bin/fm-remote-secondmate-control.sh after sourcing this library. REMOTE_UPDATE_FAILED_STATUS=3 dirty_status() { @@ -508,6 +510,7 @@ ff_target() { echo "$label: skipped: $FF_FETCH_ERROR" return 0 fi + # shellcheck disable=SC2034 # Read by bin/fm-update.sh (echo "origin-verified: $FF_RUN_VERIFIED") after sourcing this library. [ -z "$FF_FORK_UNVERIFIED" ] || FF_RUN_VERIFIED=no fi default=$(default_branch "$dir") || { diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index 510a4320988..ab5088fda9c 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -694,6 +694,7 @@ test_enriched_wedge_under_declared_wait_uses_pause_cadence() { local dir state fakebin task win pane key reason i escalations dir=$(make_supercase enriched-wedge-declared-wait) state="$dir/state"; fakebin="$dir/fakebin" + # shellcheck disable=SC2100 # Literal task id, not arithmetic. task=paused-wedge-w1; win="sess:fm-$task"; pane="$dir/pane.txt" key=$(printf '%s' "$task" | tr ':/.' '___') fm_write_meta "$state/$task.meta" "window=$win" "backend=tmux" diff --git a/tests/fm-update.test.sh b/tests/fm-update.test.sh index 09b287c3785..22eaa3920eb 100755 --- a/tests/fm-update.test.sh +++ b/tests/fm-update.test.sh @@ -383,13 +383,16 @@ test_remote_leg_raises_the_fork_sync_status() { expected_status=$(. "$ROOT/bin/fm-ff-lib.sh"; printf '%s\n' "$REMOTE_UPDATE_FAILED_STATUS") # This host's own update exits nonzero: its FF_UPDATE_FAILED classifier fired. + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. cat > "$root/bin/fm-update.sh" <<'SH' #!/usr/bin/env bash printf 'fork sync: FAILED protected fork branch\n' exit 1 SH + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. chmod +x "$root/bin/fm-update.sh" rc=0 + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. out=$(FM_HOME="$home" FM_ROOT_OVERRIDE="$root" "$control" update sm1 2>&1) || rc=$? expect_code "$expected_status" "$rc" \ "a fork-synchronization failure on this host must raise the distinct status, not the generic die" @@ -398,19 +401,23 @@ SH # The root update completes but proves no safe origin result: an ordinary # failure, not the classifier, so it stays exit 1 and the parent reads a skip. + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. cat > "$root/bin/fm-update.sh" <<'SH' #!/usr/bin/env bash printf 'firstmate: skipped: dirty working tree\n' exit 0 SH + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. chmod +x "$root/bin/fm-update.sh" rc=0 + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. out=$(FM_HOME="$home" FM_ROOT_OVERRIDE="$root" "$control" update sm1 2>&1) || rc=$? expect_code 1 "$rc" "an unsafe root origin update is an ordinary skip, not the fork-sync status" # A home guard rejection never reaches this host's update at all, so it cannot # be escalated into a fleet-wide fork-sync failure either. rc=0 + # shellcheck disable=SC2031 # False positive: cross-file taint from tests/lib.sh's unrelated fm_test_tmproot() local root; not modified in a subshell here. out=$(FM_HOME="$home" FM_ROOT_OVERRIDE="$root" "$control" update sm2 2>&1) || rc=$? expect_code 1 "$rc" "an unusable remote home is an ordinary skip, not the fork-sync status" assert_contains "$out" "remote home belongs to sm1, not sm2" \