fix(factory): make artifact inspection deterministic (#184)

Author: walker-tx

factory/coordinator.md

@@ -18,6 +18,9 @@ For every `output_schema` subagent (research, writer, each reviewer, and revisio
 
 Writer completion is valid only when its structured `completed` is true, `open_questions` is valid, and caught file validation confirms the four expected physical files (`research.md`, `meta.yaml`, `external.md`, and `speakeasy.md`) and allowed paths. Revision completion is valid only when its structured `completed` is true, `addressed` and `disputed` are valid arrays, caught file validation confirms allowed paths/artifacts, and a later confirmatory review wave verifies the edits. Structured claims never substitute for physical verification.
 
+Perform phase artifact validation only through the repository-owned helper, using the exact command for the current phase: `bash factory/scripts/inspect-guide-artifacts.sh <slug> research`, `bash factory/scripts/inspect-guide-artifacts.sh <slug> writer`, or `bash factory/scripts/inspect-guide-artifacts.sh <slug> revision`. Run it from `/workspace` in a caught boundary and validate its JSON result. Do not construct ad hoc artifact-validation commands with `find`, `test`, `jq`, or another tool, and do not substitute the linter for this check.
+Accept only the exact keys `slug`, `stage`, and `artifacts`, with string slug/stage values equal to the command arguments and a sorted, unique string artifact array. Research accepts exactly `["meta.yaml","research.md"]` or the full four-file array `["external.md","meta.yaml","research.md","speakeasy.md"]`. Writer and revision require the full four-file array. Any other output is a caught validation failure.
+
 ## Phase 1 — read inputs and resolve identity
 
 Begin Phase 1 with one caught boundary executing exactly `bash factory/scripts/inspect-inputs.sh /input/issue.json /input/catalog.json`. Read issue evidence, catalog identity fields, available personas, and existing guide slugs only from that command's JSON output; do not construct another initial-inspection tool program or read either raw input another way. Resolve exactly one provider and lowercase kebab-case slug from this output. Prefer an existing slug on a confident match; never create an alias duplicate. If provider/slug is missing, conflicting, or ambiguous, choose `blocked`, leave all three identity fields null, and report without guide edits.
@@ -28,11 +31,11 @@ For a resolved slug, execute exactly `bash factory/scripts/inspect-guide-context
 
 Scope-gate classification is strict. Material uncertainty is limited to an unknown authentication model, endpoint, required credential, security-sensitive operator choice, provider capability or feasibility, or an unresolved authoritative-source conflict that could make the guide unsafe or unusable. Presentation-only uncertainty never selects `awaiting_scope`. Missing exact UI labels, control names or locations, and equivalent Save/Update/Apply chrome are presentation-only when the underlying operation and required value are known. After a reasonable source search, preserve documented identifiers, write a resilient hedge such as the visible or equivalent control, record the documentation silence, and continue. Open questions alone do not select `awaiting_scope`; classify each by its effect on safe first connection.
 
-Start the technical-research subagent in a caught boundary with the selected persona file, authority files, resolved identity/catalog facts, issue evidence, existing artifacts, primary-source requirement, and write access only to `research.md` and `meta.yaml`. Set `output_schema` to the exact `factory/schemas/research-status.schema.json`. Apply the universal transport/schema check and one-repair limit. In another caught file-validation boundary, confirm both artifacts are physical regular files and agree with the valid output. Only material unanswered decisions select `awaiting_scope`; authoritative evidence blockers select `blocked`; operational/caught errors select `failed`. Each terminal state skips later model phases and reaches reporting.
+Start the technical-research subagent in a caught boundary with the selected persona file, authority files, resolved identity/catalog facts, issue evidence, existing artifacts, primary-source requirement, and write access only to `research.md` and `meta.yaml`. Set `output_schema` to the exact `factory/schemas/research-status.schema.json`. Apply the universal transport/schema check and one-repair limit. In another caught file-validation boundary, execute exactly `bash factory/scripts/inspect-guide-artifacts.sh <slug> research` and confirm its result agrees with the valid output. Only material unanswered decisions select `awaiting_scope`; authoritative evidence blockers select `blocked`; operational/caught errors select `failed`. Each terminal state skips later model phases and reaches reporting.
 
 ## Phase 3 — writer
 
-Start one writer in a caught boundary with `doctrine/roles/writer.md`, the selected persona file, doctrine, `research.md`, and `meta.yaml`; forbid external research. Set `output_schema` to a strict object with only `completed` (boolean) and `open_questions` (array of nonempty strings). Apply the universal one-repair protocol and writer completion verification. Only material factual decisions under the Phase 2 scope-gate definition select `awaiting_scope`; presentation-only uncertainty must be hedged and rendered, and caught errors select `failed`.
+Start one writer in a caught boundary with `doctrine/roles/writer.md`, the selected persona file, doctrine, `research.md`, and `meta.yaml`; forbid external research. Set `output_schema` to a strict object with only `completed` (boolean) and `open_questions` (array of nonempty strings). Apply the universal one-repair protocol and writer completion verification. Writer completion verification must execute exactly `bash factory/scripts/inspect-guide-artifacts.sh <slug> writer` in a caught boundary. Only material factual decisions under the Phase 2 scope-gate definition select `awaiting_scope`; presentation-only uncertainty must be hedged and rendered, and caught errors select `failed`.
 
 ## Phase 4 — bounded concurrent review/revision state machine
 
@@ -44,7 +47,7 @@ REVIEWER 3/3 — editorial clarity and audience fit, using `doctrine/roles/revie
 
 Each reviewer runs in its own caught boundary with `output_schema` equal to `factory/schemas/review-findings.schema.json` and the universal one-repair protocol. The full concurrent dispatch/collection runs in an enclosing caught boundary. Run the shell/linter in its own caught boundary from `/workspace`, exactly `/usr/local/bin/lint-guide --json /workspace/guides/<slug>`; never invoke `go` or `go run`. Validate parsed linter JSON before use. A completed review wave means valid output from all 3 reviewers plus a successfully parsed linter result. A failed reviewer output, malformed output after repair, linter failure, or invalid linter JSON fails the wave and must not complete the wave and therefore do not increment `review_rounds`; it selects `failed` and routes to reporting.
 
-Only after a completed review wave increment actual `review_rounds` by one (maximum 3). Normalize semantic duplicates without dropping sources; linter blockers equal reviewer blockers. If there are no blockers, select `converged`. If blockers remain and `review_rounds < 3`, start exactly one revision in a caught boundary with all normalized findings, doctrine, current files, and the selected persona; forbid external research and outside edits. Revision agents must not run validation commands; the coordinator owns post-edit file validation and the next complete review wave. Explicitly prohibit validation including `go`, `go run`, `npx`, Python, and `/usr/local/bin/lint-guide`; tell the revision agent to make only the requested allowed-path edits and return its structured result. Its strict `output_schema` has only `completed` (boolean), `addressed` (array), and `disputed` (array). Apply one repair and revision completion verification, then always run a confirmatory review wave; a revision can never directly converge. Repeat while capacity remains. If the confirmatory third wave has final-round blockers, select `blocked`; do not revise again. Thus at most three review/revision rounds occur, represented by at most three complete waves, and the report records the actual count.
+Only after a completed review wave increment actual `review_rounds` by one (maximum 3). Normalize semantic duplicates without dropping sources; linter blockers equal reviewer blockers. If there are no blockers, select `converged`. If blockers remain and `review_rounds < 3`, start exactly one revision in a caught boundary with all normalized findings, doctrine, current files, and the selected persona; forbid external research and outside edits. Revision agents must not run validation commands; the coordinator owns post-edit file validation and the next complete review wave. Explicitly prohibit validation including `go`, `go run`, `npx`, Python, and `/usr/local/bin/lint-guide`; tell the revision agent to make only the requested allowed-path edits and return its structured result. Its strict `output_schema` has only `completed` (boolean), `addressed` (array), and `disputed` (array). Apply one repair, then execute exactly `bash factory/scripts/inspect-guide-artifacts.sh <slug> revision` in a caught boundary for revision completion verification. Always run a confirmatory review wave; a revision can never directly converge. Repeat while capacity remains. If the confirmatory third wave has final-round blockers, select `blocked`; do not revise again. Thus at most three review/revision rounds occur, represented by at most three complete waves, and the report records the actual count.
 
 Deterministic scenario rulings: failed reviewer output -> `failed`, zero increment, report; malformed output -> one same-session repair then `failed` on exhaustion; successful revision -> mandatory confirmatory review wave; final-round blockers -> `blocked` with `review_rounds = 3`.
 

factory/scripts/inspect-guide-artifacts.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+SCRIPT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+ROOT=${FACTORY_REPO_ROOT:-$SCRIPT_ROOT}
+ROOT="$(cd "$ROOT" && pwd -P)"
+# shellcheck disable=SC1091
+source "$SCRIPT_ROOT/factory/scripts/lib.sh"
+
+[[ $# -eq 2 ]] || die "usage: ${0##*/} <slug> <research|writer|revision>"
+slug=$1
+stage=$2
+[[ "$slug" =~ ^[a-z0-9]+(-[a-z0-9]+)*$ ]] || die "invalid guide slug"
+case "$stage" in
+  research|writer|revision) ;;
+  *) die "invalid inspection stage" ;;
+esac
+
+target="$ROOT/guides/$slug"
+[[ -d "$target" && ! -L "$target" ]] || die "invalid target guide directory"
+[[ "$(realpath "$target")" == "$ROOT/guides/$slug" ]] || die "invalid target guide directory"
+
+required=(research.md meta.yaml)
+if [[ "$stage" != research ]]; then
+  required+=(external.md speakeasy.md)
+fi
+for artifact in "${required[@]}"; do
+  path="$target/$artifact"
+  [[ -e "$path" || -L "$path" ]] || die "missing required guide artifact"
+  [[ -f "$path" && ! -L "$path" ]] || die "invalid guide artifact"
+done
+
+if [[ "$stage" == research ]]; then
+  external_present=false
+  speakeasy_present=false
+  [[ -e "$target/external.md" || -L "$target/external.md" ]] && external_present=true
+  [[ -e "$target/speakeasy.md" || -L "$target/speakeasy.md" ]] && speakeasy_present=true
+  [[ "$external_present" == "$speakeasy_present" ]] || die "incomplete setup artifact pair"
+fi
+
+artifacts=()
+shopt -s nullglob dotglob
+for path in "$target"/*; do
+  artifact=${path##*/}
+  case "$artifact" in
+    research.md|meta.yaml|external.md|speakeasy.md) ;;
+    *) die "unexpected guide artifact" ;;
+  esac
+  [[ -f "$path" && ! -L "$path" ]] || die "invalid guide artifact"
+  artifacts+=("$artifact")
+done
+
+printf '%s\n' "${artifacts[@]}" | sort | jq -Rsc --arg slug "$slug" --arg stage "$stage" \
+  '{slug:$slug,stage:$stage,artifacts:(split("\n") | map(select(length > 0)))}'

