Skip to content

Commit 75a2c69

Browse files
committed
fix(factory): make artifact inspection deterministic
1 parent ac6684e commit 75a2c69

4 files changed

Lines changed: 147 additions & 3 deletions

File tree

factory/coordinator.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ For every `output_schema` subagent (research, writer, each reviewer, and revisio
1818

1919
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.
2020

21+
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.
22+
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.
23+
2124
## Phase 1 — read inputs and resolve identity
2225

2326
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
2831

2932
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.
3033

31-
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.
34+
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.
3235

3336
## Phase 3 — writer
3437

35-
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`.
38+
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`.
3639

3740
## Phase 4 — bounded concurrent review/revision state machine
3841

@@ -44,7 +47,7 @@ REVIEWER 3/3 — editorial clarity and audience fit, using `doctrine/roles/revie
4447

4548
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.
4649

47-
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.
50+
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.
4851

4952
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`.
5053

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
4+
SCRIPT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
5+
ROOT=${FACTORY_REPO_ROOT:-$SCRIPT_ROOT}
6+
ROOT="$(cd "$ROOT" && pwd -P)"
7+
# shellcheck disable=SC1091
8+
source "$SCRIPT_ROOT/factory/scripts/lib.sh"
9+
10+
[[ $# -eq 2 ]] || die "usage: ${0##*/} <slug> <research|writer|revision>"
11+
slug=$1
12+
stage=$2
13+
[[ "$slug" =~ ^[a-z0-9]+(-[a-z0-9]+)*$ ]] || die "invalid guide slug"
14+
case "$stage" in
15+
research|writer|revision) ;;
16+
*) die "invalid inspection stage" ;;
17+
esac
18+
19+
target="$ROOT/guides/$slug"
20+
[[ -d "$target" && ! -L "$target" ]] || die "invalid target guide directory"
21+
[[ "$(realpath "$target")" == "$ROOT/guides/$slug" ]] || die "invalid target guide directory"
22+
23+
required=(research.md meta.yaml)
24+
if [[ "$stage" != research ]]; then
25+
required+=(external.md speakeasy.md)
26+
fi
27+
for artifact in "${required[@]}"; do
28+
path="$target/$artifact"
29+
[[ -e "$path" || -L "$path" ]] || die "missing required guide artifact"
30+
[[ -f "$path" && ! -L "$path" ]] || die "invalid guide artifact"
31+
done
32+
33+
if [[ "$stage" == research ]]; then
34+
external_present=false
35+
speakeasy_present=false
36+
[[ -e "$target/external.md" || -L "$target/external.md" ]] && external_present=true
37+
[[ -e "$target/speakeasy.md" || -L "$target/speakeasy.md" ]] && speakeasy_present=true
38+
[[ "$external_present" == "$speakeasy_present" ]] || die "incomplete setup artifact pair"
39+
fi
40+
41+
artifacts=()
42+
shopt -s nullglob dotglob
43+
for path in "$target"/*; do
44+
artifact=${path##*/}
45+
case "$artifact" in
46+
research.md|meta.yaml|external.md|speakeasy.md) ;;
47+
*) die "unexpected guide artifact" ;;
48+
esac
49+
[[ -f "$path" && ! -L "$path" ]] || die "invalid guide artifact"
50+
artifacts+=("$artifact")
51+
done
52+
53+
printf '%s\n' "${artifacts[@]}" | sort | jq -Rsc --arg slug "$slug" --arg stage "$stage" \
54+
'{slug:$slug,stage:$stage,artifacts:(split("\n") | map(select(length > 0)))}'

factory/tests/test-coordinator.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ for phrase in \
1414
'bash factory/scripts/inspect-inputs.sh /input/issue.json /input/catalog.json' \
1515
'do not construct another initial-inspection tool program' \
1616
'bash factory/scripts/inspect-guide-context.sh <slug>' \
17+
'bash factory/scripts/inspect-guide-artifacts.sh <slug> research' \
18+
'bash factory/scripts/inspect-guide-artifacts.sh <slug> writer' \
19+
'bash factory/scripts/inspect-guide-artifacts.sh <slug> revision' \
20+
'Do not construct ad hoc artifact-validation commands' \
21+
"Accept only the exact keys \`slug\`, \`stage\`, and \`artifacts\`" \
22+
"Research accepts exactly \`[\"meta.yaml\",\"research.md\"]\` or the full four-file array" \
23+
'Writer and revision require the full four-file array' \
1724
'never inspect repository files directly or run another Phase 1 file-discovery tool' \
1825
'permit only bounded reads of the helper-generated spill artifact' \
1926
'use only installed jq and sed for spill reads; Python is unavailable' \
@@ -48,6 +55,13 @@ done
4855

4956

