Skip to content

Add guarded session cleanup and scratch-local temporary files - #31

Merged
kikashy merged 4 commits into
mainfrom
codex/issue-30-safe-clean
Oct 23, 2026
Merged

kikashy merged 4 commits into
mainfrom
codex/issue-30-safe-clean

Conversation

@kikashy

@kikashy kikashy commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Closes #30.

What changes

Add dry-run cleanup for explicitly owned session data: eligible worktrees, reviewer clones,
orphan scratch/locks, unused shared caches, and old large transcripts. Explicit roots, session
enrollment and disposal seals establish the candidate set. Cleanup reports root-relative paths,
sizes, reasons and per-path outcomes, and refuses missing ownership or safety evidence.

Git publication/PR checks, process visibility, keep entries, inventory checks and conservative
filesystem operations protect deletion. Unknown untracked or ignored content, nested repositories,
symlinks, live processes and changed evidence prevent removal. Historical unsealed data remains
protected. Builder TMPDIR is created beneath scratch; mutation copies and fixture Git discovery
support temporary files inside the clone. Update the brief and cleanup policy together.

Public surface

Add jpdev clean [--root DIR]... [--older-than DURATION] [--min-transcript-bytes N] [--yes], clean init and clean seal.
Defaults are dry run, no implicit roots, a seven-day age threshold, and a 10 MiB transcript-only size floor (adjustable, including zero). Document the disposal
record format, keep-file format, JSON Lines output and exit codes. Complete initial Linux host
process visibility is required; incomplete evidence has no override.

Checks

The required full local checklist passed before the initial push:

== check summary (dev.json) ==
PASS     diff-check               0s
PASS     local-paths              0s
PASS     agent-docs               0s
    sync-agent-docs: CLAUDE.md matches AGENTS.md
    sync-agent-docs: CLAUDE.md matches AGENTS.md
PASS     bash-n                   0s
    bash -n: every script parses
PASS     json                     0s, python <home>/.pyenv/versions/3.8.20/bin/python3 (Python 3.8.20)
    parsed 6 JSON documents
PASS     shellcheck               4s
    shellcheck: clean
PASS     tests                    48s
    420 passed, 0 failed
== 7 passed, 0 failed, 0 skipped, 0 not run ==

The interpreter path is redacted. CI fixture-portability corrections were verified with targeted checks. The parent-rename PathLike case passes on Python 3.8, 3.12 and 3.14.

The review fix commit passes 67 focused cleanup tests and all four touched checklist steps (whitespace, local paths, Bash syntax and ShellCheck). All 18 new safeguard mutations are caught, including the final logical-file-byte regression. Original evidence covers 43 cleanup mutations plus four independently verified TMPDIR/source-copy mutations. All removal exercises use disposable fixtures created for this task.

CI run 39957484955 passes for eea02d969338de33605d700f26b5e78b1dd4ec81: all seven checklist steps and 420 repository tests. The targeted reread is complete: all findings are resolved, with no new findings in that scope. The independent review identified two HIGH findings (bare/saved Git metadata and inherited Git evidence settings) and one MEDIUM finding (cache/scratch eligibility races). The fix commit addresses all three, adds refusal of hidden tracked-file index flags, and completes the transcript-size threshold. The original reviewer confirmed all dispositions after green CI and caught 19/19 targeted mutations, including an additional final-directory race probe. Verbatim reports and dispositions.

Review and sign-off

The user temporarily authorized one independent clean-room Codex review of this Codex implementation
because Claude quota is unavailable. This is a same-vendor exception, not cross-vendor compliance.
Builder and reviewer were explicitly selected as gpt-6.1-sol/high; the independent review and same-reviewer targeted reread followed green CI. No unresolved review finding or design decision remains in the reviewed scope.
Each commit carries a Developer Certificate of Origin sign-off matching its author.

Limits

Git administrative entries and branch refs are retained. Refusal during removal can retain a
quarantine and partially removed candidate; the command does not claim transactional rollback.
No merge, tag, release, version change, global installation, or cleanup of existing machine data.

A Codex session wrote the change.

kikashy and others added 4 commits October 23, 2026 07:54
Add explicit-root dry-run cleanup for session-created worktrees, reviewer
rooms, scratch, shared caches and transcripts. Require disposal inventories,
publication and PR evidence, reserves and complete host process visibility,
and recheck filesystem and Git state during descriptor-relative removal.

Confine builder TMPDIR beneath scratch, exclude nested generated state from
mutation copies, and isolate fixture Git discovery. Document cleanup
ownership, milestones, formats, defaults and conservative refusals.

Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: kikashy <kikashy@hotmail.com>
Use a deterministic host process view around the real disposable child PID
so cleanup fixtures do not require access to protected PID 1 information.
Keep production cleanup checks unchanged and retain mutation coverage for
the live-process and host-mount safeguards.

