Skip to content

th-6578ee: attest-push-hint missed cd <repo> && git push — the shape agents actually use - #338

Merged
brentrager merged 5 commits into
mainfrom
th-attest-hook-cwd
Aug 6, 2026
Merged

th-6578ee: attest-push-hint missed cd <repo> && git push — the shape agents actually use#338
brentrager merged 5 commits into
mainfrom
th-attest-hook-cwd

Conversation

@brentrager

Copy link
Copy Markdown
Contributor

The bug

The hook resolved the repo from the PreToolUse payload's .cwd. That's the session's directory — and a session sits in one repo for its whole life, so agents write:

cd /path/to/other-repo && git push

The hook saw the session cwd, found no scripts/ci/attest.sh there, and exited 0. Silently. On the common case.

Verified live: a real cd ~/dev/smooai/smooai && git push from a smooth session pushed with no prompt, while the identical push with cwd=smooai correctly asked.

Why the tests didn't catch it

The original 10-case suite hand-fed cwd values instead of the shape a real agent emits. The tests agreed with the bug. It only surfaced by running an actual git push in a live session.

Fix

Resolve last-wins, mirroring where the shell actually ends up:

  1. git -C <path> push — explicit, beats everything
  2. the last cd <path> in the chain
  3. .cwd

Plus quote-stripping and ~ expansion.

Also fixes push detection: (-[^ ]+\s+)*push let a flag's value eat the push token, so git -C /some/repo push read as not-a-push.

Verification — 14/14

cd …/smooai && git push (and ~, -u, --force-with-lease) ASK
git -C …/smooai push ASK
git push with cwd=smooai ASK
:ack, --dry-run, --delete, th pearls push, git status, attest.sh allow
any push in a repo without scripts/ci/ allow

Shipped resolving the repo from the PreToolUse payload's `.cwd`. That is the
SESSION's directory, and a session sits in one repo for its whole life — so
agents write `cd /path/to/other-repo && git push`. The hook saw the session
cwd, found no scripts/ci/attest.sh there, and exited 0. Silently. On the
common case.

Verified live 2026-08-06: a real `cd ~/dev/smooai/smooai && git push` from a
smooth session pushed with no prompt, while the identical push with cwd=smooai
correctly asked. My original 10-case suite passed only because I hand-fed it
cwd values instead of the shape a real agent emits — the tests agreed with the
bug.

Now resolves last-wins: `git -C <path>` > last `cd <path>` > `.cwd`, with
quote-stripping and ~ expansion.

Also fixes push detection: `(-[^ ]+\s+)*push` let a flag's VALUE eat the
`push` token, so `git -C /some/repo push` read as not-a-push. Flags that take
an argument are now matched explicitly.

14/14 cases, including every miss above.
@brentrager
brentrager enabled auto-merge (squash) August 6, 2026 17:02
@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 99d0a39

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@smooai/smooth Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…lable

The fix branch and the installed copy both read 0.4.0, so
`claude plugin update smooth-agent@smooth` could report 'already up to date'
and never install it. Today's 0.2.0 -> 0.4.0 update only worked because a real
version delta existed.

A plugin change that ships no version bump is a change nobody can install.
Bump both manifests together — `claude plugin tag` validates that plugin.json
and the marketplace entry agree.
…g else

The plugin manifests carried their OWN hand-maintained version, so they drifted
from the repo. smooth-agent sat installed at 0.2.0 for 26 days (2026-07-11 ->
2026-08-06) while this repo shipped 20+ versions. Two hooks merged in that
window never ran on anyone's machine, and attest-push-hint was reported as
'live' three separate times while doing nothing.

`claude plugin update` only reinstalls on a version DELTA, so a plugin change
shipped without a bump is a change nobody can install. Relying on remembering
to bump by hand is what produced the 26-day gap.

sync-versions.mjs already propagates package.json -> Cargo.toml + Cargo.lock
during `pnpm ci:version` (changesets/action -> ci-version.mjs). Adding the two
plugin manifests to that same list means every release bumps the plugin, so an
installed copy is always comparable to the published one.

Over-bumping when only Rust changed is harmless — plugin update just reinstalls
the same hooks. plugin.json and the marketplace entry are bumped together
because `claude plugin tag` validates that they agree.

Verified: sync-versions.mjs takes both to 0.26.5 and both stay valid JSON.
The Cargo.lock exception matched `smooai-smooth-operator(-core|-server)?` —
an allowlist of three names. Two more crates joined that dep graph upstream:

    smooai-smooth-operator-adapter-memory
    smooai-smooth-operator-ingestion

Neither is in the list, so every `pnpm version:sync` rewrote their locked
version from the smooth-operator release (1.23.1) to this workspace's (0.26.5).
That is the churn in th-16ff70 — 'flips 0.23.x<->1.23.x on every local build,
DIRTY-PR tax on all lock-touching PRs' — and it also breaks `cargo --locked`,
since that source never published 0.26.5.

Matched by PREFIX now. A hand-maintained allowlist cannot track a dependency
graph that grows in someone else's repo; the invariant is 'external operator
crate', and the prefix expresses it. Safe because NO crate in this workspace is
named smooai-smooth-operator* — the engine is a separate repo (CLAUDE.md §1).

Verified: sync-versions is now idempotent on a synced tree ('No files were
updated') and leaves Cargo.lock untouched.
…ell script

This PR is the case in point: four commits touching one bash hook, two JSON
manifests, and a JS script triggered 'Rust checks (ubuntu-latest)', 'Rust
checks (windows-latest)' and 'Web checks' — a full workspace compile on two
platforms, Windows billing at 2x, plus a Vite build. ~45 min of runner time to
validate a shell script.

Gates STEPS, not jobs. A workflow-level `paths:` would mean the checks never
appear at all, and branch protection requires them — the PR would block
forever. This is the same reason the `push:` trigger exists (pearl th-e392d9)
and the same pattern smooai's pr-checks uses.

PR events only. push-to-main and merge_group have no reliable base to diff, and
those are the moments a wrongly-skipped check is most expensive, so they run
everything.

rust triggers on crates/**, Cargo.{toml,lock}, rust-toolchain.toml,
rustfmt.toml, go/**, build-smooth-dolt.sh, and this workflow. web on
crates/smooth-web/web/** and this workflow. Both include the workflow itself so
a change to the gating always exercises what it gates.

Verified: YAML parses, permissions carry pull-requests: read (dorny needs it on
pull_request), 16/19 rust steps and 4/7 web steps gated — the ungated three per
job are checkout, filter, and gate.
@brentrager
brentrager merged commit 7a8bde6 into main Aug 6, 2026
0 of 3 checks passed
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.

1 participant