Skip to content

Hash stamps with git hash-object, not shasum - #1

Merged
cleiter merged 1 commit into
cleiter:mainfrom
camerontaylor:fix/stamp-digest-git-hash-object
Sep 8, 2026
Merged

Hash stamps with git hash-object, not shasum#1
cleiter merged 1 commit into
cleiter:mainfrom
camerontaylor:fix/stamp-digest-git-hash-object

Conversation

@camerontaylor

Copy link
Copy Markdown
Contributor

stamp_digest is the one place still shelling out to shasum. decision_key already carries the reason not to:

git hash-object rather than shasum/sha1sum/openssl: it is the one hashing tool guaranteed to be present, since desvio cannot run at all without git.

worktree_fingerprint follows that rule too. This applies it to the third site.

Why it matters

shasum ships with macOS Perl. It is absent on stock Arch and on minimal container images — and its absence is silent, not loud:

  • the command substitution yields an empty digest
  • a missing stamp file also reads as the empty string
  • so stamp_changed compares "" against "" and reports unchanged

The guarded hook then skips its work on every build, starting with the first one on a virgin worktree — a cache announcing a hit before it has ever been populated.

How I hit it

Arch, using examples/paseo unmodified. desvio_install printed:

[desvio] dependencies current — skipping npm ci

into a build tree with no node_modules at all. The build then failed four steps later inside packages/protocol:

Error: Cannot find module 'zod-aot'

which reads as a Paseo problem rather than a desvio one. That indirection is most of the cost.

Before / after

With no stamp file present:

digest stamp_changed
before `` unchanged ❌
after 3996615849f1116480b0e9ee7390769bafdc8f1f changed ✅

Notes

  • git hash-object --stdin < "$f" rather than passing the path, so the bytes are hashed as-is and no clean filter can alter the digest.
  • Existing stamps are invalidated once by the algorithm change, costing one extra run of each guarded hook. No other behaviour changes.
  • I did not touch the fail-open comparison in stamp_changed — with git guaranteed present the empty-digest path disappears, and the comment above stamp_digest says turning a cache miss into a failed build is deliberately the wrong trade. Happy to add a guard if you want belt and braces.

tests/run.sh: 464 passed, 0 failed, unchanged from baseline. Linux only here — I have no macOS box to confirm on.

🤖 Generated with Claude Code

stamp_digest was the one place still shelling out to shasum. decision_key
already carries the reason not to:

  git hash-object rather than shasum/sha1sum/openssl: it is the one hashing
  tool guaranteed to be present, since desvio cannot run at all without git.

worktree_fingerprint follows that rule too. This applies it to the third site.

shasum ships with macOS Perl but is absent on stock Arch and on minimal
container images, and its absence is silent rather than loud: the command
substitution yields an empty digest, a missing stamp file also reads as the
empty string, so stamp_changed compares "" against "" and reports
'unchanged'. The hook then skips its work on every build — including the
first one on a virgin worktree, where the cache announces a hit before it
has ever been populated.

Observed on Arch with the examples/paseo config: desvio_install printed
'dependencies current - skipping npm ci' into a build tree with no
node_modules at all, and the build failed four steps later in
packages/protocol with 'Cannot find module zod-aot'.

  Before, with no stamp file:  digest '', stamp_changed -> unchanged
  After,  with no stamp file:  digest 3996615849, stamp_changed -> changed

git hash-object --stdin rather than passing the path, so the bytes are
hashed as-is and no clean filter can change the digest.

Existing stamps are invalidated once by the algorithm change, costing one
extra run of each guarded hook. tests/run.sh: 464 passed, 0 failed.

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

stamp_changed still discards stamp_digest’s exit status, so a failed digest can collapse to an empty string and produce a false “unchanged” result.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates the stamp hashing mechanism in lib/common.sh to avoid relying on shasum, which can be absent on some Linux distros / minimal images and fail silently, leading to incorrect cache “hits” and skipped work.

Changes:

  • Replace shasum -a 256 usage in stamp_digest with git hash-object --stdin.
  • Expand inline documentation explaining why git hash-object is used for stamps and the failure mode being addressed.