Collect failed test cases in one summary line and select it in dev.json so
late failures remain visible when CI truncates raw test output.

Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: kikashy <kikashy@hotmail.com>
Give the nested-mutants fixture a nonempty check list so it validates when
jsonschema is installed. Normalize the parent-replacement rename target
with os.fspath so the fixture accepts pathlib arguments on newer Python.

Keep production safeguards unchanged. Verify both failing CI cases from
clean archived source and retain the parent-identity safeguard mutation.

Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: kikashy <kikashy@hotmail.com>
Protect bare and saved Git directories at every candidate depth. Refuse
inherited Git evidence settings and index flags that can hide tracked
changes, and isolate verification from ambient Git configuration.

Repeat cache and scratch worktree eligibility after quarantine and before
each removal. Add a transcript-only logical byte floor with a 10 MiB
default and an explicit nonnegative override.

Convert the independent review reproductions into refusal regressions.
Verify the affected safeguards with focused tests and caught mutations.

Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: kikashy <kikashy@hotmail.com>

kikashy commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Independent review of Devkit issue #30

Reviewed PR #31 at head 2a15dce41e3ff5dfaf54b896014264887c53b52e, against base 8993f8448337d7f7fa6a2f4ed8a0f0b197a492b7.

Result: changes required. Two HIGH findings allow deletion of Git evidence the contract protects. One MEDIUM finding allows cache/scratch removal after its worktree eligibility changes.

This was the single independent clean-room review, using the host-selected gpt-6.1-sol at high reasoning because the change defines a deletion boundary. It used the user's temporary same-vendor exception; it is not cross-vendor compliance. No builder reasoning was read.

Findings

R1 — HIGH — Bare repositories bypass publication checks

Location: lib/clean.py:368 (repository classification in inspect; related traversal at lines 123–129).

The only repository marker recognized by inspection is an entry named .git. A bare repository, or a saved Git directory, has HEAD, objects and refs directly beneath its root. It therefore enters the non-Git branch and can be removed as a sealed review/transcript directory without any remote-publication check. The same omission permits a nested bare repository inside an otherwise disposable directory. Ownership and inventory attestation do not satisfy the separate prohibition on deleting unpublished commits.

Independent reproduction: initialize a disposable enrolled session root; create private.git with git init --bare; create a commit with commit-tree and retain it under refs/heads/private, with no remote; seal it as review; inspect and remove it with the existing process-evidence fixture seam. Inspection returned git=None, and real descriptor-relative removal deleted the repository. No Git/publication safeguard was mocked.

Evidence: review-scratch/independent-probes.py and review-scratch/independent-probes.json, probe bare_repository_publication_bypass (removed=true, git_checked=false).

Required regression: refuse a candidate that contains an unpublished bare repository or saved Git metadata, both at its root and nested beneath an otherwise permitted class. If these repository forms are unsupported, conservative refusal is sufficient. Mutating their detection must make the regression fail.

R2 — HIGH — Inherited Git environment can hide the actual staged index

Location: lib/clean.py:138 (run preserves os.environ); affected checks at lines 281–282.

Explicit --git-dir and --work-tree arguments do not neutralize GIT_INDEX_FILE. When this variable names a clean alternate index, both diff checks use that index while the candidate's actual .git/index can contain staged private changes. Snapshotting the actual index later only proves it stayed unchanged; it does not prove those staged contents are disposable. The candidate is then deleted despite the documented refusal for staged changes.

Independent reproduction: create a disposable detached reviewer clone with published HEAD; save a copy of its clean index outside the candidate; stage different contents for a tracked file, then restore the worktree file to published contents; seal the clone. The real index reports file under git diff --cached --name-only. Set GIT_INDEX_FILE to the clean saved index and remove the candidate using the process-evidence seam. Removal succeeds and deletes the clone, including its real staged index and private object. No Git command or filesystem check was mocked.

Evidence: review-scratch/independent-probes.py and .json, probe inherited_index_redirect (removed=true, actual_staged_paths="file").

Required regression: cleanup must refuse or safely neutralize inherited Git variables that redirect repository/index/object/config evidence. At minimum cover the alternate-index example and ensure the actual index remains protected. The fix should explicitly account for other Git evidence indirection rather than assuming the two command-line paths override every inherited setting. Removing the environment safeguard must make the test fail.

R3 — MEDIUM — Cache and scratch eligibility is not rechecked after quarantine

Location: lib/clean.py:472 (remove.check); eligibility predicates are at lines 375–410.

For cache and scratch records, git is None. After the last inspect, quarantine and per-entry checks revalidate inventory, roots, keeps and processes, but never revalidate the base's worktree registrations or the associated removed worktree's absence. A completed concurrent worktree creation therefore invalidates eligibility without stopping removal.

