Skip to content

infra(delta): say what is dirty and who deployed when refusing a dirty checkout - #172

Open
dadamsncsa wants to merge 1 commit into
mainfrom
infra-delta-dirty-tree
Open

dadamsncsa wants to merge 1 commit into
mainfrom
infra-delta-dirty-tree

Conversation

@dadamsncsa

Copy link
Copy Markdown
Contributor

Why

deploy --apply refuses a deployment checkout with uncommitted changes, and it should. But it said only deployment tree is dirty, so whoever it blocked had to ask around to learn what was in the tree and whether it was safe to discard. That happened on the shared staging stack: three files from an open PR branch had been copied into its checkout by hand two days earlier, and it took a Slack thread to establish that they matched the pushed branch. DEPLOYED couldn't help, because under /sw/admin/scripts/impersonate its by= is always svcdeltallmhub.

What changes

  • The refusal explains itself. It lists the dirty paths (the first 20, then a count), shows the last DEPLOYED record, and prints the commands to discard them. The original error line is unchanged.
  • The dry run warns first. The README's flow runs deploy without --apply first, so the dry run now shows the same listing and says --apply will refuse it. It still changes nothing.
  • DEPLOYED points at the person. by= reads <SUDO_USER> via <account> when the kit runs under sudo (impersonate uses sudo -u), and the account alone otherwise. It is a pointer, not an audit record, because SUDO_USER is inherited. Refs whose own kit predates this change still record the account alone, since the deployed ref's kit writes DEPLOYED.
  • README: try a branch with --ref, never by editing the checkout; to run main and a PR together, push a branch that merges both. Plus what by= does and does not tell you.

The refusal now looks like this (sandbox paths shortened):

  <root>/LLMHub has 2 uncommitted path(s):
       M CHANGELOG.md
      ?? frontend/
  last deployed: ref=main kind=branch sha=… date=2026-11-03T16:54:29-06:00 by=svcdeltallmhub
ERROR: deployment tree is dirty — it must be a clean checkout, not a working copy.
       Check with whoever made these changes, then discard them:
         git -C <root>/LLMHub reset --hard && git -C <root>/LLMHub clean -fd
       To try a branch on this stack, deploy it with --ref <branch>; never edit the checkout.

Testing

The real ./llmhub deploy [--apply] --profile staging was driven in a sandbox on a DeltaAI login node:

  • every root redirected to a temp directory;
  • LLMHUB_VM_HOST set to the login node;
  • a local bare repo as origin;
  • LLMHUB_UV_DIR under /proc, so an --apply run fails at the first runtime step, before anything is installed or downloaded.

Six cases and 25 checks: a dirty tree, 25 dirty paths, a clean tree via sudo, a clean tree without sudo, a dirty-tree dry run, and a clean-tree dry run (which must not warn).

Kit Result
this branch 25 / 25 pass
main 12 / 25 pass; it fails only the 13 checks for the new behaviour

Not yet exercised on a VM: whether SUDO_USER survives impersonate's login shell (bash -l). The first deploy's by= will show it; if it doesn't, by= stays as it is today.

Follow-ups (not in this PR)

  • status still says only tree is DIRTY with no paths, and it reports tree clean when git cannot read the checkout (2>/dev/null | head -1). It could use show_dirty too.
  • The README profile table's staging column shows svcdeltallmhub/ paths, but for any other account the code uses /projects/bfmz/<user>/llmhub-staging and /data/llmhub/staging-<user>.
  • The deeper fix is a deploy tree nobody edits in place, for example one checkout per SHA. That is out of scope here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HnSD7hPrebDeNgc4wTqoPd

…y checkout

deploy --apply refused a dirty deployment tree with a one-line error, so
whoever it blocked had to ask around to learn what was in the tree and
whether it was safe to discard. Now the refusal lists the dirty paths (the
first 20, then a count), shows the last DEPLOYED record, and prints the
commands to discard them. The original error line is kept. The dry run,
which the README runs first, shows the same listing and warns that
--apply will refuse.

DEPLOYED's by= now names the person sudo reports behind impersonate. Under
/sw/admin/scripts/impersonate (sudo -u), `id -un` is always svcdeltallmhub,
so by= could never say who deployed. It now reads "<SUDO_USER> via
<account>" when SUDO_USER is set, and the account alone otherwise. It is a
pointer, not an audit record. Refs whose own kit predates this change
still record the account alone, since the deployed ref's kit writes
DEPLOYED.

README: try a branch with --ref, never by editing the checkout, plus what
by= does and does not tell you.

Tested by driving the real `deploy [--apply] --profile staging` in a
sandbox (all roots redirected; --apply runs stopped at the first runtime
step), in six cases: a dirty tree, 25 dirty paths, a clean tree via sudo, a
clean tree without sudo, a dirty-tree dry run, and a clean-tree dry run.
25/25 checks pass. The same checks against the previous kit pass 12/25,
failing only on the new behaviour.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HnSD7hPrebDeNgc4wTqoPd

This branch has not been deployed

No deployments
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