File summaries
File Description
lib/common.sh Switches stamp digest computation from shasum to git hash-object and documents the rationale/failure mode.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/common.sh
Comment on lines 308 to +313
if [ -f "$f" ]; then
printf '%s\0%s\0' "$f" "$(shasum -a 256 < "$f" | cut -d' ' -f1)"
printf '%s\0%s\0' "$f" "$(git hash-object --stdin < "$f")"
else
printf '%s\0absent\0' "$f"
fi
done | shasum -a 256 | cut -d' ' -f1 )
done | git hash-object --stdin )
Comment thread lib/common.sh
Comment on lines +295 to +296
# git hash-object rather than shasum, for the reason already given above
# decision_key: it is the one hashing tool guaranteed to be present, since
@cleiter
cleiter merged commit 9a16736 into cleiter:main Sep 8, 2026
@cleiter

cleiter commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Thanks for your contribution. Sorry for the delay, seems like I missed the notification.

cleiter added a commit that referenced this pull request Sep 8, 2026
…lsof

Issue #2, companion to #1: desvio_preflight resolved a running daemon's
cwd with lsof alone. lsof is absent by default on Arch and on most
minimal images, and its absence is silent — the command substitution
yields an empty digest, the case statement matches no branch, and the
hook returns success. The guard whose entire job is to refuse a build
under a live daemon then permits it quietly, starting with the first
build on a virgin worktree, and a rebuild rewrites dist/ beneath a
process that lazily requires from it.

Two more sites in start.sh share the lookup and are not merely
degraded reporting: the "already serving this tree" check falls
through to a swap it shouldn't make, and the post-start proof kills
the daemon it just started and blames "something else won the port"
when the real cause is a missing lsof.

Fix: paseo_daemon_cwd(), duplicated in desvio.conf and start.sh per
the existing convention for the pidfile parse (a shared source would
break the "lift one script at a time" design these files already
document) — /proc/$pid/cwd first, lsof as the fallback, probing
/proc/self/cwd rather than the target's own entry so a daemon owned by
another user still reaches lsof instead of being misread as "no
procfs". desvio_preflight now treats a live daemon with an
undeterminable cwd as a refusal, with a message naming both remedies
(install lsof, or stop the daemon) — no escape-hatch env var, since a
documented way to turn the guard off is a way to leave it off.

start.sh had a second, independent bug at the same root: the
confirmation prompt and the actual `daemon stop` were gated on
"do we know its cwd" instead of "is a daemon live". With lsof missing
that skipped the stop entirely, started a second daemon against a
port the first one still held, and then read the OLD daemon's pid back
out of the pidfile for the proof. Introduced CURRENT_PID alongside
CURRENT_CWD so liveness and tree-knowledge are tracked separately, and
moved the prompt, the live-agent listing, and the stop itself onto
CURRENT_PID. The post-start proof, when the new daemon's tree can't be
verified, now warns and keeps it rather than killing a daemon that
answered `daemon status` just fine — killing it over an unrelated
introspection gap would leave the user with nothing and the wrong
diagnosis.

install.sh is untouched: it's macOS-only by construction (ships a .app
into /Applications, reads Info.plist via plutil), and lsof is part of
the macOS base system, so the absence this issue describes can't occur
there.

tests/test-daemon-cwd.sh: paseo_daemon_cwd against real processes,
unstubbed, so CI's matrix exercises both branches (/proc on Linux,
lsof on macOS) without a conditional in the test; desvio_preflight's
four outcomes with the helper stubbed — cwd inside the tree, cwd
elsewhere, cwd undeterminable (the regression this fixes), and no
pidfile — plus the <tree>-other sibling-prefix guard.

Verified: 538 passed/0 failed (527 baseline + 11 new), both under
default bash and under /bin/bash (macOS system 3.2.57); shellcheck
clean. End to end against this machine's real, live daemon: shadowing
lsof with a failing stub (no /proc on macOS, so this is the full
Arch-shaped failure) now makes desvio_preflight die with the
install-lsof message where before it silently returned 0; pointing
DESVIO_WORKTREE at the daemon's own tree still dies with the original
"serving from this tree" message.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

3 participants