Two independent reproductions use a rename hook to model that concurrency, with real Git operations: (a) after a sealed base/.jpdev-shared cache moves to quarantine, add a detached linked worktree; cleanup still removes the cache while the new worktree remains registered and present; (b) after separate scratch moves to quarantine, recreate its associated removed worktree with git worktree add; cleanup still removes the scratch. The hook does not alter inventory, Git verification results or removal logic. Process evidence alone cannot replace registration/absence checks because the Git command has already exited.

Evidence: review-scratch/cache-race-probe.py and .json (cache_removed=true, new_worktree_exists=true), and review-scratch/scratch-race-probe.py and .json (scratch_removed=true, associated_worktree_recreated=true).

Required regression: revalidate cache users and associated-worktree absence after quarantine and during removal, alongside the other live eligibility evidence. Both races must refuse and retain the remaining candidate. Removing those rechecks must make the regressions fail.

Checks and scope

Read the full issue, candidate diff, review checkout's AGENTS.md and docs/policy.md, and supplied check/mutation evidence. Verified the supplied CI record is completed SUCCESS for this exact head (run 39953109633). Reused the recorded 7/7 checklist and 420/420 repository-test results; did not rerun deterministic suites. Imported lib/clean.py once successfully as the applicable build/import check.

Independently reran the supplied 43 targeted cleanup mutations against disposable source copies. 43/43 were caught, with failure logs inspected for the intended safeguard failure. The disposal-inventory mutant first fails the refusal assertion and then produces follow-on fixture errors because it deleted the candidate; it was not counted solely from an unrelated error. No supplied mutant survived.

Ran separate disposable shell probes: TMPDIR baseline passed and removing the export made the stub exit 13; source-copy baseline passed, and separately omitting TMPDIR exclusion, scratch exclusion or find pruning made their probe fail. These are four additional caught mutations. See review-scratch/shell-probes/results.json and adjacent logs.

Independent regression-probe commands, after sourcing the review scratch environment:

python3 -I -B review-scratch/independent-probes.py
python3 -I -B review-scratch/cache-race-probe.py
python3 -I -B review-scratch/scratch-race-probe.py

These scripts currently reproduce and record the unsafe outcomes; the builder should convert them into refusal assertions in repository tests. They use only newly created temporary fixtures, local Git remotes and the repository's existing process-evidence seam. No full production CLI deletion with host-wide process visibility was attempted, and no network/real GitHub PR behavior was exercised by the independent probes. Existing process, upstream, PR and other refusal coverage was assessed through the supplied evidence, focused mutants and code inspection.

All review-generated files, caches, source copies and removed fixtures were confined to the designated review scratch. No tracked edits or Git metadata writes were made in the isolated review checkout; final status was clean and HEAD unchanged. No commit, push, posting, merge, tag, release, installation or cleanup of preexisting data occurred. No unexpected permission or safeguard stop occurred. Review is complete pending the single batched fix round and a targeted reread of those fixes.

Dispositions

Fix commit: eea02d969338de33605d700f26b5e78b1dd4ec81. All changes remain in this draft PR.

Finding Disposition Verification
R1 HIGH: bare/saved Git metadata Fixed in this PR by refusing unsupported layouts at root and nested paths, including partial metadata. Real-Git refusal regressions; bare, saved and partial-metadata mutations caught.
R2 HIGH: inherited Git evidence redirection Fixed in this PR by refusing inherited evidence redirection and isolating ambient configuration. Related assume-unchanged/skip-worktree flags are refused. Alternate-index, other indirection, ambient-config and both index-flag regressions/mutations caught.
R3 MEDIUM: cache/scratch eligibility races Fixed in this PR by repeating eligibility checks after quarantine and before every removal operation. Cache and scratch probes cover creation after quarantine and after the first unlink; all four mutations caught.

The fix batch also completes the issue's transcript-size threshold: an inclusive transcript-only logical regular-file byte floor, default 10 MiB, with explicit override. Boundary/parser/forwarding/logical-byte mutations are caught. Total new mutation evidence: 18/18 caught. Focused cleanup tests: 67 passed, plus the final directory-byte refinement passed separately. Touched checklist: 4/4 passed. Original complete checklist and 43+4 mutation evidence are retained.

CI run 39957484955 passes for the fix head: all seven checklist steps and 420 repository tests. The same reviewer confirmed all three findings resolved and caught 19/19 targeted mutations, including its additional final-directory probe. No unresolved finding or design decision remains in the reviewed scope. The targeted report follows verbatim.

Targeted reread of Devkit issue #30 fixes

PR: #31

