Skip to content

fix(env): verify up-to-date fast path against source, not just the lock#178

Merged
fentas merged 4 commits into
mainfrom
fix/env-source-authoritative-fastpath
Jul 4, 2026
Merged

fix(env): verify up-to-date fast path against source, not just the lock#178
fentas merged 4 commits into
mainfrom
fix/env-source-authoritative-fastpath

Conversation

@fentas

@fentas fentas commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Problem (validated report — the residual #174 couldn't reach)

A consumer hit b update --envs-only reporting "up to date"/"225 keep" while the tree stayed stale — the CSI feature absent from disk after the lock had advanced. Healed only via git archive | tar -x.

Root cause, reproduced in a failing test on v4.18.4: the fast path trusts the lock as ground truth. At an unchanged commit it compares disk vs the lock's recorded shas (#174). A lock whose commit pointer was advanced without its content (legacy corruption from an older version) satisfies that check — disk == lock, both stale vs the pinned commit — so b update skips forever and b verify (disk-vs-lock) is equally blind. Self-sealing: the run that broke it hides the breakage.

S3 state:  disk = old content, lock.sha = old sha, lock.commit = NEW commit
#174 view: disk == lock → "(up to date)"          ← never looks at source

Fix — the skip must prove the tree matches source (user-selected option B: UX over compute)

lockedStateInSync now verifies three layers; any mismatch falls through to a full re-sync (strategy + safety gated, lock re-recorded):

  1. Managed set — current config's glob match against the tree at the pinned commit must equal the lock's file set. Changed files: globs now sync immediately (previously silently deferred until the next upstream commit; the old comment suggested --force, which never existed for envs).
  2. Lock — disk bytes hash to the recorded sha (unchanged, fix(env): re-sync on hash drift at unchanged commit (auto-heal stale lock) #174).
  3. Source — for plainly-synced files (replace, no select, no b.pin), the disk content's git blob OID must match the ls-tree OID at the pinned commit. Computed in-process (gitcache.BlobOID, sha1/sha256 by repo format) — one git ls-tree subprocess total, no per-file spawns; the pinned commit comes from the cache (fetched at most once per machine); on any failure the check degrades to layer 2 rather than failing the update.

Intentional-divergence guards (no re-sync churn, each covered by a test): client/merge strategies, select-filtered files, and files carrying b.pin annotations keep the lock hash as their contract.

Semantics change (deliberate, documented)

Under replace, an interactive "keep" is now one-shot — the next update re-detects the divergence and restores source (previously the keep was silently absorbed until the next upstream commit; same end state, later). Durable local divergence ⇒ strategy: client/merge, select, or b.pin. Docs updated; the stale "b update --force re-syncs" tip in env-sync.mdx is removed.

Heal UX for the reported state

Plain b update: layer 3 detects the stale files → full sync → clean replace (plan rows are non-destructive updates since disk matches the lock) → passes strict/prompt gates automatically → disk + lock re-recorded at the pinned commit. Fully automatic, one run.

Tests

Fail-without-fix (verified by stashing the env.go change): TestSyncEnv_HealsStaleLockAtBumpedCommit, TestSyncEnv_ConfigChangeResyncsAtUnchangedCommit, TestSyncEnv_ReplaceKeepIsOneShot. Guards: PinnedFileFastPathStaysSkippable, ClientStrategyFastPathUnaffected, SelectFileFastPathSkips. gitcache: HasCommit, BlobOID (validated against git hash-object), ls-tree OID parsing. Full go build/vet/gofmt/test ./... clean.

Pushed over HTTPS via the gh token (ssh-agent has no key this session).

🤖 Generated with Claude Code

The "(up to date)" fast path trusted the lock as ground truth: at an unchanged
commit it compared on-disk hashes against the lock's recorded shas (#174) and
skipped when they matched. A lock whose commit pointer was advanced without its
content — e.g. written by an older buggy version — reads as in-sync forever
(lock == disk, both stale relative to the pinned commit), so `b update` kept
printing "(up to date)" while the tree stayed at old content, with no heal path
short of manually re-extracting the files. `b verify` (disk-vs-lock) is equally
blind to this state.

The fast path now verifies three layers, and any mismatch falls through to a
full re-sync (which reconciles via the per-file strategy + safety gate and
re-records the lock):

 1. managed set — the CURRENT config's glob match against the tree at the
    pinned commit must equal the lock's file set. Changed globs/ignore/dest
    now re-sync immediately instead of silently waiting for the next upstream
    commit (the old comment suggested `--force`, which never existed for envs).
 2. lock — on-disk bytes must hash to the recorded sha (unchanged, #174).
 3. source — for files whose bytes are supposed to equal the raw upstream blob
    (replace strategy, no select filter, no local b.pin annotations), the disk
    content's git blob OID must match the ls-tree OID at the pinned commit.
    Computed in-process (gitcache.BlobOID) against a single `git ls-tree`; the
    pinned commit is read from the cache and fetched at most once per machine;
    on failure the check degrades to layer 2 instead of failing the update.

client/merge strategies and select/pinned files intentionally diverge from
upstream, so for those the recorded lock hash remains the contract (guards
prevent re-sync churn; covered by tests).

Semantics change worth noting: under replace, an interactive "keep" now
survives only until the next update (source is authoritative) instead of being
silently absorbed until the next upstream commit. Durable local divergence
belongs to strategy client/merge, select scopes, or b.pin — documented in
env-sync.mdx, which also loses its stale `b update --force` tip.

gitcache: TreeEntry gains Type+OID (ls-tree already emits them), plus
HasCommit and BlobOID helpers.

Tests (each fails without the fix): HealsStaleLockAtBumpedCommit (the reported
"lock advanced but tree didn't" state), ConfigChangeResyncsAtUnchangedCommit,
ReplaceKeepIsOneShot; guards covered by PinnedFileFastPathStaysSkippable,
ClientStrategyFastPathUnaffected, SelectFileFastPathSkips; gitcache HasCommit +
BlobOID vs `git hash-object`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens b update --envs-only’s “(up to date)” fast path so it only skips when the working tree matches not just the lockfile but also (when verifiable) the actual source tree at the pinned commit, preventing self-sealing stale/poisoned lock states.

Changes:

  • Add a 3-layer fast-path validation in env sync (managed-set vs current config, disk vs lock hashes, and disk vs source blob OIDs for plain replace files).
  • Extend gitcache to expose cached-commit presence and surface ls-tree entry type/OID, plus add BlobOID for in-process blob hashing.
  • Add regression/guard tests for stale-lock healing and intentional divergence cases; update env-sync docs to reflect new semantics.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/gitcache/gitcache.go Adds HasCommit, enriches TreeEntry with type/OID, parses ls-tree output, and adds BlobOID to compare disk bytes to source blobs.
pkg/gitcache/gitcache_extra_test.go Adds tests for HasCommit, BlobOID, and ListTreeWithModesDir OID/type surfacing.
pkg/env/env.go Replaces the old “disk vs lock only” skip check with lockedStateInSync including managed-set + source-blob verification (best-effort).
pkg/env/sync_source_verify_test.go Adds regressions/guards for stale-lock-at-bumped-commit healing, config-change resync at unchanged commit, keep semantics, and divergence exceptions.
docs/env-sync.mdx Updates tips to remove the non-existent --force guidance and document self-healing + one-shot keep behavior.

Comment thread pkg/env/sync_source_verify_test.go Outdated
Comment thread pkg/env/env.go
Comment thread docs/env-sync.mdx Outdated
…n gate

Copilot:
- test helper returned rev-parse output truncated to 40 chars, invalid for
  sha256-object-format repos → TrimSpace instead.
- lockedStateInSync slurped every file even when only the lock comparison
  (layer 2) would run. Files outside layer 3's scope (non-replace strategy, no
  source context, select-filtered) now use the streaming lock.SHA256File; the
  full read happens only where the bytes are needed anyway (blob OID + pin
  detection).
- env-sync.mdx self-healing tip now states the fallback: source verification
  needs the pinned commit in the local cache (auto-fetched when missing) and
  degrades to lock-only verification for that run when unavailable.

Subagent:
- mayCarryPins moved to pin.go as the shared gate used by both applyPinsYAML
  and the fast path (was a drift-prone mirror of the same ext+substring check).
- the fast-path comments no longer overclaim "fetched at most once per
  machine": a failed fetch degrades for the run and is retried next update
  (transient failures self-heal; persistent ones cost one attempt per run).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread pkg/env/env.go Outdated
…xemption

Copilot: mayCarryPins is a substring gate and can false-positive on a file
that merely MENTIONS "b.pin" (comment/value). The fast path took that hit as
grounds to skip the layer-3 source check, silently downgrading such a file to
lock-only verification — reintroducing the undetectable stale-lock state this
PR exists to close, for exactly that file.

The exemption now requires structural confirmation: hasActivePins (pin.go)
parses the content and checks collectPinnedPaths only when the cheap substring
gate matches. No real pin → the file stays source-verified. Unparseable
content cannot carry effective pins (applyPinsYAML leaves such files alone),
so it also stays source-verified — which keeps templated .yaml files (invalid
YAML) under source verification too.

Test: TestSyncEnv_PinMentionDoesNotExemptFromSourceCheck — a stale file whose
content mentions b.pin (no structural pin) at a poisoned lock must re-sync and
heal; fails with the substring-only exemption. PinnedFileFastPathStaysSkippable
still green (real pins keep skipping).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread pkg/gitcache/gitcache.go Outdated
Comment thread docs/env-sync.mdx Outdated
… caveat

Copilot verification pass:
- ListTreeWithModesDir now errors on ls-tree lines without the full
  "<mode> <type> <oid>" prefix instead of admitting entries with empty
  Type/OID, which source verification would read as perpetual drift (silent
  re-syncs) rather than a loud parse problem. git always emits three fields;
  anything else is corrupt output. (Also flagged by the round-2 subagent.)
- env-sync.mdx: the offline/cache-miss fallback disables changed-glob
  detection too, not just source drift — say so.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@fentas

fentas commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

Review loop complete — 2 rounds (Copilot + subagent) + verification pass. Round 1 (7627001): streaming lock-only hashes, shared pin gate, sha256-safe helper, honest fetch-retry comments, docs fallback. Round 2 (7750925): structural pin confirmation so a b.pin mention can't exempt a file from source verification. Final (88b1ae8): fail-fast ls-tree parsing, fallback-docs caveat. Fast path now verifies managed set + lock + source blob OIDs; stale-lock states self-heal on plain b update. 4 fail-without-fix regressions + 3 stability guards. CI green. Merging.

@fentas fentas merged commit 05166f3 into main Jul 4, 2026
10 checks passed
@fentas fentas deleted the fix/env-source-authoritative-fastpath branch July 4, 2026 21:29
fentas pushed a commit that referenced this pull request Jul 5, 2026
🤖 I have created a release *beep* *boop*
---


## [4.18.5](v4.18.4...v4.18.5)
(2026-07-04)


### Bug Fixes

* **env:** verify up-to-date fast path against source, not just the lock
([#178](#178))
([05166f3](05166f3))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants