Skip to content

fix(claude-k8s): write env-guard scripts to per-pod scratch, not shared CephFS (BLO-33641) - #1814

Closed
kkroo wants to merge 2 commits into
masterfrom
fix/blo-33641-guard-scripts-per-pod
Closed

kkroo wants to merge 2 commits into
masterfrom
fix/blo-33641-guard-scripts-per-pod

Conversation

@kkroo

@kkroo kkroo commented Sep 13, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • On the claude_k8s adapter each agent run is a Kubernetes Job whose main container mounts paperclip-data, a ReadWriteMany CephFS volume, at /paperclip — which is also $HOME
  • buildEnvGuardSetupShell() installs the PEN-1305 PreToolUse guard by shell-redirecting two .mjs files into ${CLAUDE_CONFIG_DIR:-$HOME/.claude}, i.e. into that shared volume
  • A > redirect is an O_TRUNC, so every agent pod start issues setattr size=0 against the same inode, contended by ~92 CephFS clients at once
  • In production that serialized pod startup behind a single MDS capability: pods scheduled fine, then sat in Init:0/1 behind ceph-csi's per-volume NodeStageVolume lock until the 600 s startup budget expired
  • This pull request writes those scripts to per-pod scratch instead, and stops rewriting settings.json when nothing changed
  • The benefit is that agent pod starts no longer contend one shared inode — removing the mechanism behind 253 failed runs in 12 h

Linked Issues or Issue Description

Refs BLO-33641 (Paperclip issue tracker).

Bug. Agent runs fail with k8s_pod_schedule_failed:

Pod startup failed: Timed out waiting for pod containers to start (600s):
  phase=Pending, init/write-prompt: waiting (PodInitializing),
  claude: waiting (PodInitializing)

Despite the error name this is not a scheduling failure — there were zero FailedScheduling events and no node pressure (no Memory/Disk/PID pressure on any of the 17 nodes). Pods schedule immediately, then hang.

Production measurements

Measure Value
k8s_pod_schedule_failed, 6 h 102
k8s_pod_schedule_failed, 12 h 253
Distinct volumes in FailedMount events 1
MDS ops blocked on that volume 72
…that were setattr size=0 on one inode 71
Oldest blocked op > 1800 s
CephFS clients mounting that volume ~92

Every stuck pod reported the same ceph-csi error — the per-volume in-flight lock, not Multi-Attach:

MountVolume.MountDevice failed for volume "pvc-729b17f4-78a7-497e-888d-7edf2193f20e" :
  rpc error: code = Aborted desc = an operation with the given Volume ID
  0001-0024-de65dd2b-...-3bdd0e68-... already exists

pvc-729b17f4-… is paperclip/paperclip-data (cephfs, ReadWriteMany). Dumping the MDS blocked ops showed 71 of 72 were setattr size=0 against inode 0x100070a0300, all parked at flag_point: acquired locks. Resolving that inode:

path: /volumes/csi/csi-vol-3bdd0e68-.../070c7c84-.../.claude/paperclip-env-guard.mjs
client_caps holders: 8
   client 42553657 issued pAsxLsXsxFsxcrwb  wanted pAsxXsxFxwb   <- exclusive
   client 41597843 issued pLs               wanted p             <- blocked
   ... six more at pLs/p

One shared file, eight clients, seven of them blocked waiting for the file caps the truncate needs.

Where it comes from

GUARD_DIR="${CLAUDE_CONFIG_DIR:-$HOME/.claude}"
mkdir -p "$GUARD_DIR"
printf %s '<b64>' | base64 -d > "$GUARD_DIR/paperclip-env-guard.mjs"
printf %s '<b64>' | base64 -d > "$GUARD_DIR/safe-env-inspect.mjs"
printf %s '<b64>' | base64 -d | node -          # unconditional settings.json write

The agent pod spec confirms the overlap — HOME=/paperclip, and /paperclip is the shared PVC:

volumes:   prompt: emptyDir | runtime-cache: emptyDir | data: persistentVolumeClaim | ...
mounts:    /tmp/prompt <- prompt | /runtime-cache <- runtime-cache | /paperclip <- data
env:       HOME=/paperclip

So all three writes land on the shared volume on every single pod start.

