Skip to content

Commit a5bac76

Browse files
Jammy2211Jammy2211
authored
Merge pull request #233 from PyAutoLabs/claude/automind-task-planning-163wk7
fix: pre_build must not stage untracked human work
2 parents 2a4fb11 + ef33156 commit a5bac76

5 files changed

Lines changed: 391 additions & 37 deletions

File tree

‎docs/pre_build_failure_audit.md‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,27 @@ rejection reasoning, confirmed: the line stages nothing anyone needs).
138138
release" vestige should be deleted (recommended — it is #126's mechanism) or
139139
kept deliberately. Deleting it makes releases require clean mains, which
140140
Heart already checks.
141-
- **Open:** atomicity — a mid-sequence fatal leaves a half-pushed release
142-
surface. Worth a fail-fast pre-pass (all repos validated before any push)?
143-
Costed as a follow-up, not this PR.
141+
- **RESOLVED — atomicity.** Was: "a mid-sequence fatal leaves a half-pushed
142+
release surface. Worth a fail-fast pre-pass (all repos validated before any
143+
push)?" Answered yes and implemented: `pre_build.sh` now walks every repo in
144+
`WORKSPACE_SPECS` before the first is touched, aborting if any checkout is
145+
missing or carries untracked files under the directories the run reformats
146+
and stages (`notebooks/`, `scripts/`, `slam_pipeline/`).
147+
148+
The trigger was a near-miss during the 2026-08-07 release: `git add <dir>/`
149+
stages *untracked* files, so an uncommitted script in a workspace's
150+
`scripts/` would be black-formatted and pushed inside the "pre build" commit
151+
— the same leak class as #126, which §3 fixed for `dataset/` and `config/`
152+
while leaving the `scripts/` path open. It was caught only because the
153+
operator moved the file out by hand. Reproduced against the pre-fix script on
154+
fixture repos: the private file was committed and pushed, exit 0, silently.
155+
156+
Staging was narrowed in the same change — `git add -u` for tracked edits and
157+
deletions, plus newly created files added by explicit path — so the
158+
directory-wide form that causes this cannot return. Both legs are covered by
159+
`tests/test_pre_build_staging.py`, which runs the real script against
160+
throwaway git fixtures. There is deliberately **no** `--allow-dirty`
161+
override: an override is precisely the operator vigilance this replaces.
144162

145163
## Trust nothing here
146164

‎pre_build.sh‎

Lines changed: 107 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,89 @@ if command -v gh >/dev/null 2>&1; then
5050
bash "$PYAUTOBASE/PyAutoBrain/bin/ensure_workspace_labels.sh"
5151
fi
5252

