diff --git a/.agents/skills/execution-playbooks/references/plan-execute-v1.md b/.agents/skills/execution-playbooks/references/plan-execute-v1.md index 9dbdf7f4..b16c3c1e 100644 --- a/.agents/skills/execution-playbooks/references/plan-execute-v1.md +++ b/.agents/skills/execution-playbooks/references/plan-execute-v1.md @@ -10,14 +10,16 @@ The planner owns the design; the executing worker receives the plan, not the sou 1. Accept a brief whose `## Execution plan` section is complete. The section must carry these five labels, each written as a label line followed by its list entries: `Files to touch` (at least one path-like entry, i.e. real file paths), `Ordered steps` (steps numbered, e.g. `1.`), `Acceptance criteria`, `Verification commands` (a command to run), and `Out of scope` (an explicit out-of-scope statement). 2. Execute the plan as written; do not redesign, re-scope, or re-plan the change. -3. Follow `tlc-implement` for the implementation method and write the checklist under `data//artifacts/`. +3. Follow `tlc-implement` for the implementation method and write the checklist only under the Squad base's `data//artifacts/`, never in the project repository. + A project commit must never carry a Squad internal artifact. 4. Stop with `blocked:` when the plan is incomplete, internally contradictory, or contradicted by the code, naming the exact gap rather than filling it by inference. 5. Report acceptance evidence and the verification command result; leave review, fixes, delivery, and merge to the existing owners. ## Required evidence - The brief's materialized `## Execution plan` section, with every required field present and non-empty. -- A checklist under `data//artifacts/` recording the plan fields as realized: files touched as planned, steps executed, acceptance met, and the verification command run. +- A checklist under the Squad base's `data//artifacts/` recording the plan fields as realized: files touched as planned, steps executed, acceptance met, and the verification command run. +- No project commit contains this Squad internal artifact. - A `blocked:` report naming the missing or contradictory plan element when execution cannot proceed safely. ## Exit predicate diff --git a/bin/sq-brief.sh b/bin/sq-brief.sh index 8a6a740b..65c3e9e4 100755 --- a/bin/sq-brief.sh +++ b/bin/sq-brief.sh @@ -187,6 +187,7 @@ if [ "$PLAYBOOK_SET" -eq 1 ]; then visual-parity@1) PLAYBOOK_ID=visual-parity; PLAYBOOK_VERSION=1; PLAYBOOK_SECTION=$(cat "$SQUAD_ROOT/.agents/skills/execution-playbooks/references/visual-parity-v1.md"); EXPECTED_KIND='recon|strike' ;; multi-phase-plan@1) PLAYBOOK_ID=multi-phase-plan; PLAYBOOK_VERSION=1; PLAYBOOK_SECTION=$(cat "$SQUAD_ROOT/.agents/skills/execution-playbooks/references/multi-phase-plan-v1.md"); EXPECTED_KIND=recon ;; "eval@1") PLAYBOOK_ID='eval'; PLAYBOOK_VERSION=1; PLAYBOOK_SECTION=$(cat "$SQUAD_ROOT/.agents/skills/execution-playbooks/references/eval-v1.md"); EXPECTED_KIND=recon ;; + # The generated checklist location and commit boundary live in the playbook reference. plan-execute@1) PLAYBOOK_ID=plan-execute; PLAYBOOK_VERSION=1; PLAYBOOK_SECTION=$(cat "$SQUAD_ROOT/.agents/skills/execution-playbooks/references/plan-execute-v1.md"); EXPECTED_KIND=strike ;; orchestrate@1) echo "error: orchestrate@1 disposition=reject; Commander, XO, backlog, and supervision already own programme coordination" >&2; exit 1 ;; autopilot-full@1|autopilot-stack@1) echo "error: $PLAYBOOK disposition=defer; re-evaluate only for a real multi-PR programme where commander merge approval is the measured bottleneck and yolo cannot cover it; no auto-merge or drill bypass" >&2; exit 1 ;; diff --git a/bin/sq-pr-artifact-guard.sh b/bin/sq-pr-artifact-guard.sh new file mode 100755 index 00000000..bfa74595 --- /dev/null +++ b/bin/sq-pr-artifact-guard.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# Report task-owned Squad artifacts found in a PR's changed-file list. +# Usage: sq-pr-artifact-guard.sh (reads newline-delimited paths on stdin) +set -eu + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/sq-pr-lib.sh +. "$SCRIPT_DIR/sq-pr-lib.sh" + +if [ "$#" -ne 1 ] || ! fm_pr_task_id_valid "$1"; then + echo "error: invalid PR artifact guard request" >&2 + exit 2 +fi +ID=$1 +matches=$(awk -v prefix="data/$ID/" 'index($0, prefix) == 1 { print }' | LC_ALL=C sort -u) +if [ -n "$matches" ]; then + printf 'warning: PR contains Squad internal artifact path(s) for task %s; remove them before merge:\n%s\n' "$ID" "$matches" >&2 +fi diff --git a/bin/sq-pr-check.sh b/bin/sq-pr-check.sh index 5f4773b6..e9a5a1be 100755 --- a/bin/sq-pr-check.sh +++ b/bin/sq-pr-check.sh @@ -5,6 +5,8 @@ # 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. +# It also warns, without failing, when the PR changed-file list names this +# task's own data// artifact paths. # Usage: sq-pr-check.sh set -eu @@ -78,6 +80,43 @@ if [ "$PROVIDER" = github ] && [ -n "$WT" ] && [ -d "$WT" ] && command -v gh >/d fi fi +# Warn before arming if the task's own Squad artifacts appear in its project PR. +# Failure to fetch the file list is non-fatal and leaves the existing flow intact. +PR_FILES= +if [ "$PROVIDER" = github ] && command -v gh >/dev/null 2>&1; then + PR_FILES=$(gh api "repos/$PROJECT_PATH/pulls/$NUMBER/files" --paginate --jq '.[].filename' 2>/dev/null || true) +elif [ "$PROVIDER" = gitlab ] && command -v glab >/dev/null 2>&1; then + ENCODED_PROJECT=${PROJECT_PATH//\//%2F} + if PR_DIFFS=$(glab api "projects/$ENCODED_PROJECT/merge_requests/$NUMBER/diffs" --paginate --hostname "$HOST" 2>/dev/null); then + if ! PR_FILES=$(printf '%s' "$PR_DIFFS" | python3 -c 'import json, sys +text = sys.stdin.read() +decoder = json.JSONDecoder() +offset = 0 +while offset < len(text): + while offset < len(text) and text[offset].isspace(): + offset += 1 + if offset == len(text): + break + page, offset = decoder.raw_decode(text, offset) + if not isinstance(page, list): + raise ValueError("expected a JSON array from a paginated GitLab API response") + for diff in page: + if not isinstance(diff, dict): + raise ValueError("expected GitLab diff records to be JSON objects") + for field in ("new_path", "old_path"): + path = diff.get(field) + if path: + print(path)'); then + printf 'warning: GitLab merge request %s was not inspected for Squad internal artifacts because its changed-file response could not be parsed.\n' "$URL" >&2 + fi + else + printf 'warning: GitLab merge request %s was not inspected for Squad internal artifacts because its changed-file list could not be fetched.\n' "$URL" >&2 + fi +fi +if [ -n "$PR_FILES" ]; then + printf '%s\n' "$PR_FILES" | "$SCRIPT_DIR/sq-pr-artifact-guard.sh" "$ID" || true +fi + META_TMP= pr_check_cleanup() { fm_pr_poll_cleanup diff --git a/bin/sq-test-run.sh b/bin/sq-test-run.sh index 90eadd04..f4522abf 100755 --- a/bin/sq-test-run.sh +++ b/bin/sq-test-run.sh @@ -202,7 +202,7 @@ family_for_basename() { sq-teardown-endpoint-safety.test.sh) printf '%s\n' backend-dispatch ;; - sq-pr-check-security.test.sh|sq-pr-merge.test.sh|sq-review-diff.test.sh|\ + sq-pr-artifact-guard.test.sh|sq-pr-check-security.test.sh|sq-pr-merge.test.sh|sq-review-diff.test.sh|\ sq-teardown.test.sh|sq-x-mode.test.sh) printf '%s\n' pr-forge ;; diff --git a/docs/scripts.md b/docs/scripts.md index 77cbc70a..6bb9cfd3 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -124,6 +124,7 @@ The shared drill gate refusal for unit lifecycle entrypoints is summarized in [a | `sq-pr-poll.sh` | Provide the byte-static sentry program for validated PR/MR-poll sidecars | | `sq-pr-check-migrate.sh` | Quarantine older task polls without execution and rebuild only canonical polls | | `sq-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll | +| `sq-pr-artifact-guard.sh` | Warn when a PR changed-file list names this task's own Squad-base artifact paths | | `sq-pr-merge.sh` | Record PR metadata, then merge a task's canonical full GitHub URL | | `sq-promote.sh` | Promote a recon task in place to a protected strike task with an explicit delivery mode | | `sq-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed recon deliverables, retire XO bases | diff --git a/tests/sq-brief.test.sh b/tests/sq-brief.test.sh index c0c6c221..605d3d7c 100755 --- a/tests/sq-brief.test.sh +++ b/tests/sq-brief.test.sh @@ -411,7 +411,10 @@ test_remaining_lifecycle_playbooks() { assert_grep "execution-class" "$brief" "plan-execute contract missing execution-class scope" assert_grep "materialized from the planning artifact" "$brief" "plan-execute contract missing the plan materialization rule" assert_grep "tlc-implement" "$brief" "plan-execute contract missing tlc-implement" - assert_grep "data//artifacts/" "$brief" "plan-execute contract missing the checklist location" + assert_grep 'Squad base' "$brief" "plan-execute contract missing the Squad-base checklist location" + assert_grep 'data//artifacts/' "$brief" "plan-execute contract missing the checklist path" + assert_grep "never in the project repository" "$brief" "plan-execute contract must forbid storing the checklist in the project" + assert_grep "A project commit must never carry a Squad internal artifact" "$brief" "plan-execute contract must forbid committing Squad artifacts" assert_grep "blocked:" "$brief" "plan-execute contract missing the stop-with-blocked rule" out=$(SQUAD_BASE="$home" "$ROOT/bin/sq-brief.sh" lifecycle-plan-execute-recon repo --recon --playbook plan-execute@1 2>&1); status=$? [ "$status" -ne 0 ] || fail "plan-execute@1 must refuse recon" diff --git a/tests/sq-pr-artifact-guard.test.sh b/tests/sq-pr-artifact-guard.test.sh new file mode 100755 index 00000000..22bb1b80 --- /dev/null +++ b/tests/sq-pr-artifact-guard.test.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Behavior tests for reporting task-owned Squad artifacts in PR file lists. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +GUARD="$ROOT/bin/sq-pr-artifact-guard.sh" + +matches=$(printf '%s\n' \ + 'src/main.sh' \ + 'data/task-a/artifacts/checklist.md' \ + 'data/task-a/report.md' \ + 'data/other-task/artifacts/checklist.md' \ + | "$GUARD" task-a 2>&1) +assert_contains "$matches" 'data/task-a/artifacts/checklist.md' 'guard omitted the task checklist path' +assert_contains "$matches" 'data/task-a/report.md' 'guard omitted another task-owned artifact path' +assert_not_contains "$matches" 'data/other-task/' 'guard reported artifacts belonging to another task' + +clean=$(printf '%s\n' 'src/main.sh' 'data/task-ab/readme.md' | "$GUARD" task-a 2>&1) +[ -z "$clean" ] || fail "guard should be silent for a clean PR list, got: $clean" + +pass "sq-pr-artifact-guard.sh: reports task-owned artifact paths and stays silent for clean PRs" diff --git a/tests/sq-pr-check-security.test.sh b/tests/sq-pr-check-security.test.sh index 8acdf4c1..b4e2e96e 100755 --- a/tests/sq-pr-check-security.test.sh +++ b/tests/sq-pr-check-security.test.sh @@ -66,6 +66,10 @@ SH printf '%s\n' "$*" >> "$SQUAD_TEST_GH_LOG" case " $* " in *" headRefOid "*) printf '%s\n' "${SQUAD_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}" ;; + *"/pulls/"*"files"*) + [ "${SQUAD_TEST_GH_API_FAIL:-0}" = 0 ] || exit 1 + [ -n "${SQUAD_TEST_GH_FILES:-}" ] && printf '%s\n' "$SQUAD_TEST_GH_FILES" + ;; *" state "*) [ "${SQUAD_TEST_GH_FAIL:-0}" = 0 ] || exit 1 [ "${SQUAD_TEST_GH_SLEEP:-0}" = 0 ] || sleep "$SQUAD_TEST_GH_SLEEP" @@ -78,14 +82,28 @@ SH printf '%s\n' "$*" >> "$SQUAD_TEST_SQ_GH_LOG" exit "${SQUAD_TEST_GH_AXI_RC:-0}" SH - # Plain glab, reproducing the real CLI's contract: its field output on stdout - # and exit 0 on success, and a non-zero exit with no stdout on any failure. + # Plain glab, reproducing the pinned CLI's documented API contract and refusing + # unsupported output-selector flags so tests cannot mask an invalid invocation. cat > "$fakebin/glab" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$SQUAD_TEST_GLAB_LOG" +for arg in "$@"; do + case "$arg" in + --paginate|-R|--hostname) ;; + -*) exit 2 ;; + esac +done [ "${SQUAD_TEST_GLAB_FAIL:-0}" = 0 ] || exit 1 [ "${SQUAD_TEST_GLAB_SLEEP:-0}" = 0 ] || sleep "$SQUAD_TEST_GLAB_SLEEP" -printf 'title:\tfixture merge request\nstate:\t%s\nauthor:\tsomeone\n' "${SQUAD_TEST_GLAB_STATE:-opened}" +case " $* " in + *" api "*) + [ -n "${SQUAD_TEST_GLAB_JSON:-}" ] || exit 0 + cat "$SQUAD_TEST_GLAB_JSON" + ;; + *) + printf 'title:\tfixture merge request\nstate:\t%s\nauthor:\tsomeone\n' "${SQUAD_TEST_GLAB_STATE:-opened}" + ;; +esac SH chmod +x "$fakebin/gh" "$fakebin/sq-gh" "$fakebin/glab" : > "$dir/gh.log" @@ -2885,6 +2903,124 @@ EOF pass "GitLab merge requests are followed on any instance and never wake falsely" } +test_gitlab_artifact_guard() { + local dir state url files out rc + dir=$(make_case gitlab-artifact-guard) + state="$dir/home/state" + url=https://gitlab.example/group/subgroup/project/-/merge_requests/7 + write_task_meta "$dir" + files="$dir/diffs.json" + cat > "$files" <<'JSON' +[{"old_path":"src/main.sh","new_path":"src/main.sh"},{"old_path":"data/task-a/artifacts/checklist.md","new_path":"data/task-a/artifacts/checklist.md"}] +[{"old_path":"data/task-a/artifacts/page-2.md","new_path":"data/task-a/artifacts/page-2.md"},{"old_path":"data/other-task/report.md","new_path":"data/other-task/report.md"}] +JSON + + set +e + out=$(SQUAD_ROOT_OVERRIDE="$dir/root" SQUAD_BASE="$dir/home" \ + SQUAD_TEST_GUARD_LOG="$dir/guard.log" SQUAD_TEST_GH_LOG="$dir/gh.log" \ + SQUAD_TEST_SQ_GH_LOG="$dir/sq-gh.log" SQUAD_TEST_GLAB_LOG="$dir/glab.log" \ + SQUAD_TEST_GLAB_JSON="$files" \ + PATH="$dir/fakebin:$BASE_PATH" \ + "$PR_CHECK" task-a "$url" 2>&1) + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "GitLab arming with a stubbed file list failed: $out" + case "$out" in + *"data/task-a/artifacts/checklist.md"*) ;; + *) fail "GitLab artifact guard did not name the task-owned artifact path: $out" ;; + esac + case "$out" in + *"data/task-a/artifacts/page-2.md"*) ;; + *) fail "GitLab artifact guard did not name a task-owned artifact from a later page: $out" ;; + esac + case "$out" in + *"data/other-task/report.md"*) fail "GitLab artifact guard named another task's artifact: $out" ;; + esac + grep -qF -- "merge_requests/7/diffs" "$dir/glab.log" \ + || fail "GitLab artifact guard did not request the merge request diffs" + grep -qF -- "--paginate" "$dir/glab.log" \ + || fail "GitLab artifact guard did not request all diff pages" + grep -qF -- "--hostname gitlab.example" "$dir/glab.log" \ + || fail "GitLab artifact guard did not pin the merge request's host" + assert_no_grep '--output' "$dir/glab.log" \ + "GitLab artifact guard requested an unsupported output selector" + assert_no_grep '--jq' "$dir/glab.log" \ + "GitLab artifact guard requested an unsupported output selector" + fm_pr_poll_artifacts_valid "$state" task-a "$POLL" \ + || fail "GitLab artifact guard run left no valid poll" + + : > "$files" + set +e + out=$(SQUAD_ROOT_OVERRIDE="$dir/root" SQUAD_BASE="$dir/home" \ + SQUAD_TEST_GUARD_LOG="$dir/guard.log" SQUAD_TEST_GH_LOG="$dir/gh.log" \ + SQUAD_TEST_SQ_GH_LOG="$dir/sq-gh.log" SQUAD_TEST_GLAB_LOG="$dir/glab.log" \ + SQUAD_TEST_GLAB_JSON="$files" \ + PATH="$dir/fakebin:$BASE_PATH" \ + "$PR_CHECK" task-a "$url" 2>&1) + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "GitLab arming with a clean file list failed: $out" + case "$out" in + *"Squad internal artifact"*) fail "GitLab artifact guard warned for a clean file list: $out" ;; + esac + + set +e + out=$(SQUAD_ROOT_OVERRIDE="$dir/root" SQUAD_BASE="$dir/home" \ + SQUAD_TEST_GUARD_LOG="$dir/guard.log" SQUAD_TEST_GH_LOG="$dir/gh.log" \ + SQUAD_TEST_SQ_GH_LOG="$dir/sq-gh.log" SQUAD_TEST_GLAB_LOG="$dir/glab.log" \ + SQUAD_TEST_GLAB_FAIL=1 \ + PATH="$dir/fakebin:$BASE_PATH" \ + "$PR_CHECK" task-a "$url" 2>&1) + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "GitLab arming was fatal when the file list could not be fetched: $out" + case "$out" in + *"was not inspected"*) ;; + *) fail "GitLab artifact guard did not loudly report an unavailable file list: $out" ;; + esac + fm_pr_poll_artifacts_valid "$state" task-a "$POLL" \ + || fail "GitLab arming with a failed file-list fetch left no valid poll" + + printf '%s\n' '[{"old_path":"src/main.sh","new_path":' > "$files" + set +e + out=$(SQUAD_ROOT_OVERRIDE="$dir/root" SQUAD_BASE="$dir/home" \ + SQUAD_TEST_GUARD_LOG="$dir/guard.log" SQUAD_TEST_GH_LOG="$dir/gh.log" \ + SQUAD_TEST_SQ_GH_LOG="$dir/sq-gh.log" SQUAD_TEST_GLAB_LOG="$dir/glab.log" \ + SQUAD_TEST_GLAB_JSON="$files" \ + PATH="$dir/fakebin:$BASE_PATH" \ + "$PR_CHECK" task-a "$url" 2>&1) + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "GitLab arming was fatal when the file list could not be parsed: $out" + case "$out" in + *"could not be parsed"*) ;; + *) fail "GitLab artifact guard did not loudly report an unparseable file list: $out" ;; + esac + fm_pr_poll_artifacts_valid "$state" task-a "$POLL" \ + || fail "GitLab arming with an unparseable file-list response left no valid poll" + + pass "GitLab artifact guard parses paginated default JSON and reports fetch and parse failures" +} + +test_github_artifact_guard() { + local dir url out + dir=$(make_case github-artifact-guard) + write_task_meta "$dir" + url=https://github.com/o/r/pull/7 + out=$(SQUAD_TEST_GH_FILES=$(printf '%s\n' 'src/main.sh' 'data/task-a/artifacts/checklist.md' 'data/other-task/report.md') \ + run_check_entry "$dir" task-a "$url" 2>&1) + case "$out" in + *"data/task-a/artifacts/checklist.md"*) ;; + *) fail "GitHub artifact guard did not name the task-owned path: $out" ;; + esac + case "$out" in + *"data/other-task/report.md"*) fail "GitHub artifact guard named another task's path: $out" ;; + esac + grep -qF -- "repos/o/r/pulls/7/files" "$dir/gh.log" \ + || fail "GitHub artifact guard did not request the PR changed-file list" + pass "GitHub artifact guard inspects the PR file-list wiring" +} + seed_canonical_poll() { local dir=$1 id=$2 url=$3 template=${4:-$POLL} state provider host path number state="$dir/home/state" @@ -3327,6 +3463,8 @@ test_gitlab_merged_poll_retires() { test_parser_matrix test_gitlab_merge_watch +test_gitlab_artifact_guard +test_github_artifact_guard test_merged_poll_retires_once test_persistent_XO_retirement_is_poll_only test_retirement_crash_recovery