fix(macos): a sourced augur could not enumerate its own shares - #165
Merged
Conversation
`macos_share_roots` reads `$MACOS_SHARE` on its first line, but the assignment lived in the dispatch tail, 54 lines BELOW the `AUGUR_SOURCE_ONLY` early return. augur runs `set -euo pipefail`, so in every context that reaches the script through the project's own test seam -- `AUGUR_SOURCE_ONLY=1 source ./augur`, which every `tests/NN_*.sh` uses -- that first line died with `MACOS_SHARE: unbound variable`. An EXECUTED `augur up --macos` was never affected: the assignment runs before dispatch, and the refresher is a plain `( … ) &` subshell rather than a re-exec, so it inherits the binding. Nothing shipped sources the file. The damage was to what could be TESTED and to what could be diagnosed. TWO CONSEQUENCES. The sweep was structurally untestable offline. `_refresh_macos_shares_locked` calls `macos_share_roots`, and every offline test reaches it through the seam, so no test could execute the sweep at all. That is why nothing caught this. And a refresher started from a sourcing context ran dead in a way that looked healthy. The roots are consumed by `done < <(macos_share_roots …)`, a process substitution, so the unbound-variable death killed only the producer subshell. The `while read` loop got no input, `n` stayed 0, and the function took its `n == 0` early return -- which promoted the marker and reported NOWORK. The loop survived, the pidfile stayed live, `share_refresher_running` reported healthy, and nothing was ever msynced. Measured on an Apple Silicon host: 31 hours of it, 1.1 MB of the same error in the refresher log, while `start_share_refresher`'s idempotence (`share_refresher_running && return 0`) made every later `up --macos` leave the live pidfile alone. The live E2E arm "a host edit became visible in the running guest" failed for the full 30 s budget; with the stale refresher cleared, the same arm passes in 5 s. THE FIX, in three parts. `MACOS_SHARE` is bound above the seam, so a sourced augur can reach the sweep. Verified safe: `workspace_slug` reads only `$WORKSPACE_DIR`, which is set at augur:12, and nothing between the two positions reassigns it. `macos_share_roots` now refuses with a named error when `MACOS_SHARE` or `WORKSPACE_DIR` is empty, instead of dying anonymously inside a process substitution the caller cannot see. It is a presence check only, and deliberately so: a mismatched-but-set pair is legitimate (tests/41 pairs `workspace-testslug` with its own root on purpose). The sweep stops conflating "nothing changed" with "nothing was scanned". `n == 0` had two meanings and only one was good news. The failure path now warns, names which of the two happened, and -- the part that matters -- does NOT promote the marker. Promoting it would put every host edit made during the broken window permanently past `find -newer`, so the edits would never be swept even after the bug was fixed. The genuine no-work path is unchanged: quiet, and it still promotes. Tests. tests/41 gains a section that executes the sweep through the seam, which was impossible before this commit, plus arms pinning both marker outcomes -- and they assert the marker's own state, not just the message. Mutation-checked: making the predicate `roots` instead of `scanned`, and making the failure path promote the marker anyway, each turn three arms red. tests/36, tests/38 and tests/e2e_macos_vm.sh carried comments that described the old ordering as load-bearing; those are corrected, no code. Verified: `make offline-tests` ALL GREEN (30 scripts), `make shellcheck` clean, `bash -n augur` OK, and `bash -c 'AUGUR_SOURCE_ONLY=1 source ./augur; macos_share_roots testvm'` now prints the four share rows and exits 0. Not verified here: the live macOS E2E, which needs an Apple Silicon host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🧭 Egress drift reminderThis PR changes the egress core ( If this change alters the egress posture, please:
Non-blocking reminder. It does not verify correctness (that is the egress fail-closed E2E + |
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.
The latent defect behind the
make e2eincident while validating #163/#164.macos_share_rootsreads$MACOS_SHAREon its first line, but the assignmentlived in the dispatch tail, 54 lines below the
AUGUR_SOURCE_ONLYearlyreturn.
augurrunsset -euo pipefail, so every context reaching the scriptthrough this project's own test seam —
AUGUR_SOURCE_ONLY=1 source ./augur, whatevery
tests/NN_*.shuses — died there withMACOS_SHARE: unbound variable.An executed
up --macoswas never affectedWorth stating up front, because the incident makes it look worse than it is: the
assignment runs before dispatch, and the refresher is a plain
( … ) &subshellrather than a re-exec, so it inherits the binding. Nothing shipped sources the
file. The damage was to what could be tested, and to what could be
diagnosed.
Two consequences
The sweep was structurally untestable offline.
_refresh_macos_shares_lockedcalls
macos_share_roots, and every offline test reaches it through the seam — sono test could execute the sweep at all. That is why nothing caught this.
A refresher started from a sourcing context ran dead while looking healthy. The
roots are consumed by
done < <(macos_share_roots …)— a process substitution —so the unbound-variable death killed only the producer subshell. The
while readloop got no input,
nstayed 0, and the function took itsn == 0early return,which promoted the marker and reported NOWORK. The loop survived, the pidfile
stayed live,
share_refresher_runningreported healthy, and nothing was evermsynced.
Measured on an Apple Silicon host: 31 hours of exactly that, 1.1 MB of the same
error in the refresher log, while
start_share_refresher's idempotence(
share_refresher_running && return 0) made every laterup --macosleave thelive pidfile alone. The live E2E arm "a host edit became visible in the running
guest" failed for its full 30 s budget. With the stale refresher cleared, the same
arm passes in 5 s.
The fix
1. Bind
MACOS_SHAREabove the seam, so a sourced augur can reach the sweep.Verified safe rather than assumed:
workspace_slugreads only$WORKSPACE_DIR(set at
augur:12), and nothing between the two positions reassigns it.2.
macos_share_rootsrefuses with a named error whenMACOS_SHAREorWORKSPACE_DIRis empty, instead of dying anonymously inside a processsubstitution the caller cannot observe. Presence check only, deliberately: a
mismatched-but-set pair is legitimate (
tests/41pairsworkspace-testslugwithits own root on purpose).
3. Stop conflating "nothing changed" with "nothing was scanned."
n == 0hadtwo meanings and only one was good news.
Important
The failure path does not promote the marker. Promoting it would put every
host edit made during the broken window permanently past
find -newer, so thoseedits would never be swept even after the bug was fixed. The genuine no-work
path is unchanged — quiet, and it still promotes.
The warn also names which of the two happened, so the next occurrence is one
line to diagnose rather than a log dig.
Tests
tests/41gains a section that executes the sweep through the seam — literallyimpossible before this commit — plus arms pinning both marker outcomes. They assert
the marker's own state, not just the message.
Mutation-checked, twice, independently:
rootsinstead ofscannedthe skipped edit is swept by the next sweepThat second one is the property that matters: it fails precisely when edits made
during a broken window would be lost.
tests/36,tests/38andtests/e2e_macos_vm.shcarried comments describing theold ordering as load-bearing. Those are corrected — comments only, no code.
Verified / not verified
Verified here, on this branch rebased onto
4cf2a60(so alongside #163 and #164):make offline-testsALL GREEN (30 scripts),make shellcheckclean,bash -n augurOK, both mutants above, and:which died with
MACOS_SHARE: unbound variablebefore this change.Not verified here: the live macOS E2E, which needs an Apple Silicon host. Since
an executed
up --macoswas never on the broken path, this is not expected tochange any live arm — but
mainis the release gate, so it is worth one run.🤖 Generated with Claude Code