Pre-flight gates so a provincial rerun cannot silently skip its work (#246 Phases 1-2) - #248
Merged
NewGraphEnvironment merged 12 commits intoAug 31, 2026
Merged
Conversation
Memory audit of link — 6 files, 148 lines. Classification: | Action | Count | Files | |---|---|---| | skip — canonical doc exists | 4 | bcfp-vw-column-coding, study-areas-peace-fraser-skeena, study-area-run-cypher-ops, crossings-built-from-primitives | | skip — canonical, kept as recall trigger | 1 | downstream-guard-path-not-membership | | keep in memory — infrastructure identity | 1 | m1-link-db-env | | migrate to research/ | 0 | — | | migrate to CLAUDE.md | 0 | — | Nothing migrated, and that is the correct outcome rather than a miss: every durable fact in this repo's memory already has a canonical home. RUNBOOK.md carries the mechanics, research/ carries the study-area and parity write-ups (21 files), and CLAUDE.md carries status. The memory files are recall triggers pointing at those, which is what they should be. `m1-link-db-env` stays in memory permanently under step 3c — it holds host names, ports, credentials and tunnel state. Migrate the behaviour, not the identity. Two fixes rather than migrations: - This commit: the version header said v0.45.1 / 2026-08-06 while DESCRIPTION reads 0.46.0. Same drift the 2026-08-06 audit fixed, recurring because nothing ties the header to DESCRIPTION. - Machine-local (not in this diff): `study-areas-peace-fraser-skeena` still described drainage closure as `wscode_ltree` ancestry via `public.wsg_outlet`. That method over-includes and the table was dropped on 2026-08-28 (#227), so the memory was actively misleading — corrected in place to point at `lnk_wsg_resolve()` and RUNBOOK §8b. Destination note for a future run: `docs/` is a gitignored pkgdown artifact here; the established durable-docs surface is top-level `research/`. Refs soul#47 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Phases 1-2 (unblock cyphers + pre-flight gates) plus the bucketing derivation. The destructive wipe, the paid 4-host run and the provenance audit are a separate session driven by the merged script. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
The cyphers ran fresh 0.31.0 — which exports neither frs_wsg_drainage nor frs_wsg_outlets — because link's DESCRIPTION declares fresh in Suggests and pak::local_install() defaults to dependencies = NA. The Remotes pin at DESCRIPTION:34 was therefore never resolved. Since 20 files call fresh:: with no requireNamespace() guard, fresh is a hard runtime dependency that was mis-declared as optional. Moving it to Imports makes the existing pin load-bearing with no change to the pak call: upgrade = FALSE suppresses gratuitous upgrades, not required ones, and 0.31.0 does not satisfy the >= 0.33.0 floor. lnk_preflight_fresh() then asserts symbols rather than a version string, a proxy that fails both ways. A drift guard walks link's own namespace — not the R/ directory, which does not exist in an installed package — so a new fresh:: call site cannot ship undeclared. Verified by restoring the bug. Also fixes the prep sentinel, wrong in both directions. study_area_run.sh grepped for "snapshot_bcfp.sh: complete", which cypher_prep.sh's tail -5 logs BEFORE lnk_persist_init runs: a persist_init FATAL exited 1 with the sentinel already present, so the gate passed and WSGs ran against a half-prepped cypher. Meanwhile the snapshot's legitimate skip path never emits it, so a skipped load read as FATAL. Anchored "=== READY" is correct in all four measured cases. Relates to #246, #183 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Splits pre-flight into two blocks answering two different questions. A
cypher's software is predictable from the dispatcher before it exists — its
link comes from git reset --hard origin/$BRANCH, its fresh from link's
DESCRIPTION — so preflight_local() validates what the workers are going to
get, and preflight_hosts() confirms after prep that they got it. Predict
before spend; verify before write.
preflight_local(): dispatcher fresh completeness, branch pushed (fetching
first, since @{upstream} is a local ref and skipping the fetch makes the
check a false green), worktree clean, fwapg SHA resolvable and exported,
primitive vintage, and both DO credentials.
The credential probe is the substantive fix. doctl and tofu use different
tokens; both were minted 2026-05-18 and both expired 2026-08-30, and only
the first was checked. `tofu plan` cannot serve as the second probe: against
a zero-resource workspace it returns "Plan: N to add" without contacting DO
at all. The tfvars token is now sent to /v2/account directly, with 401, an
unreachable API, and any other status reported distinctly.
Vintage lives in R because the load-bearing parts are what testthat can
prove and shell cannot: absent is not fresh, a NULL timestamp is not fresh,
and an empty result set is not a pass. last_analyze alone is unusable —
measured NULL on all ten primitives — so the query takes GREATEST of it and
last_autoanalyze.
Parity keys on repo_sha, not link_sha. link_sha is a real SHA on the
load_all dispatcher and NA on every pak-installed cypher, so comparing it
would fail every legitimate run; fresh_sha is NA on both, so comparing it
would be a vacuous NA == NA pass. repo_sha is read on each host from the
checkout it installed from, which is evidence rather than a restatement of
what the dispatcher believes.
Two post-conditions. Every host must account for its whole bucket before
consolidate, and every run WSG must have rows in the persist after. Both
matter because schema_consolidate DELETEs the destination bucket before it
COPYs: a host that produced nothing does not merely fail to add rows, it
removes the rows already there and returns ok = TRUE.
No global bypass. --preflight-note= downgrades only vintage and parity, and
only with a written justification, mirroring lnk_wsg_downstream_check().
--auto-install is remediation, not a skip: it re-runs the install stage and
re-checks once.
Every gate exercised in both directions on m1 with no spend; the table is in
planning/active/findings.md.
Relates to #246, #183
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
data-raw/study_area_buckets.R partitions the 96 focal watershed groups into drainage-independent components by union-find over per-WSG fresh::frs_wsg_drainage() closures, resolves each through lnk_wsg_resolve() for the species filter and downstream-first order, and packs the components onto hosts by greedy LPT. Components rather than study areas because overlapping closures make schema_consolidate last-writer-wins on the shared WSGs — whichever host finished last would silently decide the answer. The script asserts disjointness rather than assuming it, and refuses an empty closure rather than treating it as an isolated component. Not wscode_ltree roots: that reproduces the sliver misclassification RUNBOOK 8b documents, filing NATR under Fraser though it drains to the Peace and SPAT under Skeena though it drains the Stikine. Reproduces the issue's asserted numbers — 125 closure, 119 modelable, 22 components, 39 on the dispatcher, and the same six WSGs dropped by species presence. The cypher split is 27/26/27 rather than 28/24/28 because weight is stream-segment count rather than WSG count; the components are identical and only the packing moves. The focal set is baked in rather than read from fresh.streams, so the answer does not change when that schema is wiped. research/study_areas.md is generated by --write and two runs are byte-identical. Relates to #246 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
R CMD check: 0 errors. The 3 warnings and 2 notes are all pre-existing — non-ASCII in 13 files, four Rd cross-reference gaps, two undocumented arguments, the top-level directory note, and unqualified setNames / getFromNamespace in older code. None of the four new lnk_preflight_* files appears anywhere in the check output. lintr: the package already carried 350 indentation_linter hits as prevailing house style. The new files add 15 of that same class and zero of any other. Relates to #246 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Round-1 review found a real bug in the parity gate, of exactly the class
the gate exists to prevent. collect_stamps() writes the host stamps to a
TSV and judge_stamps() read them with utils::read.delim(), whose default
na.strings = "NA" turned the deliberate "NA" sentinel back into a real NA.
`%in% c("NA","")` does not match that and `!=` silently drops it, so a run
with fwapg_sha unresolved on EVERY host printed "host parity clean" and
proceeded — the precise provenance hole #246 exists to close.
The test written to guard that seam did not cross it: it built the frame in
R. Reproduced, fixed on both sides (na.strings = character(0) in the reader,
and the judge now treats a real NA as unresolved and normalises before
comparing, so any future reader is judged correctly), and covered by a test
that writes the file and reads it back the way the shell does. Verified by
restoring the bug.
Four more, all found by the same review:
- The "second layer" schema-collision guard was unreachable: it ran only
when --schema= was absent, and the guard above had already exited for
every non-bcfishpass config in that case. Removed rather than left as
decoration — a guard that cannot go red reads as coverage.
- cypher_prep's ~/.Renviron filter used `|| true`, which cannot tell grep's
exit 1 (nothing matched) from >= 2 (read error). Since `>` truncates
first, an error would have replaced ~/.Renviron with the empty file,
destroying the settings the filter exists to preserve.
- wsgs_run_pipeline.sh never passed CYPHER_PREP_BRANCH, so it installed
main's link while executing the feature branch's cypher_prep.sh — which
now calls a function main's link does not have. Every cypher would FATAL
at prep. study_area_run.sh already passed it; the two callers had drifted.
- A bare --preflight-only silently skipped both credential probes and the
branch check, all gated on N_CY > 0, while the header advertised them. It
now reports what it did NOT check.
Also restricted the LNK_PREFLIGHT_DO_URL test seam to https so a stray value
cannot send a live bearer token somewhere in plaintext.
Relates to #246
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Round 2 concentrated on round 1's fixes, and the highest-severity finding was
sitting inside one of them.
The ~/.Renviron guard added in round 1 was unreachable AND fatal. Under
`set -euo pipefail` a bare `grep -vE ... > tmp` aborts the script the moment
grep selects no lines (exit 1), so `RC=$?` and the FATAL branch could never
run — and "no lines matched" is the ORDINARY case: an empty ~/.Renviron on a
fresh droplet, or any re-prep of a host holding only the three owned keys.
Prep would have died with no message after git reset and pak::local_install,
the umbrella would have FATALed on the missing sentinel, and the spin cost
was already paid. `|| RC=$?` makes the guard reachable and the normal case
survivable. Verified against an empty file and a file with unrelated keys.
Second: the STAMP_COLS invariant was enforced only by two lists happening to
agree, and the test that claimed to guard it compared R against R — nothing
read the shell script, so it could not have failed. Dropping a field from
lnk_preflight_stamp() would have left read.delim silently left-shifting the
remaining columns and padding the last, letting a cypher on a different
commit pass as "parity OK". Removed the duplication rather than testing
around it: judge_stamps now calls .lnk_preflight_stamp_cols() for col.names,
so there is no second list to drift. The stale comments that referenced
STAMP_COLS are corrected, including the test comment that overclaimed.
Third: `git fetch ... || true` defeated the gate whose own comment says the
fetch is load-bearing. A failed fetch leaves the stale local @{upstream},
prints "origin/$BRANCH is at HEAD", and the run spins droplets before
cypher_prep's own reset discovers the problem. Now a failed fetch fails.
Relates to #246
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
…246) The completeness gate added in Phase 2 exited before consolidate while CYPHERS_UP=1, so the EXIT trap burned every cypher — destroying the WSGs that had succeeded. One failed WSG on one host would have thrown away a whole paid run, which is verbatim the accident the soft-fail comment sixty lines above exists to prevent. I added a data-loss path while adding a data-loss guard. The fix removes the reason the abort was there. Each host's consolidate bucket is narrowed to the WSGs it actually reported, and since schema_consolidate DELETEs its bucket before COPYing, a bucket holding only WSGs that are about to be re-COPYed cannot delete anything it does not replace. The successful work lands, and the gap surfaces at the coverage post-condition after the burn — where it costs nothing. A host that reported nothing is skipped rather than handed an empty bucket (which would make schema_consolidate stop() and take the other hosts with it), and zero usable sources gets its own branch so `list()` cannot read as "consolidated" when nothing was. bucket_done() matches the WSG code rather than the surrounding prose, so a reworded cat() in wsg_run_one.R degrades to "this host reported nothing" — handled loudly — instead of to a wrong set. Two more: - cypher_prep's ~/.Renviron rewrite ran at the ambient umask, so a 0600 file came back 0644 and the intermediate was world-readable at a predictable path. That file carries PG_PASSWORD_SHARE. Reproduced 0600 -> 0644, fixed with umask 077 around the block plus an explicit chmod. - burn_cyphers piped doctl straight into grep, so a doctl failure produced no output, grep matched nothing, and a leaked droplet billing indefinitely was reported as "✓ no cypher droplets". Three outcomes now, not two. Relates to #246 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Round 4, scoped narrowly to round 3's fixes, found the third consecutive
blocker sitting inside the previous fix — and it was the same class I had
just fixed in cypher_prep.sh without sweeping the diff for other instances.
`bucket_r=$(... | grep -v '^$' | ...)` on an empty reported-bucket: grep
exits 1, pipefail propagates it out of a plain assignment, and set -e aborts
ONE LINE ABOVE the skip branch written for exactly that case. Both new
guards were unreachable, and the abort ran with CYPHERS_UP=1 — so the very
scenario round 3 fixed still burned the droplets and destroyed the work that
had succeeded. `sed` on a missing log file aborted the same way.
Rather than remember the safe form at each call site, it now lives in one
place: csv_lines() / csv_count(), built on sed (which exits 0 having deleted
every line) instead of grep (which does not). bucket_done() refuses an
unreadable log rather than letting sed abort.
Writing the test for that path then surfaced a bug in the helper itself:
`printf '%s'` emits no trailing newline, so `wc -l` counted separators and
reported one item fewer than there were — a host that completed its whole
bucket would have been reported incomplete. Caught by reading the probe's
own output ("job1 expected=1" for a two-element bucket), not by reasoning.
Fixed and checked across six inputs including empty, single, and blanks.
Third finding: `complete_fail` was set, warned about once, and never read,
and the coverage post-condition could not serve as the backstop I claimed.
It asserts rows EXIST, not that they are from this run — the persist
accumulates and consolidate's DELETE is bucket-scoped, so a WSG excluded
from a narrowed bucket keeps its previous run's rows and passes. The run now
carries RUN_INCOMPLETE to the very end and exits non-zero AFTER consolidate,
recompute and compare have written their artifacts, so the operator gets
both the output and an honest exit status.
Relates to #246
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Round 5, scoped to round 4's fixes, found nothing — verified by restoring each of the three fixes (each returns its failure) and by exercising the new helpers across 16 inputs including globs, quotes, leading dashes and unset args under set -u. First round in five that did not contain a defect of the class it was written to remove. It noted two surviving `grep -v '^$'` instances. Both are provably unreachable today, since study_area_wsgs.R stop()s on an empty resolve and a bucket therefore cannot be blank. Swept anyway: this exact form has aborted three different code paths across three rounds, and an unsafe idiom left in the file is what gets copied next. Relates to #246 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
Asked whether the cypher split respects WSG closure. It does, on all four properties I could check: component raw closures are pairwise disjoint (0 overlapping pairs), every host bucket is drainage-closed (0 violations), DS-first order holds within each host across concatenated components (0 violations), and none of the six species-dropped WSGs carries a blocking dam, so the one gap the species filter opens is benign. The first two followed from packing whole components whose closures are disjoint — but "follows by construction" is a claim, and four of the five review rounds on this branch found that kind of claim to be wrong. Both are now asserted in the generator against real closures rather than derived. The ordering check is the one worth having explicitly: a host can hold several components, and its bucket is their DS-first lists concatenated. That is only safe because no flow path crosses a component boundary, which is now checked rather than assumed. It resolves closures for every bucket member, not just the focal ones, since a bucket holds closure members that were never focal. Both verified by restoring the bug — moving one WSG off its component's host reports "host 1 bucket is not drainage-closed - missing HARR", and reversing a host's order reports "TAKL is ordered before its downstream LFRA, HARR, ...". research/study_areas.md is byte-identical, so the assertions verify without changing the output. Relates to #246 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Phases 1–2 of #246, plus the bucketing derivation. Not the run: the
destructive multi-host wipe, the paid 4-host run and the provenance audit are
deliberately a separate session, so the run is driven by merged, reviewed code
and the gates get exercised on a real run rather than being their own test.
A study-area run today would silently skip 80 of 119 WSGs and exit 0.
The root cause was a declaration, not a missing install line
The cypher image bakes
fresh0.31.0, which exports neitherfrs_wsg_drainagenorfrs_wsg_outlets.DESCRIPTION:34already pinnedNewGraphEnvironment/[email protected]— but the pin was never resolved, becausefreshsat in Suggests andpak::local_install()defaults todependencies = NA, hard dependencies only. Meanwhile 20 files callfresh::with zero
requireNamespace()guards, including a default argument on anexported function. So fresh is a hard runtime dependency that was declared
optional, and
library(link)loads cleanly on a host whose fresh cannot runthe pipeline.
Moving it to Imports fixes this with no change to the
pakcall:upgrade = FALSEsuppresses gratuitous upgrades, not required ones.The failure was silent in the worst way —
wsg_run_one.Rcatches themissing-symbol error and
quit(status = 1)s, the bucket loop logs[WARN]and continues, and the host exits 0 with nothing persisted.
Two corrections to the issue, folded into its body: the floor is
v0.33.0, not "newer than 0.31.0" (
frs_wsg_drainagearrived in v0.32.0,frs_wsg_outletsonly in v0.33.0); and the proposedmax(last_analyze)vintage gate is unimplementable — measured NULL on all ten primitives,
which in bash reads as "nothing to see".
Pre-flight, in two blocks
A cypher's software is predictable from the dispatcher before the cypher
exists: its link from
git reset --hard origin/$BRANCH, its fresh fromDESCRIPTION. Sopreflight_local()validates what the workers are going toget — free, pre-spin — and
preflight_hosts()confirms after prep that theygot it. Predict before spend; verify before write.
Three things that looked like checks and were not
tofu planproves the DO tokenPlan: N to addwithout contacting DO. And the tfvarsdo_tokenis a different credential from doctl's — both minted 2026-05-18, both expired 2026-08-30link_shaacross hostsload_alldispatcher,NAon every pak-installed cypher, so it can only fail.fresh_shaisNAon both, so it can only pass. Keys onrepo_sha, read on each host from its own checkoutsnapshot_bcfp.sh: completeis emitted beforelnk_persist_initruns, so a persist_init FATAL passed the gate and WSGs ran against a half-prepped cypher. The skip path never emits it, so a skipped load read as FATAL. Anchored=== READYis right in all four measured casesNo global bypass.
--preflight-note="<why>"downgrades only vintage andparity, and only with a written justification, mirroring
lnk_wsg_downstream_check(override=).--auto-installis remediation, not askip.
--preflight-onlyruns every local gate with zero spend.Post-conditions
schema_consolidateDELETEs the destination bucket and then COPYs, so ahost that produced nothing does not merely fail to add rows — it removes
the rows already there and returns
ok = TRUE. Under #246 the run wasdeleting data and reporting success. Every host must now account for its
bucket, and every run WSG must have rows in the persist afterwards.
Bucketing is derived, not asserted
data-raw/study_area_buckets.R— union-find over per-WSGfrs_wsg_drainage()closures, greedy LPT over the resulting components,writing
research/study_areas.md. It asserts disjointness rather thanassuming it, because overlapping closures make consolidate last-writer-wins.
Reproduces the issue's numbers from first principles: 96 focal → 125
closure → 119 modelable, 22 components, 39 on the dispatcher, and the same
six groups dropped by species presence. (Cypher split is 27/26/27 vs the
issue's 28/24/28 — the components are identical, only the packing moves,
because weight is stream-segment count rather than WSG count.)
Verification
Every gate exercised against both known answers on m1, no spend:
--vintage-max-days=7→ all four named with ages=200→✓ oldest 99.5 d--config=default→ FATAL--config=default --schema=fresh_defaultdo_token✗ HTTP 401✓ HTTP 200✗ unreachable(distinct from 401)✗ no upstream✓ at HEAD✗; dirty, no cyphers →WARNfwapg_sha→ FAILTests
FAIL 0 | PASS 1603(+90).R CMD check0 errors; the 3 warningsand 2 notes are pre-existing and name no new file. lintr: the package already
carried 350
indentation_linterhits as house style; the new files add 15 ofthat class and zero of any other.
/code-check: five roundsna.stringsseam — parity printed "clean" withfwapg_shaunresolved on every host~/.Renvironguard was unreachable and killed prep on every fresh dropletCYPHERS_UP=1, so the trap burned cyphers and destroyed work that had succeededgrep-under-set -eclass as round 2Rounds 1→4 each landed a blocker inside the previous fix. The mechanism was
not carelessness about the rule — the rule was written in the comment directly
above each defect — it was fixing one instance of a class without sweeping
for the others.
grepexiting 1 underset -euo pipefailaborted threeseparate paths across three rounds. Ended by replacing the remembered form
with
csv_lines()/csv_count(), which cannot be got wrong, and sweepingthe two remaining instances even though both are provably unreachable today.
One bug was caught by reading a probe's own output rather than by review:
printf '%s'emits no trailing newline, sowc -lcounted separators and ahost that completed its whole bucket would have been reported incomplete.
Follow-ups
fresh.snapshot_stampso vintage is a recorded fact ratherthan an inference from
pg_stat_user_tables.two DO credentials belong to different accounts,
cypher_up.shcreatesdroplets in account A while
burn_cyphersverifies against account B andreports clean while they bill. Say the word and I'll file it.
Relates to #246 and #183. This PR does not complete #246 — Phases 3–5
(the wipe, the paid run, the provenance audit) are still outstanding, so
the issue stays open after merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_014kSEJoad4Bc553qU8eFiE6