5057
child_start_contract="$(sed -n '/^## Phase 2/,/^## Phase 5/p' "$CONTRACT")"
58+
phase2_contract="$(sed -n '/^## Phase 2/,/^## Phase 3/p' "$CONTRACT")"
59+
phase3_contract="$(sed -n '/^## Phase 3/,/^## Phase 4/p' "$CONTRACT")"
60+
phase4_contract="$(sed -n '/^## Phase 4/,/^## Phase 5/p' "$CONTRACT")"
61+
grep -Fq 'bash factory/scripts/inspect-guide-artifacts.sh <slug> research' <<<"$phase2_contract" || fail 'research phase does not invoke exact artifact helper'
62+
grep -Fq 'bash factory/scripts/inspect-guide-artifacts.sh <slug> writer' <<<"$phase3_contract" || fail 'writer phase does not invoke exact artifact helper'
63+
grep -Fq 'bash factory/scripts/inspect-guide-artifacts.sh <slug> revision' <<<"$phase4_contract" || fail 'revision phase does not invoke exact artifact helper'
64+
5165
if grep -Eq '(^|[,{[:space:]])(model|harness)[[:space:]]*:|--(model|harness)' <<<"$child_start_contract"; then
5266
fail 'child start contains an explicit model or harness override'
5367
fi
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#!/usr/bin/env bash
2+
set -euo pipefail
3+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
4+
# shellcheck disable=SC1091
5+
source "$ROOT/factory/tests/test-helper.sh"
6+
INSPECTOR="$ROOT/factory/scripts/inspect-guide-artifacts.sh"
7+
TMP="$(mktemp -d)"
8+
trap 'rm -rf "$TMP"' EXIT
9+
REPO="$TMP/repo"
10+
mkdir -p "$REPO/guides/box"
11+
12+
printf '%s\n' dossier >"$REPO/guides/box/research.md"
13+
printf '%s\n' metadata >"$REPO/guides/box/meta.yaml"
14+
FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out"
15+
jq -e '(keys == ["artifacts", "slug", "stage"]) and .slug == "box" and .stage == "research" and .artifacts == ["meta.yaml", "research.md"]' "$TMP/out" >/dev/null
16+
17+
printf '%s\n' partial >"$REPO/guides/box/external.md"
18+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out" 2>"$TMP/err"; then
19+
fail 'research inspection accepted a partial setup artifact pair'
20+
fi
21+
assert_contains 'factory: incomplete setup artifact pair' "$(cat "$TMP/err")"
22+
rm "$REPO/guides/box/external.md"
23+
24+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out" 2>"$TMP/err"; then
25+
fail 'writer inspection accepted missing setup files'
26+
fi
27+
assert_contains 'factory: missing required guide artifact' "$(cat "$TMP/err")"
28+
29+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box revision >"$TMP/out" 2>"$TMP/err"; then
30+
fail 'revision inspection accepted missing setup files'
31+
fi
32+
assert_contains 'factory: missing required guide artifact' "$(cat "$TMP/err")"
33+
34+
printf '%s\n' external >"$REPO/guides/box/external.md"
35+
printf '%s\n' speakeasy >"$REPO/guides/box/speakeasy.md"
36+
FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out"
37+
jq -e '.stage == "research" and .artifacts == ["external.md", "meta.yaml", "research.md", "speakeasy.md"]' "$TMP/out" >/dev/null
38+
FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out"
39+
jq -e '(keys == ["artifacts", "slug", "stage"]) and .artifacts == ["external.md", "meta.yaml", "research.md", "speakeasy.md"]' "$TMP/out" >/dev/null
40+
FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box revision >/dev/null
41+
42+
printf '%s\n' unexpected >"$REPO/guides/box/notes.md"
43+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out" 2>"$TMP/err"; then
44+
fail 'writer inspection accepted unexpected artifact'
45+
fi
46+
assert_contains 'factory: unexpected guide artifact' "$(cat "$TMP/err")"
47+
rm "$REPO/guides/box/notes.md"
48+
49+
rm "$REPO/guides/box/external.md"
50+
ln -s /etc/passwd "$REPO/guides/box/external.md"
51+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box writer >"$TMP/out" 2>"$TMP/err"; then
52+
fail 'writer inspection accepted symlinked artifact'
53+
fi
54+
assert_contains 'factory: invalid guide artifact' "$(cat "$TMP/err")"
55+
56+
rm -rf "$REPO/guides/box"
57+
mkdir "$REPO/real-box"
58+
ln -s "$REPO/real-box" "$REPO/guides/box"
59+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box research >"$TMP/out" 2>"$TMP/err"; then
60+
fail 'inspection accepted symlinked guide directory'
61+
fi
62+
assert_contains 'factory: invalid target guide directory' "$(cat "$TMP/err")"
63+
64+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" '../box' writer >"$TMP/out" 2>"$TMP/err"; then
65+
fail 'inspection accepted unsafe slug'
66+
fi
67+
assert_contains 'factory: invalid guide slug' "$(cat "$TMP/err")"
68+
if FACTORY_REPO_ROOT="$REPO" bash "$INSPECTOR" box review >"$TMP/out" 2>"$TMP/err"; then
69+
fail 'inspection accepted invalid stage'
70+
fi
71+
assert_contains 'factory: invalid inspection stage' "$(cat "$TMP/err")"
72+
73+
printf 'PASS: deterministic guide artifact inspection\n'

0 commit comments

Comments
 (0)