What Changed

  • env-guard.ts — the two .mjs files now go to $GUARD_SCRIPT_DIR, defaulting to /runtime-cache/paperclip-guard. That emptyDir is mounted unconditionally by job-manifest.ts (RUNTIME_CACHE_MOUNT_PATH), so it is always available; PAPERCLIP_GUARD_SCRIPT_DIR overrides it, and a failed mkdir -p falls back to $GUARD_DIR (previous behavior).
  • env-guard.tssettings.json stays in CLAUDE_CONFIG_DIR (Claude Code must find it there), but the merge script now computes the desired PreToolUse list and writes only when it differs from what is already on disk. Steady state is a pure read rather than a third unconditional truncate.
  • env-guard.ts — guard entries are matched by filename (endsWith("paperclip-env-guard.mjs")) rather than exact path, so an existing settings.json still pointing at the old in-$HOME location is migrated to the new path instead of accumulating a second hook entry. Without this, the path change alone would have appended a duplicate on every existing volume.
  • env-guard.test.ts — two regression tests (below).

The scripts are byte-identical for every pod and are only ever read locally by the PreToolUse hook, so nothing is lost by keeping them per-pod. This does not change what the guard does, only where it lives.

Verification

Both tests were written first and observed failing against unmodified env-guard.ts:

× writes the guard scripts to a per-pod path, not the shared CLAUDE_CONFIG_DIR
  AssertionError: expected 'GUARD_DIR' not to be 'GUARD_DIR'

× does not rewrite settings.json when the hook is already present
  - Expected   1789266295271.2898
  + Received   1789266295369.298      (mtime moved -> unconditional write)

After the change:

cd vendor/paperclip-adapter-claude-k8s
npx vitest run src/server/env-guard.test.ts
#  Test Files  1 passed (1)
#       Tests  279 passed (279)

Re-verified after rebasing onto current master: 279/279.

The existing idempotency test (merges the PreToolUse hook idempotently, preserving existing hooks) is unchanged and still passes, so the pre-existing contract — run twice, exactly one Bash guard entry, unrelated Stop hook preserved — still holds.

Pre-existing failures in this package, not caused by this PR

job-manifest.test.ts and tsc --noEmit both fail in a standalone checkout of vendor/paperclip-adapter-claude-k8s, because vendor/ is not a member of the root pnpm-workspace.yaml, so @paperclipai/adapter-utils and @types/node do not resolve there. I confirmed this is independent of the change rather than assuming it:

# job-manifest.test.ts, with env-guard.ts reverted to HEAD:
Error: Cannot find package '@paperclipai/adapter-utils/server-utils'
       imported from .../src/server/job-manifest.ts     <- file untouched by this PR
 Test Files  1 failed (1)

# tsc --noEmit, error counts:
baseline (HEAD):   112 errors,  3 in env-guard.ts
with this change:  112 errors,  3 in env-guard.ts

Identical counts. The 3 in env-guard.ts are Cannot find name 'Buffer' on the three pre-existing Buffer.from(...) lines, which this PR does not touch. CI resolves these normally.

Risks

Low.

  • Behavior preserved on the fallback path. If /runtime-cache is unavailable, mkdir -p ... || GUARD_SCRIPT_DIR="$GUARD_DIR" restores exactly the old locations, so the worst case is today's behavior.
  • Migration is handled. Filename-based matching rewrites a stale hook path in place. A settings.json written by an older image gets one entry pointing at the new path, not two.
  • The guard still runs on every Bash tool call — unchanged matcher, unchanged script contents, unchanged hook semantics. Only the file location and the write condition changed.
  • settings.json is still written when it genuinely needs to change (first run, corrupt/missing file, stale path), so a pod can never start without the guard installed.
  • The setup shell still fails open (|| echo "... settings merge skipped" >&2), so a merge error cannot block a run from starting — same as before.
  • Ordering is unchanged: job-manifest.test.ts asserts the guard install precedes ccrotate next, and the guard writes still come first in the joined fragment.

This does not fix the CephFS condition on its own. BLO-33641 also found the MDS has mds_max_caps_per_client = 1048576 with clients sitting just under it and recall_caps: 0, and 96 % of the MDS cache pinned — so cap pressure needs a separate operational fix. What this PR removes is the single-inode contention point that converted that pressure into failed pod starts.

Model Used

