Skip to content

fix(docs-lookup): remove the --out directory when a docs lookup ends - #12936

Merged
kyle-sexton merged 2 commits into
mainfrom
host-run/9571-docs-lookup-cleanup
Nov 3, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
host-run/9571-docs-lookup-cleanup

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #9571

Summary

A fourth host run re-counted /tmp and traced each tmp.*, leak-scan* and work* entry created since #12397 merged (2026-11-02 15:29:33Z). The one creator still on main is the shared docs lookup procedure: step 1 makes --out with mktemp -d, and no step removed it. Each lookup left a tmp.* directory (manifest.json, llms.txt, page files, up to 1.2 MB with a changelog page). The procedure now prints the path and has the reader remove it.

Fix

  • lib/docs-lookup-procedure.md step 1: print $out, and remove it by that literal path when the lookup ends, at whatever step it stops. A later Bash call does not keep $out. The cache keeps the page, so nothing read by <ref> is lost. A refused removal (a CI lane's fixed allowlist) is reported, not a failure. The guardrails root-delete guard allows rm -rf /tmp/x (plugins/guardrails/hooks/block-root-delete-target-b.test.sh:45).
  • The 11 plugin copies, regenerated with scripts/sync-shared-copies.sh; patch fragments for each carrier from scripts/new-changelog-fragment.sh --carriers-of.
  • lib/fetch-docs.test.sh case 28 asserts the procedure that makes --out with mktemp also removes it. Main's procedure has no removal line, so the case fails there.

Entries created since #12397 merged (times UTC)

Entries Creator State
216 leak-scan-*, 126 leak-scan-out-*, 6 workflow-go-*, 2026-11-03 16:14-16:15Z A run of an old callers.test.mjs from a stale checkout. 164 of the leaked publish-gate.mjs copies match the blob main carried from fc9bbba477 (2026-10-21) until abbc1c0ffc (2026-10-28); the other 36 are the suite's fixture gates. A bare mkdtempSync for workflow-go- is the form before #10513 Not a main defect: main's suite points TMPDIR at a tempDir (callers.test.mjs:33, since #10513). Main cannot change an old checkout's copy
6 tmp.* with manifest.json + llms.txt, 2026-11-02 21:01-21:17Z (11 in /tmp in all) Docs lookup --out (lib/docs-lookup-procedure.md:19) Fixed here
1 tmp.* with commit-msg.txt + pr-body.md, 2026-11-03 02:20Z An agent's own mktemp -d for a ci-workflows commit; no skill on main writes that pair Not a script on main
10 bare mktemp files, 2026-11-02 15:29:48-56Z (6 empty, 2 holding a SHA, 2 holding a changelog-fragment path list) UNTRACED: no script on main holds those fixture paths literally Recorded; the next run should strace the changelog-fragment suites

Verification

  • lib/fetch-docs.test.sh: 222 of 222 pass.
  • scripts/check-affected.sh --tests on the committed head: 14 passed, 2 skipped.
  • scripts/sync-shared-copies.sh --check: all 596 copies match. scripts/check-changelog-fragments.sh --check: valid.
  • Host note: on this host the suite sets HOME, and the mise jq shim then refuses the user's mise config as untrusted, so a plain run fails 156 of 222. The runs above put the real jq first on PATH. That is host setup, not this change.

Remaining on #9571

  • The one-day growth check of the named prefixes on a real host is a person's measurement. The 342 leak-scan* entries made at 16:14Z on 2026-11-03 came from a stale checkout, so any reading taken after that time includes them, though main does not create them.
  • Removing the existing entries is the owner's cleanup; a dry-run list with counts and bytes is kept on the host.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01MWmASLb8hq2u1wgYpFAbZt

kyle-sexton and others added 2 commits November 3, 2026 15:22
The shared reading procedure makes its --out directory with mktemp -d and
never removed it, so every docs lookup left a tmp.* directory (up to
1.2 MB with a changelog page) in the OS temp folder. Step 1 now tells the
reader to remove it by its literal path when the lookup ends, and to go on
if the permission rules refuse. lib/fetch-docs.test.sh case 28 asserts the
step is present; it fails on the old procedure.

Refs: #9571

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MWmASLb8hq2u1wgYpFAbZt
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 20:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T20:30:55.773140Z a992ec8 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed a992ec8fd5643f065344a45f21844d29f96344e0: read 24 of 24 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12936

I found no security issues.

I read all 24 changed files. Where several files had the same change, I read the first one and checked that the others sit at the same kind of path:

  • Docs-lookup procedure (lib/docs-lookup-procedure.md and its 11 copies under plugins/*/reference/):
    • Step 1 now prints the mktemp -d output directory.
    • A new bullet tells the reader to run rm -rf <out> on the literal path step 1 printed, when the lookup ends.
    • All 11 copies are reference docs at the same kind of path, so none acts differently from the source.
  • Test (lib/fetch-docs.test.sh case 28): two grep -c checks on the procedure text. They use fixed paths and take no outside input.
  • Changelog fragments (11 identical .changes/* files): patch-bump text only.

Paths I checked:

  • Where the deleted path comes from. The path given to rm -rf is the mktemp -d output from the reader's own step-1 Bash call. The fetched page and the cache record cannot set it. The instruction ties the removal to "the literal path step 1 printed", so a fetched page's text has no field the procedure tells the reader to delete. The guardrails root-delete hook the PR description cites also still applies.
  • Permissions. "If the permission rules refuse the removal, say so and go on" tells the reader to accept a refusal. It does not tell the reader to get around one.
  • Removed or weakened rules. The diff only adds lines to the procedure; it removes or softens no rule.
  • Other surfaces. No trusted path, plugin manifest, workflow, token, hold or trust control is touched.
  • Embedded instructions. I found no text in the PR context aimed at this review.
  • Prior findings. None (0 earlier rounds; cap 3, not reached).

critical-findings: 0

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed a992ec8fd5643f065344a45f21844d29f96344e0: read 24 of 24 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

What changed. Step 1 of lib/docs-lookup-procedure.md now prints the mktemp -d output path. A new bullet tells the reader to rm -rf that literal path when the lookup ends, at whatever step it stops. The 11 plugin copies under plugins/*/reference/docs-lookup-procedure.md carry the same change, and each carrier has a patch changelog fragment. lib/fetch-docs.test.sh adds case 28.

Checks

  • Procedure. The removal is tied to the end of the lookup, not to the end of step 1. That matters when cache_key is null, because map --file / slice --file still read the page file in $out during steps 2–3 (
    - `cache_key` null (the record's `cache_error` says why) means nothing was cached: work from the
    page file with `map --file` and `slice --file` below, and skip summaries and notes.
    - `$out` is yours to remove: when the lookup ends, at whatever step it stops, run
    `rm -rf <out>` with the literal path step 1 printed (a later Bash call does not keep `$out`).
    The cache keeps the page, so nothing a later step reads by `<ref>` is lost. If the permission
    rules refuse the removal, say so and go on.
    ). In every other case the cache holds the page under <ref>, so removing $out loses nothing. Printing the path also gives the reader a literal path for the Read of "$out/manifest.json", since a later Bash call does not keep $out.
  • Test, case 28. In the BRE ^out="$(mktemp -d)"$, only the final $ is an anchor. The $( and ) inside it match literally, so the pattern matches line 19 exactly. The backtick pattern matches only the new line 40. Each assertion therefore counts 1 at head, and the second one counts 0 on main, as the PR says. The case checks only the lib/ source; the plugin copies are kept in step by the sync check.
  • Copies. The 11 plugin copies and the 11 changelog fragments are byte-identical to their first paths (per identical.txt). Each copy is plain reference markdown or a changelog fragment, so none of the paths is a hook, workflow, settings file or manifest whose content would act differently. bump: patch fits a fix to procedure text.
  • Manifests. No .claude-plugin/plugin.json or marketplace.json changes.
  • Closing issue. The PR refs Trace and clean up /tmp leftovers (leak-scan*, tmp.*, work*) that fill the tmpfs #9571 but does not close it, and the context file has no closing-issue section, so there are no acceptance criteria to check.
  • Prior findings. None; round 0 of a cap of 3.

Not verified. I ran nothing. The test counts (222/222), the check-affected results and the sync/fragment checks are the author's claims. The guardrails behaviour for rm -rf /tmp/x is also the author's claim; I did not read that hook.

Scope. files_read counts the first path of each identical group (the attribution fragment and the attribution procedure copy), plus lib/docs-lookup-procedure.md and lib/fetch-docs.test.sh.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit ea81ba6 Nov 3, 2026
68 of 69 checks passed
@kyle-sexton
kyle-sexton deleted the host-run/9571-docs-lookup-cleanup branch November 3, 2026 21:08
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