Reviewed fix head: eea02d969338de33605d700f26b5e78b1dd4ec81, against the original reviewed head 2a15dce41e3ff5dfaf54b896014264887c53b52e.

R1, R2 and R3 are resolved. No new findings in the targeted reread scope. The original report remains preserved. This reused the same independent reviewer at its original gpt-6.1-sol/high launch, under the temporary same-vendor exception; it is not cross-vendor compliance.

Dispositions

Finding Disposition Evidence
R1 — HIGH: bare/saved Git metadata bypasses publication checks Fixed in this PR. Recognized bare/saved Git layouts at candidate root and nested are refused; only a verified candidate's canonical .git metadata directory is exempt. My original bare-repository reproduction and a nested variant now refuse and preserve the unpublished refs. Root/nested saved directories and partial metadata are covered by the caught focused mutants.
R2 — HIGH: inherited Git environment hides staged index Fixed in this PR. Unsupported inherited GIT_* settings are refused, and verification isolates ambient Git config. Related assume-unchanged and skip-worktree flags are refused. My alternate-index reproduction now refuses with inherited Git evidence settings: GIT_INDEX_FILE; the actual staged path remains file. Independent hidden-flag probes retain private tracked contents. Inherited-redirection, both index flags and ambient-global-config mutants are caught.
R3 — MEDIUM: cache/scratch eligibility changes after quarantine are missed Fixed in this PR. Eligibility is repeated after quarantine, before entries and before final candidate-directory removal. Both original quarantine races now refuse and retain the candidate. Focused mutants cover races during removal. Independent last-file race probes also refuse for cache and scratch; removing only the final directory check is caught.

Transcript threshold acceptance completion

Reviewed the CLI, implementation, tests and README/policy changes for --min-transcript-bytes N. The default is 10485760 bytes, the boundary is inclusive, zero disables the floor, and only transcript records use it. Directory totals sum regular-file logical sizes and exclude directory sizes.

Independent probes used real default-sized transcript fixtures: 10 MiB minus one byte refused, exactly 10 MiB removed, and 10 MiB plus one byte removed. A small review directory was unaffected by the default. The focused threshold mutants cover the floor, inclusive comparison, default, invalid/nonnegative integer input, regular-file-only summation and forwarding the CLI value. All were caught. Documentation and behavior agree on these points.

Verification and limits

Read the bounded reread instructions, complete fix diff, repository policy, original findings and retained independent probe code. The supplied CI record is SUCCESS for this exact head: run 39957484955 (lint and tests green). Reused the supplied 67-test cleanup result, final directory-byte regression result, 4/4 touched checklist and prior unchanged full-check evidence. No deterministic full suite or broad second review was run.

Independently reran 18/18 focused fix/threshold mutants, each in a new disposable source copy. Inspected the failure logs: the ambient-config mutation fails because Git reads the deliberately invalid fixture config; the strict threshold mutation fails on the intended equality cases. The other rows fail their relevant assertions. Additionally, removing only the final candidate-directory check was caught by my last-file cache race probe. 19/19 targeted mutations caught; none survived.

Independent probes and outputs are under review-scratch/reread/:

  • probes.py, probes.json, probes.log: original finding reproductions converted into refusal/retention checks, hidden index flags and default transcript boundaries.
  • final-probes.json, final-baseline.log: cache and scratch races after the last file removal.
  • mutations.py, mutations.log, mutants/matrix.json and per-row logs: the 18 supplied focused mutations, adapted to reviewer paths.
  • final-directory-mutant.py, final-directory-mutant.log, final-directory-mutation.json: the separately caught final-directory safeguard mutation.

The probes import the fix source and use the existing process-evidence seam; they perform real filesystem/Git operations only on new disposable fixtures. They do not establish production host-wide process visibility or real GitHub/network behavior. The previously recorded product limits, including conservative refusals and possible partial removal with remaining evidence retained, still apply.

Final isolated-checkout status is clean and HEAD is unchanged at the reviewed fix head. No tracked edits, Git writes in the review checkout, posting, commits, pushes, merges, tags, releases, installation or cleanup of preexisting data occurred. No unexpected permission or safeguard stop occurred. This targeted reread is complete.

@kikashy
kikashy marked this pull request as ready for review October 23, 2026 13:16
@kikashy
kikashy merged commit 72afbd5 into main Oct 23, 2026
3 checks passed
@kikashy
kikashy deleted the codex/issue-30-safe-clean branch October 23, 2026 13:17
@github-actions

Copy link
Copy Markdown

🎉 Thank you for your contribution to Judgment Pack!

If you’d like another task, choose a path that interests you:

@kikashy kikashy mentioned this pull request Oct 23, 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.

jpdev clean: remove provably safe scratch, and a cleanup rule in the policy

1 participant