factory/tests/test-coordinator.sh

@@ -14,6 +14,13 @@ for phrase in \
   'bash factory/scripts/inspect-inputs.sh /input/issue.json /input/catalog.json' \
   'do not construct another initial-inspection tool program' \
   'bash factory/scripts/inspect-guide-context.sh <slug>' \
+  'bash factory/scripts/inspect-guide-artifacts.sh <slug> research' \
+  'bash factory/scripts/inspect-guide-artifacts.sh <slug> writer' \
+  'bash factory/scripts/inspect-guide-artifacts.sh <slug> revision' \
+  'Do not construct ad hoc artifact-validation commands' \
+  "Accept only the exact keys \`slug\`, \`stage\`, and \`artifacts\`" \
+  "Research accepts exactly \`[\"meta.yaml\",\"research.md\"]\` or the full four-file array" \
+  'Writer and revision require the full four-file array' \
   'never inspect repository files directly or run another Phase 1 file-discovery tool' \
   'permit only bounded reads of the helper-generated spill artifact' \
   'use only installed jq and sed for spill reads; Python is unavailable' \
@@ -48,6 +55,13 @@ done
 
 
 child_start_contract="$(sed -n '/^## Phase 2/,/^## Phase 5/p' "$CONTRACT")"
+phase2_contract="$(sed -n '/^## Phase 2/,/^## Phase 3/p' "$CONTRACT")"
+phase3_contract="$(sed -n '/^## Phase 3/,/^## Phase 4/p' "$CONTRACT")"
+phase4_contract="$(sed -n '/^## Phase 4/,/^## Phase 5/p' "$CONTRACT")"
+grep -Fq 'bash factory/scripts/inspect-guide-artifacts.sh <slug> research' <<<"$phase2_contract" || fail 'research phase does not invoke exact artifact helper'
+grep -Fq 'bash factory/scripts/inspect-guide-artifacts.sh <slug> writer' <<<"$phase3_contract" || fail 'writer phase does not invoke exact artifact helper'
+grep -Fq 'bash factory/scripts/inspect-guide-artifacts.sh <slug> revision' <<<"$phase4_contract" || fail 'revision phase does not invoke exact artifact helper'
+
 if grep -Eq '(^|[,{[:space:]])(model|harness)[[:space:]]*:|--(model|harness)' <<<"$child_start_contract"; then
   fail 'child start contains an explicit model or harness override'
 fi