Claude Opus 5 (1M context) — model ID claude-opus-5[1m], via Claude Code. Extended thinking enabled; tool use (Bash, kubectl/psql/ceph against the live sfo12 cluster for every measurement above, vitest, tsc). Written test-first.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots — n/a, no UI surface
  • I have updated relevant documentation to reflect my changes — n/a, behavior documented in code comments
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

🤖 Generated with Claude Code

…ed CephFS (BLO-33641)

Every agent pod start ran:

    GUARD_DIR="${CLAUDE_CONFIG_DIR:-$HOME/.claude}"
    ... | base64 -d > "$GUARD_DIR/paperclip-env-guard.mjs"

On the k8s adapter $HOME is /paperclip, the shared ReadWriteMany CephFS
volume every agent pod mounts, so that redirect is an O_TRUNC against a
single inode contended by ~92 CephFS clients.

Measured in production: 71 of 72 blocked MDS ops were 'setattr size=0'
against this one inode, with the oldest aging past 1800s. Pods scheduled
fine, then hung in Init:0/1 behind ceph-csi's per-volume NodeStageVolume
lock until the 600s startup budget expired, which Paperclip records as
k8s_pod_schedule_failed - 253 in 12h.

The two .mjs files are byte-identical for every pod and are only ever
read locally, so nothing is lost by keeping them per-pod. They now go to
/runtime-cache/paperclip-guard (the emptyDir job-manifest.ts already
mounts unconditionally), with a fallback to $GUARD_DIR when that path is
unavailable, and PAPERCLIP_GUARD_SCRIPT_DIR to override.

settings.json must stay in CLAUDE_CONFIG_DIR for Claude Code to read it,
so the merge script now writes only when the resulting PreToolUse list
actually differs - steady state is a pure read instead of a third
unconditional truncate on the shared volume. Guard entries are matched by
filename rather than full path, so a settings.json still pointing at the
old in-$HOME location is migrated rather than accumulating a second entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Omar Ramadan <omar@blockcast.net>
@allyblockcast

allyblockcast Bot commented Sep 13, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-33641
🔗 Paperclip issue: PEN-1305

1 similar comment
@allyblockcast

allyblockcast Bot commented Sep 13, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-33641
🔗 Paperclip issue: PEN-1305

@github-actions

Copy link
Copy Markdown

