fix: audit and eliminate 23 of 25 dryrun baseline exemptions#118
Merged
Conversation
Two categories of fix:
1. Real dry-run bugs (state written despite --dry-run):
- install.go: saveSyncSourceIfRemote called unconditionally after
installer.RunContext succeeds; gate with !installCfg.DryRun
- install.go: updateSyncedAt called in the "already up to date" path
before the dry-run check; gate identically
2. Structural: add dryRun bool to helper functions so the archtest can
see the guard mechanically, removing the need for baseline entries:
- dotfiles: backupConflicts, backupFile, restoreFile
- shell: patchZshrcBlock
- snapshot/local: SaveLocal
- sync/source: SaveSource, DeleteSource
For the remaining two exemptions (doctor.go read-only diagnostic probes)
and the seven capture.go + one diff.go read-only system probes, the
latter group is now handled by adding those files to dryRunExemptFiles
(all their RunCommandOutput calls are read-only queries). The doctor
entries are kept in the baseline with explicit audit comments.
Baseline: 25 entries → 2 (only doctor read-only probes remain).
fullstackjam
force-pushed
the
fix/dryrun-baseline-audit
branch
from
8afe0b2 to
c3ed3b6
Compare
The dry-run baseline regeneration (ARCHTEST_UPDATE_BASELINE=1) stripped the hand-authored exemption reasons from no-raw-http.txt as a side effect. That file is unrelated to this PR's dry-run audit, so revert it to main: the two entries and line numbers are unchanged, only the audit-trail comments explaining why each raw-http site is exempt are restored.
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.
Summary
install.go:saveSyncSourceIfRemotewas called without a dry-run gate afterinstaller.RunContextsucceeds (soopenboot install @user/slug --dry-runwas writing~/.openboot/sync_source.json);updateSyncedAtin the "already up to date" path was called before the dry-run check.dryRun boolto helper functions that were previously exempted as "caller gated":backupConflicts/backupFile/restoreFile(dotfiles),patchZshrcBlock(shell),SaveLocal(snapshot),SaveSource/DeleteSource(sync). Each function now self-protects; a future caller without a gate will be caught by archtest.dryRunExemptFiles:snapshot/capture.go(brew list, npm list, defaults read, git config --get, version probes) andsync/diff.go(git remote get-url). All calls in those files aresystem.RunCommandOutputread-only queries — they belong in the exempt-files list, not the violation baseline.Baseline: 25 entries → 2.
Test plan
make test-unitpasses (all archtest rules green, all L1 unit + integration tests pass)go vet ./...cleanTestDryRunGuardpasses with only the 2 documented doctor entries remainingfalseexplicitly, maintaining current semantics)