factory/tests/test-inspect-guide-artifacts.sh

@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+set -euo pipefail
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+# shellcheck disable=SC1091
+source "$ROOT/factory/tests/test-helper.sh"
+INSPECTOR="$ROOT/factory/scripts/inspect-guide-artifacts.sh"
+TMP="$(mktemp -d)"
+trap 'rm -rf "$TMP"' EXIT
+REPO="$TMP/repo"
+mkdir -p "$REPO/guides/box"
+
+printf '%s\n' dossier >"$REPO/guides/box/research.md"
+printf '%s\n' metadata >"$REPO/guides/box/meta.yaml"
+FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out"
+jq -e '(keys == ["artifacts", "slug", "stage"]) and .slug == "box" and .stage == "research" and .artifacts == ["meta.yaml", "research.md"]' "$TMP/out" >/dev/null
+
+printf '%s\n' partial >"$REPO/guides/box/external.md"
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out" 2>"$TMP/err"; then
+  fail 'research inspection accepted a partial setup artifact pair'
+fi
+assert_contains 'factory: incomplete setup artifact pair' "$(cat "$TMP/err")"
+rm "$REPO/guides/box/external.md"
+
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out" 2>"$TMP/err"; then
+  fail 'writer inspection accepted missing setup files'
+fi
+assert_contains 'factory: missing required guide artifact' "$(cat "$TMP/err")"
+
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box revision >"$TMP/out" 2>"$TMP/err"; then
+  fail 'revision inspection accepted missing setup files'
+fi
+assert_contains 'factory: missing required guide artifact' "$(cat "$TMP/err")"
+
+printf '%s\n' external >"$REPO/guides/box/external.md"
+printf '%s\n' speakeasy >"$REPO/guides/box/speakeasy.md"
+FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out"
+jq -e '.stage == "research" and .artifacts == ["external.md", "meta.yaml", "research.md", "speakeasy.md"]' "$TMP/out" >/dev/null
+FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out"
+jq -e '(keys == ["artifacts", "slug", "stage"]) and .artifacts == ["external.md", "meta.yaml", "research.md", "speakeasy.md"]' "$TMP/out" >/dev/null
+FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box revision >/dev/null
+
+printf '%s\n' unexpected >"$REPO/guides/box/notes.md"
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out" 2>"$TMP/err"; then
+  fail 'writer inspection accepted unexpected artifact'
+fi
+assert_contains 'factory: unexpected guide artifact' "$(cat "$TMP/err")"
+rm "$REPO/guides/box/notes.md"
+
+rm "$REPO/guides/box/external.md"
+ln -s /etc/passwd "$REPO/guides/box/external.md"
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out" 2>"$TMP/err"; then
+  fail 'writer inspection accepted symlinked artifact'
+fi
+assert_contains 'factory: invalid guide artifact' "$(cat "$TMP/err")"
+
+rm -rf "$REPO/guides/box"
+mkdir "$REPO/real-box"
+ln -s "$REPO/real-box" "$REPO/guides/box"
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out" 2>"$TMP/err"; then
+  fail 'inspection accepted symlinked guide directory'
+fi
+assert_contains 'factory: invalid target guide directory' "$(cat "$TMP/err")"
+
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" '../box' writer >"$TMP/out" 2>"$TMP/err"; then
+  fail 'inspection accepted unsafe slug'
+fi
+assert_contains 'factory: invalid guide slug' "$(cat "$TMP/err")"
+if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box review >"$TMP/out" 2>"$TMP/err"; then
+  fail 'inspection accepted invalid stage'
+fi
+assert_contains 'factory: invalid inspection stage' "$(cat "$TMP/err")"
+
+printf 'PASS: deterministic guide artifact inspection\n'