Activity

1. ooloth

   added

   bugSomething isn't workingSomething isn't working

   on Oct 10, 2026
2. ooloth commented on Oct 23, 2026

   

   ooloth

   on Oct 23, 2026

   OwnerAuthor

   More actions

   Approach

   Each skill that needs a scratch file asks mktemp for a new one at the moment it needs it, and then uses the path mktemp printed for the rest of that workflow. commit writes its partial-staging patch to that file. write-pr-description writes the PR body there with the Write tool instead of a cat heredoc. A check in scripts/checks/linting.bash fails when any shared skill names a fixed path under /tmp, so a skill added later can't bring the problem back. The rule is also recorded in docs/invariants/.

   What a good fix must achieve 🎯

   	Property	Category
   P1	Two sessions staging partial hunks at the same time each stage only the patch they wrote	🛡️ Safety
   P2	Two sessions opening PRs at the same time each post the body they wrote	🛡️ Safety
   P3	No shared skill names a fixed path under /tmp. The check runs over every SKILL.md, including ones added later	🛡️ Safety
   P4	The steps work in any harness that loads ~/.agents/skills, not only Claude Code	🛡️ Safety
   P5	An agent can follow the steps without shell variables surviving from one Bash call to the next, and without editing inside .git/	🙂 Experience

   Assumptions 🤔

   • Another harness reads these skills. ~/.agents/skills links to the same folder as ~/.claude/skills, and Codex is installed. If nothing else ever reads them, the session scratchpad becomes a viable option.
   • Claude Code asks for approval before editing files under .git/. That is my understanding of its protected paths, but I haven't confirmed it, because this session runs in bypass mode. If it doesn't ask, option C below ties with the chosen one.

   Constraints 🧱

   • Bash tool calls share no shell state, so a path held in a variable is gone by the next call. The path has to appear in output, where the agent can copy it.
   • Sessions that share one working tree also share one git index, so no file path can keep their staging apart. P1 covers sessions in different repos or worktrees.

   Options ⚖️

   Option	P1	P2	P3	P4	P5
   🏆 A. A new mktemp file per use, plus a check over every skill (chosen)	✅	✅	✅	✅	✅
   B. The session scratchpad directory	✅	✅	✅	❌	✅
   C. A path inside the repo (git rev-parse --git-path partial.patch)	✅	✅	✅	✅	❌
   D. No file: pipe the patch and body through stdin	❌	✅	✅	✅	❌
   E. Fix the two skills with no check	✅	✅	❌	✅	✅

   Decided by 🧭

   The scratchpad exists only in Claude Code, and only through its system prompt, so B fails in any other harness that loads these skills. C needs no remembered path, but the agent has to edit the patch inside .git/, which Claude Code guards. D would make the agent retype the patch with its chosen hunks inside a heredoc, which is exactly the kind of error the patch file prevents. E fixes today's two files, but nothing stops the next skill from writing to /tmp/whatever. That leaves A, which costs one extra step: the agent copies the printed path into the steps that follow.

   Deferred ⏳

   • write-pr-description writes the body with a cat heredoc, against use-gh's advice to use the Write tool and read the file back. Moving to mktemp touches those exact lines, so I'd fix it in the same change rather than defer it. That means adding Write to the skill's allowed-tools.
   • use-trekker/SKILL.md:74 has --log-file /tmp/test.log in an example done-when. The new check will flag it, so the example changes in this work too.