@ally head 8732a84 has been awaiting review for 2.0h with no review on either surface (pulls/1814/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 8732a84.

@github-actions
github-actions Bot requested review from allyblockcast and removed request for allyblockcast September 13, 2026 06:40
@github-actions

Copy link
Copy Markdown

@ally head 8732a84 has been awaiting review for 4.1h with no review on either surface (pulls/1814/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 8732a84.

@github-actions
github-actions Bot requested review from allyblockcast and removed request for allyblockcast September 13, 2026 09:46
@github-actions

Copy link
Copy Markdown

@ally head 8732a84 has been awaiting review for 7.2h with no review on either surface (pulls/1814/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 8732a84.

@allyblockcast allyblockcast Bot 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.

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 8732a84

Critical Issues (0)

Important Issues (2)

  • [native-codex] vendor/paperclip-adapter-claude-k8s/src/server/env-guard.ts:872 — The shared settings.json now records a literal hook command pointing into /runtime-cache/paperclip-guard, but /runtime-cache is an emptyDir unique to each pod. The file is still shared across pods, so each pod startup races to rewrite the same shared settings file with its own pod-local path; after another pod wins, this pod invokes a nonexistent script and the env-dump guard is silently bypassed (the hook fails open).
    • Keep the shared hook target stable, or make the configuration itself pod-local. If the hook must remain in shared settings.json, use a stable shared entrypoint that dispatches to the current pod-local script rather than embedding a per-pod absolute path, and add a multi-pod/shared-settings regression test.
  • [gstack/review] vendor/paperclip-adapter-claude-k8s/src/server/env-guard.ts:837 — The guard’s remediation text still tells blocked users to run node $HOME/.claude/safe-env-inspect.mjs, but this change writes safe-env-inspect.mjs to $GUARD_SCRIPT_DIR (normally /runtime-cache/paperclip-guard). The documented escape hatch therefore points at a nonexistent or stale shared-volume file after deployment.
    • Construct the remediation command from the same runtime guard-script directory, or print the configured path; add an assertion that the message matches the installed helper location.

Suggestions (1)

  • [pr-review-toolkit] vendor/paperclip-adapter-claude-k8s/src/server/env-guard.test.ts:349 — Add a test that executes the generated setup shell with a separate guard directory and verifies the installed hook points there and remains usable after a second pod uses the same settings directory.

Strengths

  • The diff is narrowly scoped and adds regression coverage for both per-pod script placement and steady-state settings idempotency.
  • Filename-based migration avoids accumulating duplicate guard hooks when upgrading existing shared settings.
  • The runtime-cache volume is mounted in both init and main containers, and the existing hook semantics remain otherwise unchanged.

Recommended Action

  1. Fix Important issues before merge.
  2. Address the shared-settings/path consistency issue this cycle.
  3. Consider the additional cross-pod regression coverage.

…ed helper

Ally review on #1814 raised two Important findings against the per-pod
guard-script move (BLO-33641):

1. settings.json is shared across pods but the hook command embedded this
   pod's script dir. A pod that fell back to CLAUDE_CONFIG_DIR would rewrite
   the shared file and point every other pod at a path it does not have,
   where the hook died as a bare module-resolution error (silent fail-open).
   The recorded command is now ENV_GUARD_HOOK_COMMAND, identical for every
   pod: it resolves /runtime-cache/paperclip-guard first, then the
   CLAUDE_CONFIG_DIR fallback, and reports a guard missing from both on
   stderr (still exit 0 per the recorded BLO-22514 fail-open decision).

2. The block message still told users to run $HOME/.claude/safe-env-inspect.mjs,
   which no longer receives the file. It now names the helper beside the
   guard that actually ran.

Tests first (3 red -> green, 283/283): command is byte-identical across
differing PAPERCLIP_GUARD_SCRIPT_DIR values and embeds neither; the command
dispatches to the config-dir copy through `sh -c`; a missing guard is
reported rather than silent; the remediation names the sibling helper.

Also updates PROVENANCE.md (row + integrity hash) and bumps the vendored
package to 0.2.6-blockcast.6 -- the `Vendored claude_k8s adapter` job was
red on the stale hash.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Omar Ramadan <omar@blockcast.net>
@kkroo

kkroo commented Sep 13, 2026

Copy link
Copy Markdown
Author

Addressed both Important findings in c229325 (tests written first, 3 red → 283/283 green):

  1. Shared settings.json vs per-pod script dir — the recorded hook command is now ENV_GUARD_HOOK_COMMAND, a pod-independent string that resolves at hook time: /runtime-cache/paperclip-guard/paperclip-env-guard.mjs first, then ${CLAUDE_CONFIG_DIR:-$HOME/.claude}/… (the same fallback the setup shell uses). The merge script no longer reads PAPERCLIP_GUARD_SCRIPT_DIR, so no pod can rewrite the shared file with its own path. A guard missing from both locations now prints [paperclip-env-guard] guard script missing at … to stderr instead of dying as a bare Cannot find module — it still exits 0, deliberately, per the recorded BLO-22514 fail-open decision in the same file. Regression tests: command byte-identical across two different PAPERCLIP_GUARD_SCRIPT_DIR values and embeds neither; dispatch through sh -c to the config-dir copy blocks env (exit 2) / allows ls; missing guard is reported, not silent.

  2. Remediation text — the block message now names the helper beside the guard that actually ran (dirname(process.argv[1])/safe-env-inspect.mjs), asserted by a test that runs the embedded artifact from a temp dir.

Also: PROVENANCE.md row + integrity hash updated and the vendored package bumped to 0.2.6-blockcast.6 — the Vendored claude_k8s adapter job was red on the stale hash. The other five reds on 8732a84 were runner-pool mid-job kills per verify, not diff failures.

🤖 Generated with Claude Code

@kkroo

kkroo commented Sep 13, 2026

Copy link
Copy Markdown
Author

Closing as superseded by #1810 (merged 2026-09-13T09:58Z), which fixes the same BLO-33641 root cause — the fleet-wide O_TRUNC on the shared paperclip-env-guard.mjs inode — with a better shape: content-addressed guard filename + install-once (test -f || tmp && mv), helper at a stable name that the block message already points to, and a pod-independent PAPERCLIP_GUARD_FILE hook command. That already covers what this PR did and both of Ally's findings here (stable hook command; remediation path matches the installed helper), so there is nothing left to carry. Branch left in place.

🤖 Generated with Claude Code

@kkroo kkroo closed this Sep 13, 2026
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