53+
# Positional fields: repo project [generate=true] [slam=false]
54+
# Declared as data, not as a call list, because TWO passes read it: the
55+
# uncommitted-work preflight below and the execution loop at the bottom. A
56+
# second hand-maintained list would drift out of step with this one, and the
57+
# preflight would then silently skip a repo it is meant to protect.
58+
# The repo names are checked against PyAutoMind/repos.yaml (the body map) by
59+
# `repos_sync.py --check`; the flags are Build policy and live only here.
60+
# (The former readme_pkg arg / README version bump was deleted per the audit in
61+
# docs/pre_build_failure_audit.md: its sed edit was never staged and the runner
62+
# side was removed under #120. Phase 4 task 4 of the build-chain campaign
63+
# (#155) then resolved the pins themselves: the three surviving `<pkg> vX` lines
64+
# were REMOVED from the READMEs in favour of "install the latest release" plus
65+
# the `version.minimum_library_version` floor, which Heart's version_skew check
66+
# actually verifies. Do not re-add a README version bump here or on the runner —
67+
# an unowned pin is what went 2 months stale.)
68+
# The last entry is the AI assistant repo. No notebook generation; release.yml's
69+
# release_workspaces job stamps its workspace version and regenerates
70+
# wiki/core/api_audit_baseline.json against the released wheels.
71+
WORKSPACE_SPECS=(
72+
"autofit_workspace autofit true false"
73+
"autogalaxy_workspace autogalaxy true false"
74+
"autolens_workspace autolens true true"
75+
"autofit_workspace_test autofit false false"
76+
"autogalaxy_workspace_test autogalaxy false false"
77+
"autolens_workspace_test autolens false false"
78+
"euclid_strong_lens_modeling_pipeline - false false"
79+
"HowToGalaxy howtogalaxy true false"
80+
"HowToLens howtolens true false"
81+
"HowToFit howtofit true false"
82+
"autofit_workspace_developer - false false"
83+
"autolens_workspace_developer - false false"
84+
"autolens_assistant autolens false false"
85+
)
86+
87+
# The directories run_workspace reformats with black and stages. Anything
88+
# untracked under them BEFORE a run is human work, never run output.
89+
MUTATED_DIRS=(notebooks scripts slam_pipeline)
90+
91+
# Preflight: no workspace may carry uncommitted work under MUTATED_DIRS.
92+
#
93+
# run_workspace both black-formats and `git add`s those directories, and both
94+
# operations reach untracked files — so a human's in-progress script would be
95+
# reformatted on disk and pushed inside the "pre build" commit. That is the
96+
# same leak class as the tracked-dataset leak (#126), which was fixed for
97+
# dataset/ and config/ by dropping their staging; the scripts/ path still had
98+
# the hole, and it was caught by hand during the 2026-08-07 release only
99+
# because the operator happened to notice.
100+
#
101+
# This runs over EVERY repo before the first one is touched, mirroring the
102+
# PyAutoHands gate above. run_workspace commits and pushes each repo before
103+
# moving to the next, so a per-repo check that aborted midway would leave the
104+
# earlier repos already published.
105+
echo ""
106+
echo "=== Checking workspaces for uncommitted work ==="
107+
WIP_REPORT=""
108+
for spec in "${WORKSPACE_SPECS[@]}"; do
109+
# `read` rather than `set --`: this loop runs at top level, where `set --`
110+
# would clobber the script's own positional parameters.
111+
read -r wip_repo _ <<< "$spec"
112+
wip_dir="$PYAUTOBASE/$wip_repo"
113+
# Checked here so a missing checkout fails with a clear message during the
114+
# preflight, rather than as a bare `cd` error partway through the run once
115+
# earlier repos have already been committed and pushed.
116+
if [ ! -d "$wip_dir/.git" ]; then
117+
echo "ABORT: $wip_repo is missing or is not a git repo ($wip_dir)." >&2
118+
exit 1
119+
fi
120+
# `ls-files --others` tolerates pathspecs that match nothing (unlike
121+
# `git add`), so the dirs need no per-repo existence guard here.
122+
# `--exclude-standard` honours .gitignore, keeping output/ and friends out.
123+
wip="$(git -C "$wip_dir" ls-files --others --exclude-standard -- "${MUTATED_DIRS[@]}")"
124+
if [ -n "$wip" ]; then
125+
WIP_REPORT="${WIP_REPORT} ${wip_repo}:"$'\n'"$(printf '%s\n' "$wip" | sed 's/^/ /')"$'\n'
126+
fi
127+
done
128+
if [ -n "$WIP_REPORT" ]; then
129+
echo "ABORT: uncommitted work under directories pre_build formats and commits." >&2
130+
printf '%s' "$WIP_REPORT" >&2
131+
echo "Commit, stash or move these before releasing — pre_build must not author them." >&2
132+
exit 1
133+
fi
134+
echo " Clean: no untracked files under ${MUTATED_DIRS[*]} in any workspace."
135+
53136
run_workspace() {
54137
local repo="$1"
55138
local project="$2"
@@ -88,11 +171,24 @@ run_workspace() {
88171
# release commits, which is the mechanism that leaked simulated datasets
89172
# (#126). Releases require clean mains (Heart gates on it); human work is
90173
# committed by humans. See docs/pre_build_failure_audit.md §3/§6 (#156).
174+
local stage_dirs=()
91175
for d in notebooks scripts; do
92-
if [ -d "$d" ]; then git add "$d/"; fi
176+
if [ -d "$d" ]; then stage_dirs+=("$d"); fi
93177
done
94178
if [ "$slam" = "true" ] && [ -d "slam_pipeline" ]; then
95-
git add slam_pipeline/
179+
stage_dirs+=("slam_pipeline")
180+
fi
181+
if [ ${#stage_dirs[@]} -gt 0 ]; then
182+
# Tracked edits and deletions: black's reformatting, regenerated and
183+
# retired notebooks.
184+
git add -u -- "${stage_dirs[@]}"
185+
# Plus what this run CREATED — a new notebook from generate.py. The
186+
# preflight proved these directories held no untracked files before the
187+
# run, so anything untracked now is run output. Added by explicit path
188+
# rather than as `git add <dir>/`, which also sweeps in untracked files
189+
# and would re-open the hole the preflight closes.
190+
git ls-files --others --exclude-standard -z -- "${stage_dirs[@]}" \
191+
| xargs -0 --no-run-if-empty git add --
96192
fi
97193
# Root-level artifacts (llms-full.txt, workspace_index.json, README Colab
98194
# URLs) are produced and committed by release.yml's release_workspaces job
@@ -108,33 +204,15 @@ run_workspace() {
108204
fi
109205
}
110206

111-
# Positional args: repo project [generate=true] [slam=false]
112-
# The repo names are checked against PyAutoMind/repos.yaml (the body map) by
113-
# `repos_sync.py --check`; the flags are Build policy and live only here.
114-
# (The former readme_pkg arg / README version bump was deleted per the audit in
115-
# docs/pre_build_failure_audit.md: its sed edit was never staged and the runner
116-
# side was removed under #120. Phase 4 task 4 of the build-chain campaign
117-
# (#155) then resolved the pins themselves: the three surviving `<pkg> vX` lines
118-
# were REMOVED from the READMEs in favour of "install the latest release" plus
119-
# the `version.minimum_library_version` floor, which Heart's version_skew check
120-
# actually verifies. Do not re-add a README version bump here or on the runner —
121-
# an unowned pin is what went 2 months stale.)
122-
run_workspace "autofit_workspace" "autofit" true false
123-
run_workspace "autogalaxy_workspace" "autogalaxy" true false
124-
run_workspace "autolens_workspace" "autolens" true true
125-
run_workspace "autofit_workspace_test" "autofit" false false
126-
run_workspace "autogalaxy_workspace_test" "autogalaxy" false false
127-
run_workspace "autolens_workspace_test" "autolens" false false
128-
run_workspace "euclid_strong_lens_modeling_pipeline" "" false false
129-
run_workspace "HowToGalaxy" "howtogalaxy" true false
130-
run_workspace "HowToLens" "howtolens" true false
131-
run_workspace "HowToFit" "howtofit" true false
132-
run_workspace "autofit_workspace_developer" "" false false
133-
run_workspace "autolens_workspace_developer" "" false false
134-
# The AI assistant repo. No notebook generation; release.yml's
135-
# release_workspaces job stamps its workspace version and regenerates
136-
# wiki/core/api_audit_baseline.json against the released wheels.
137-
run_workspace "autolens_assistant" "autolens" false false
207+
# Execute. Same list the preflight above walked — see WORKSPACE_SPECS for the
208+
# field meanings and the policy notes.
209+
for spec in "${WORKSPACE_SPECS[@]}"; do
210+
# Unquoted on purpose, as in the preflight: the fields are
211+
# whitespace-separated and none contains a space. A no-generate repo carries
212+
# `-` in the project field — word splitting cannot express an empty field,
213+
# and run_workspace never reads project when generate is false.
214+
run_workspace $spec
215+
done
138216

139217
# Release readiness (version skew, including the version.txt-ahead crash that
140218
# used to be checked here) is now Heart's job, not Build's: PyAutoHands is a

‎skills/pre_build/pre_build.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,11 @@ bash $HOME/Code/PyAutoLabs/PyAutoHands/bin/autohands pre_build <minor_version>
5555
The script handles every mechanical step of the pre-build flow:
5656

5757
1. Fails before any side effects unless PyAutoHands is on clean `main`; the run produces no PyAutoHands files and never stages or commits that repository.
58-
2. Ensures the canonical `pending-release` label exists on each release-window repo.
59-
3. For every workspace, runs black on the staged dirs (`scripts/`, `slam_pipeline/`), runs `generate.py` for projects with a notebook target, and stages only what the run itself produced (`notebooks/`, `scripts/`, plus `slam_pipeline/` for `autolens_workspace`). It does not stage `dataset/` or `config/` — nothing in the run modifies them, and sweeping pre-existing human work into release commits was the #126 leak mechanism. Root-level artifacts and README Colab URLs are committed by `release.yml` on the runner, not here.
60-
4. Commits and pushes each workspace (skipping if no changes are staged).
61-
5. Dispatches `gh workflow run release.yml --repo PyAutoLabs/PyAutoHands --field minor_version=<N>`.
58+
2. Sweeps **every** workspace for untracked files under the directories it reformats and stages (`notebooks/`, `scripts/`, `slam_pipeline/`) and aborts, naming each repo and path, if any exist. This runs before the first repo is touched, because the script commits and pushes each workspace before moving to the next — a check that aborted midway would leave earlier repos already published. Remedy is to commit, stash or move the files; there is deliberately no override flag, since an override is exactly the operator vigilance this replaces.
59+
3. Ensures the canonical `pending-release` label exists on each release-window repo.
60+
4. For every workspace, runs black on the staged dirs (`scripts/`, `slam_pipeline/`), runs `generate.py` for projects with a notebook target, and stages only what the run itself produced (`notebooks/`, `scripts/`, plus `slam_pipeline/` for `autolens_workspace`) — tracked edits and deletions via `git add -u`, plus newly created files by explicit path. It never runs `git add <dir>/`, which also sweeps in untracked files; that is what committed and pushed a human's uncommitted script during the 2026-08-07 release rehearsal, and is the same leak class as #126. It does not stage `dataset/` or `config/` — nothing in the run modifies them. Root-level artifacts and README Colab URLs are committed by `release.yml` on the runner, not here.
61+
5. Commits and pushes each workspace (skipping if no changes are staged).
62+
6. Dispatches `gh workflow run release.yml --repo PyAutoLabs/PyAutoHands --field minor_version=<N>`.
6263

6364
Release-readiness — including the version-skew check that used to run here
6465
(`verify_workspace_versions.sh`) — is gated **upstream by PyAutoHeart**

‎tests/test_pre_build_skill.py‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
def test_pre_build_skill_checks_every_executor_repo():
1111
script = (ROOT / "pre_build.sh").read_text()
1212
body = (ROOT / "skills" / "pre_build" / "pre_build.md").read_text()
13-
executor_repos = set(re.findall(r'^run_workspace "([^"]+)"', script, re.MULTILINE))
13+
# Repos come from the WORKSPACE_SPECS array — the single list that both the
14+
# uncommitted-work preflight and the execution loop read.
15+
executor_repos = set(
16+
re.findall(r'^\s+"(\S+)\s+\S+\s+\S+\s+\S+"', script, re.MULTILINE)
17+
)
1418
fixed_dependencies = set(re.findall(r'\$PYAUTOBASE/([^/"$]+)', script))
1519
preflight = body.split("Check that all required repositories exist", 1)[1]
1620
preflight = preflight.split("For each, verify", 1)[0]
@@ -27,3 +31,33 @@ def test_pre_build_guards_pyautohands_instead_of_staging_it():
2731
assert 'if [ "$HANDS_BRANCH" != "main" ] || [ -n "$HANDS_STATUS" ]' in script
2832
assert script.index(guard) < script.index("=== Ensuring pending-release labels ===")
2933
assert "git add -A" not in script
34+
35+
36+
def test_pre_build_never_stages_a_directory():
37+
"""`git add <dir>/` also stages untracked files.
38+
39+
That is how a human's uncommitted script was reformatted and pushed inside
40+
a "pre build" commit during the 2026-08-07 release. Staging must name the
41+
tracked set (`git add -u`) and add created files by explicit path.
42+
"""
43+
script = (ROOT / "pre_build.sh").read_text()
44+
45+
assert not re.search(r"git add\s+[\"']?\$?\w+/", script)
46+
assert 'git add -u -- "${stage_dirs[@]}"' in script
47+
48+
49+
def test_pre_build_wip_preflight_precedes_every_mutation():
50+
"""The preflight must sweep all repos before the first is touched.
51+
52+
run_workspace commits and pushes each workspace before moving to the next,
53+
so a per-repo check that aborted midway would leave earlier repos already
54+
published.
55+
"""
56+
script = (ROOT / "pre_build.sh").read_text()
57+
58+
preflight = script.index("=== Checking workspaces for uncommitted work ===")
59+
assert preflight < script.index("run_workspace() {")
60+
# The invocations themselves, not the words — both appear in prose above.
61+
assert preflight < script.index('black "$d/"')
62+
assert preflight < script.index("git add -u --")
63+
assert "ABORT: uncommitted work" in script

0 commit comments

Comments
 (0)