3. ooloth commented on Oct 23, 2026

   

   ooloth

   on Oct 23, 2026

   OwnerAuthor

   More actions

   Design

   Each skill that needs a scratch file now gets a fresh directory from mktemp -d and puts the file inside it. A directory is used rather than a file because the Write tool won't overwrite a file it hasn't read, and mktemp creates the file empty. In commit, one command creates the patch and prints its path, and the agent copies that printed path into the edit and apply steps. In write-pr-description, the agent writes the body into that directory with the Write tool, reads it back, and passes it to gh pr create --body-file. tools/agents/check-skills.py is extended so that, as well as checking frontmatter, it reads every file in every shared skill and reports each line that names a fixed path under /tmp, /private/tmp or /var/tmp. linting.bash already runs that script, so the rule covers skills added later. docs/invariants/skills.md records the rule for anyone writing a skill. The frontmatter check's behaviour doesn't change. A new test file covers the scanner. The two skill edits are verified by giving a fresh agent the edited skill and a real task.

   What the design must deliver 🎯

   	Property	Category
   P1	Two sessions staging partial hunks at the same time each stage only the patch they wrote	🛡️ Safety
   P2	Two sessions opening PRs at the same time each post the body they wrote	🛡️ Safety
   P3	No shared skill names a fixed path under /tmp. The check runs over every file in every skill, including ones added later	🛡️ Safety
   P4	The steps work in any harness that loads ~/.agents/skills, not only Claude Code	🛡️ Safety
   P5	An agent can follow the steps without shell variables surviving from one Bash call to the next, and without editing inside .git/	🙂 Experience
   P6	The check fails when it finds no skills, or a file it can't read as text	🛡️ Safety
   P7	The frontmatter check reports and exits exactly as it does today	🛡️ Safety
   P8	A failure names the file, the line and the path, so whoever sees it can fix it without searching	🙂 Experience

   Types 🧬

   Progressions considered

   Option	P3	P6	P7	P8
   🏆 A. A second check in check-skills.py that returns FixedTmpPath records (chosen)	✅	✅	✅	✅
   B. One generic SkillProblem(path, line, reason) shared by both checks	✅	✅	❌	✅
   C. A separate check-tmp-paths.py with its own walk over the skills	❌	✅	✅	✅

   Decided by: C gives the repo two definitions of "every skill", and the day the folder layout changes, one of them silently stops covering it (P3). B forces the frontmatter check's messages and summary into a new shape (P7), and nothing needs that. A keeps a single walk over the skills and leaves the frontmatter output alone.

   Progression

   Input: skills directory (defaults to tools/agents/config/skills; tests pass a temporary one)
     ↓ skill_files()
   list[Path]: every file under it except __pycache__, failing when the list is empty
     ↓ read_text() per file
   str | UnreadableFile(path)
     ↓ fixed_tmp_paths(path, text)
   list[FixedTmpPath(path, line, fixed_path)]
     ↓ main()
   exit 0, or exit 1 after printing "<file>:<line>: names <fixed_path>; use a directory from mktemp -d instead"
   

   • After skill_files(), a run can no longer pass by checking nothing: an empty list ends the run with an error. (P6)
   • UnreadableFile keeps the file's path, so a file that isn't UTF-8 fails the run by name instead of being skipped silently. (P3, P6)
   • Each FixedTmpPath holds the path, the line number and the exact text that matched, which is everything needed to find and fix it. (P8)
   • /tmp only counts when it starts a path. ${TMPDIR:-/tmp}/x, ./build/tmp/x, ~/tmp/x and a mention of a bare `/tmp` in prose don't match. This keeps the check to the risk named in the ticket. (P3)

   Assertions 🛡️

   The types can't check the content of files a person edits, and that is the only constraint here.

   • The files are UTF-8 text. This is boundary validation, because a person can add any kind of file to a skill. A file that fails it becomes an UnreadableFile failure; it does not crash the script.

   Nothing in this change can only be broken by a bug, so no assertion is planned.

   Telemetry 📡

   Nothing needs recording. The check's whole behaviour is visible in its output and exit code.

   Tests 🧪

   Neither the types nor the boundary check can tell whether the pattern catches the paths it should and leaves the others alone, or whether an agent actually follows the new steps. That's why these need tests.

   Expectation	Property	Technique
   A SKILL.md naming /tmp/changes.patch fails the check, and the output names its file and line	P3, P8	Integration
   A script inside a skill, not its SKILL.md, naming /tmp/x fails the check	P3	Integration
   /private/tmp/x and /var/tmp/x fail the check	P3	Example
   $(mktemp -d)/x, ${TMPDIR:-/tmp}/x, ./build/tmp/x and prose mentioning /tmp pass	P3	Example
   A skills directory with no skills fails	P6	Example
   A file that isn't UTF-8 fails, by name	P6	Example
   A skill with broken frontmatter still fails with today's message	P7	Integration
   The real skills directory passes after the edits (this is the check over every instance, run by linting.bash)	P3	Integration
   A fresh agent given only the edited commit skill, in a repo with two unrelated hunks in one file, stages one hunk through a mktemp -d path	P1, P4, P5	Agent replay
   A fresh agent given only the edited write-pr-description skill, with a stub gh on PATH that records --body-file, writes the body to a mktemp -d path with the Write tool	P2, P4, P5	Agent replay

   • Integration: each test runs the script as a subprocess against a temporary skills directory, the way scripts/checks/tests/test_docs.py tests docs.py, because the script's contract is its output and exit code.
   • Example: the pattern's edge cases are a short, named list. Each one documents a decision about what counts as a fixed path, which generated input wouldn't express.
   • Agent replay: the repo's harness standard verifies an instruction change by giving a fresh agent the failing input. That is the only way to observe what an agent does with the new steps. The stub gh keeps the PR replay from creating a real PR.

   I checked two things against the real system before planning on them. The pattern matches exactly the 7 lines in commit, write-pr-description and use-trekker, and nothing else in the skills directory. Staging a single hunk from a mktemp -d patch with git apply --cached works in a throwaway repo.
4. ooloth

   closed this as completedin d1d2845on Oct 23, 2026