[brettheap]

Lane: openreposhape-1 (openRepoShape-1)
Claim: #526 (comment)

What

Three changes, one for each line of #526.

1. The work directory a run makes (#457, body: _work_root). Without --work-dir, plan, check and execute each make an openreposhape-adopt-* directory in the system temp folder. However the command ends (success, refusal, failed command or exception), main now removes that directory where it holds no file. "No file" means nothing at all, or only empty directories: a refused gh repo clone leaves an empty source-readonly/. Only os.rmdir removes, so nothing written there is ever deleted. A directory that holds something stays: the clone a plan --source <org/repo> records, and the legs and split execute built. Where execute's stays, its last line names it, on stdout after a success and on stderr after anything else:

NOTE the work directory this run made, `<path>`, is kept, because it holds what the run made there: delete it once nothing in it is needed.

A --work-dir the person gave is never touched. An exception still reaches the caller as it was raised.

2. A recorded clone that is gone (#457, comment 6134291709: Plan.open_source). plan --source <org/repo> records its temporary clone as source.local_path. When that path no longer exists and the plan records a source.repository, check and execute (given no --source) now clone that repository again, as --source <org/repo> would, and say so once:

NOTE the plan's `source.local_path`, `<path>`, no longer exists, so its `source.repository`, testorg/Thing, is cloned again, as `--source testorg/Thing` would clone it.

A clone made again at another commit is not the plan's source. check finds plan-stale, and execute refuses it before any leg exists, each naming both commits. This is the same check --source already meets.

The same holds for a plan of a local clone that has since been moved or deleted. plan records as source.repository the org/repo that the clone's origin URL ends in, whatever its host, so check and execute now ask gh to clone that name, and plan-stale holds what comes back to the plan's commit. On main, both refused source-unclonable there too, with gh's parse error.

3. The suite's temporary directory (#461, body). A session autouse fixture in tests/conftest.py, temporary_directory, points TMPDIR (for every child) and tempfile.tempdir (for this process) at <basetemp>/tmpdir. That is one level under the session's basetemp, not one per test, to keep paths short for the Windows runner. Suites that give a child a TMPDIR of their own still do, because each lays it over os.environ for that child.

Why (repro on main)

Measured on main 2247dbf, with TMPDIR at a fresh directory, the conftest source, and --local-remote-dir in place of GitHub. A gh stub stood in for gh repo clone by cloning a repository on disk and setting origin to the GitHub URL, as gh does.

run	main	this branch
plan of a local source	1 empty dir left	none
check of a local source	1 empty dir left	none
execute refused before building (leg-remote-exists)	1 empty dir left	none
execute that succeeds	1 dir (split and both legs, 511 entries), not mentioned	kept, and named on the last line
plan --source testorg/Northwind	1 dir holding the recorded clone	the same (kept)
check of that plan, clone intact	1 empty dir left	none
check, recorded clone deleted	exit 2, REFUSED source-unclonable: could not clone /…/source-readonly/Northwind: expected the "[HOST/]OWNER/REPO" format …, then Remediation: check gh auth status …	exit 0, the NOTE, plan ok
execute --yes, recorded clone deleted	exit 2, the same refusal	exit 0, adoption verified
check, clone deleted, source moved on	exit 2, source-unclonable	exit 1, FINDING plan-stale: … written against 351afab849a6 but main is now at 797454f69579

check and execute handed the vanished PATH to gh repo clone as if it were an org/repo. Passing --source testorg/Northwind again recovered on main, and still does.

One run of tests/test_adopt_pins.py on main (107 passed) left 117 empty openreposhape-adopt-* directories in the system temp folder. On this branch, every targeted run below left 0 in a stand-in system temp folder: the stand-in held no entry at all at the end of each run. The two halves were also measured apart on tests/test_adopt_pins.py. With only the conftest fixture (the tool's change reverted), the 117 empty directories land in the session's <basetemp>/tmpdir and 0 in the system temp folder. With both, none are left in either: every session <basetemp>/tmpdir of the targeted runs held 0 openreposhape-adopt-* at the end.

How

• _work_root(args). It keeps the directory it makes on args.made_work_root. A --work-dir is never recorded there.
• main. Its existing try gets a finally that calls _leave_the_work_root(args, ended == 0). That runs after the except handlers have printed, so the NOTE is the last line the tool prints; after a defect or Ctrl-C, Python's traceback still follows it. No handler moved.
• _removed_if_it_holds_no_file. First _holds_a_file walks the tree read-only. A file, a symlink, or a directory that cannot be listed keeps everything. Only then does it os.rmdir deepest first. A root that is already gone counts as removed.
• _leave_the_work_root. It prints the kept line for execute only, after _flush_stdout(), inside contextlib.suppress(OSError, UnicodeError). A closed pipe or an encoding problem cannot replace the command's own exception.
• Plan.open_source. Where no --source was given, _where_the_clone_is_gone replaces a local_path that does not exist with the plan's source.repository, and prints the NOTE. _spelled and _shown spell both values. A plan that records no repository is unchanged, so it still meets Source.open's existing refusals. test_adopt_template_values.py's source-unresolvable cases for a hand-edited local_path hold it to that.
• adopt-project.py: check finds, and execute refuses before any leg exists, a path the root keeps at the shape's own pin and a --work-dir this user cannot write in, so contracts/shape-pin.yaml kept in the root and a chmod 500 work directory no longer pass check and fail only after both legs are pushed (#524) #581 (adopt-project.py check: refuse a kept file at the shape's pin path, and an unwritable --work-dir, before any leg exists #524). That PR adds _unwritable_work_dir beside the _work_root calls in cmd_check and cmd_execute. This branch changes neither call site. git merge-tree --write-tree 652b6863 edaaae49 is clean, against adopt-project.py: check finds, and execute refuses before any leg exists, a path the root keeps at the shape's own pin and a --work-dir this user cannot write in, so contracts/shape-pin.yaml kept in the root and a chmod 500 work directory no longer pass check and fail only after both legs are pushed (#524) #581's head, which has merged main a2243f1, and the merge passes (see Tests).

Tests

tests/test_adopt_temp_work_roots.py, nine tests, all written first and failing on main:

• test_plan_and_check_remove_the_empty_work_directory_they_made: a --work-dir given stays.
• test_a_check_with_findings_and_a_refused_execute_leave_none: exit 1 and exit 2, and no kept line.
• test_a_defect_still_removes_the_empty_work_directory_and_propagates: a RuntimeError mid-check reaches the caller.
• test_a_refused_clone_leaves_no_work_directory: the empty source-readonly/.
• test_a_refused_execute_says_which_work_directory_it_keeps: the last stderr line.
• test_an_execute_that_built_keeps_its_work_directory_and_says_where: the last stdout line. Needs git filter-repo.
• test_check_and_execute_clone_the_recorded_repository_again: no gh call while the clone is there, one NOTE each after it is gone.
• test_a_clone_made_again_at_another_commit_is_stale: plan-stale naming both commits, and no leg directory.
• test_the_suite_keeps_its_temporary_directory_in_the_basetemp: TMPDIR, tempfile.gettempdir() and a child's, all <basetemp>/tmpdir.

The runs that clone an org/repo run in-process, with adopter.run standing in for gh repo clone, so they run on Windows too, where a #! stub cannot.

Revert table (each decision reverted alone, in a scratch worktree, the nine tests run):

reverted	fails
1, the work directory (_work_root, the three helpers, main's finally)	6: the first six above
2, a gone clone (_where_the_clone_is_gone and its call)	2: …clone_the_recorded_repository_again, …another_commit_is_stale
3, the conftest fixture	1: …keeps_its_temporary_directory_in_the_basetemp
all three	9

Targeted runs at 652b686, dirty 0, /usr/bin/python3 -m pytest, every one EXIT=0. The set is every test file the corrected grep adopt-project|adopt_project|\bADOPT\b names (71), plus every suite that sets TMPDIR (test_cli_reference, test_family_add_refusals, test_leg_path_yaml_scalars, test_openreposhape_command, test_scaffold_leg_path_collisions, test_scaffold_leg_paths, test_scaffold_manifest_values, test_scaffold_pin_collisions, test_setup_sh, test_shape_doctor, test_tracking_branch_ref): 76 files.

mode	passed	skipped
git-filter-repo on PATH	6,034	13
hidden (PATH=<stub>:/usr/local/bin:/usr/bin:/bin)	5,720	327

Each run left 0 openreposhape-adopt-* (0 entries at all) in its stand-in system temp folder.

On the merge with #581's head edaaae4 (a scratch merge commit), the new tests plus test_adopt_shape_pin_and_work_dir.py and the four test files #581 changes all pass: 316 passed on PATH, and 264 passed with 52 skipped hidden.

The suites that set TMPDIR themselves still mean what they did. Each passes its own TMPDIR in its child's environment, laid over os.environ, so it outranks the session's. test_cli_reference's tempfile.gettempdir() placeholder and test_setup_sh's openreposhape-shape-* count both read the in-process tempfile. That now names the same folder their children inherit, as it did before.

Windows

• Newline emulation. The nine tests pass under a plugin kept outside the repository that makes text-mode Path.write_text and text-mode stdin write \r\n. On PATH: 9 passed, 21 line ends translated. Hidden: 8 passed, 1 skipped, 18 translated. No test counts \n in bytes. The tests write files only through make_source_repo and write_bytes; the plan is the tool's own LF file.
• Paths. Every path and branch a test makes is NTFS-legal. The NOTE and the kept line spell a path with _spelled, and the tests compare with conftest's spelled, so a Windows path's backslashes are escaped on both sides.
• MAX_PATH. TMPDIR is <basetemp>\tmpdir, one short segment under the basetemp: as deep as each test's own tmp_path, and shorter than its name.
• tempfile reads TMPDIR before TEMP and TMP on every platform, so TMPDIR alone moves Python's temp folder there. TEMP and TMP are left alone for native children.
• A vanished Windows local_path. On main, C:\…\source-readonly\Thing was refused source-unresolvable by WINDOWS_ABSOLUTE_RE, as --source '…' is neither a path that exists nor an org/repo name. That named a --source the person never passed. Where the plan records a repository, it now takes the same NOTE-and-clone path as on POSIX.

Not in this PR

• A clone no plan records stays. Such a clone is: a check or execute that clones again because the recorded clone is gone; any --source <org/repo> run, as on main; and a plan --source <org/repo> refused after its clone. Each keeps the clone in its own temp directory. check never rewrites the plan, so a second check clones again. Decision 1 removes only a directory that holds no file.
• A vanished local_path with no recorded repository. This is a local source whose origin names no repository. On POSIX it is still handed to gh repo clone as an org/repo and refused source-unclonable with gh's parse error. On Windows it is still source-unresolvable, naming --source. Listed for Tracking: adopt-project.py, converting an existing repository in place #457's checklist.
• No README, AGENTS.md or docs/cli.md change. No help text changed, and none of the three documents names these directories.

Known limits

• Only execute prints the kept line. plan --source <org/repo> already prints cloning <repo> (read only) into <path> and records the path. A check that clones again prints the same cloning line.
• A caller that runs cmd_plan, cmd_check or cmd_execute directly never reaches main's finally. That covers tests that run them in-process. Where such a test sets no temporary folder of its own, its directory lands in the session's <basetemp>/tmpdir, which pytest keeps or removes with the rest; in the targeted runs, none did.
• The session's <basetemp>/tmpdir collects every directory that is kept, for example each successful execute without --work-dir, until pytest's retention removes the basetemp.

Closes #526

This closes, on #457, the body's line on _work_root (each plan, check and execute leaves an openreposhape-adopt-* temp dir) and the line from comment 6134291709 (plan --source org/repo records a temp clone, and check and execute refuse source-unclonable once it is cleaned). On #461 it closes the body's line on tests/conftest.py having no TMPDIR fixture. Each is marked → #526 there; tick them when this closes.

🤖 Generated with Claude Code

Summary by Sourcery

Ensure adoption runs clean up disposable temporary directories and recover safely when a plan’s recorded source clone is no longer available.

New Features:

• Reclone a plan’s recorded repository when its temporary source clone has disappeared, while detecting changes in the recloned commit as stale plans.

Bug Fixes:

• Clean up automatically created adoption work directories when they contain no files, while preserving and reporting directories containing useful run output.

Enhancements:

• Route the test suite’s temporary files through pytest’s retained session temporary directory for more reliable cleanup and shorter Windows paths.

Tests:

• Add coverage for work-directory cleanup, preserved execution artifacts, vanished source clones, stale reclones, exception handling, and test temporary-directory configuration.

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 8 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[brettheap]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-15T14:46:13.825879Z	652b686	Manual request

ℹ️ 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.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[sourcery-ai]

Reviewer's Guide

The PR prevents empty automatically created adoption work directories from accumulating, recovers from deleted recorded source clones by recloning and validating the repository commit, and confines test temporary files to pytest's session directory, with comprehensive regression tests for cleanup, output, failure paths, and Windows-oriented behavior.

Sequence diagram for adoption work-directory cleanup

sequenceDiagram
    participant Command as adopt-project
    participant Work as WorkDirectory
    participant Output as stdout/stderr

    Command->>Work: _work_root(args)
    Work-->>Command: temporary path and made_work_root
    Command->>Command: run plan, check, or execute
    Command-->>Command: success, refusal, or exception
    Command->>Work: _leave_the_work_root(args, succeeded)
    Work->>Work: _holds_a_file(root)
    alt empty or empty directories only
        Work->>Work: _removed_if_it_holds_no_file(root)
    else files or symlinks exist
        Work->>Output: execute: print kept-directory NOTE
    end

Loading

Sequence diagram for recloning a vanished recorded source

sequenceDiagram
    participant CheckExecute as check or execute
    participant Plan as Plan.open_source
    participant GH as gh repo clone
    participant Source as Source validation

    CheckExecute->>Plan: open_source(None, work_root)
    Plan->>Plan: _where_the_clone_is_gone(local_path)
    alt recorded local_path exists
        Plan-->>CheckExecute: use local_path
    else local_path is gone and source.repository exists
        Plan-->>CheckExecute: print reclone NOTE
        Plan->>GH: clone source.repository
        GH-->>Plan: recreated source clone
    end
    Plan->>Source: validate source at plan commit
    alt commit differs
        Source-->>CheckExecute: plan-stale
    else commit matches
        Source-->>CheckExecute: source accepted
    end

Loading

File-Level Changes

Change	Details	Files
Added lifecycle cleanup for automatically created adoption work directories while preserving any generated files and reporting retained execute artifacts.	Record only tool-created temp roots for cleanup. Detect files, symlinks, and unreadable content before removing empty directory trees. Run cleanup from main's finally block across success, refusal, command failure, and exceptions. Print the retained execute directory on the appropriate final output stream.	adopt-project.py
Made plans recover when their recorded temporary source clone has disappeared by recloning the recorded repository.	Replace a missing recorded local path with the plan's repository only when no source override is supplied. Emit a single explanatory NOTE and reuse existing source commit validation for stale reclones. Preserve existing behavior for plans without a recorded repository or with an existing clone.	adopt-project.py
Isolated the test suite's temporary files under pytest's retained session basetemp.	Add an autouse session fixture redirecting child TMPDIR and in-process tempfile.tempdir to basetemp/tmpdir. Restore environment and tempfile state after the session while allowing per-child TMPDIR overrides.	tests/conftest.py
Added focused regression coverage for work-root cleanup, source recloning, stale-plan handling, output ordering, exception propagation, and temporary-directory configuration.	Test empty-root removal versus retained artifacts and user-provided work directories. Test vanished clone recovery, commit mismatch behavior, and no premature leg creation. Test cross-process and in-process temporary-directory routing.	tests/test_adopt_temp_work_roots.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#526	Remove automatically created work directories when they contain no files after plan, check, or execute completes, while preserving user-supplied work directories and nonempty roots; identify a nonempty root retained by execute.	✅
#526	When a plan’s recorded local source clone is missing and the plan records its repository, let check and execute clone that repository again and apply the existing stale-plan checks if the clone is at a different commit.	✅
#526	Point the test suite’s temporary directory at a directory under pytest’s session basetemp, for both child processes and in-process tempfile use.	✅

Possibly linked issues

• adopt-project.py: remove the temp work roots it makes without --work-dir, keep a recorded --source clone, and point the tests' TMPDIR at their basetemp #526: The PR directly implements all three issue lines: removes empty temporary work roots, reclones missing recorded sources, and points tests’ TMPDIR at basetemp.
• adopt-project.py: remove the temp work roots it makes without --work-dir, keep a recorded --source clone, and point the tests' TMPDIR at their basetemp #526: The PR addresses adopt-project.py: remove the temp work roots it makes without --work-dir, keep a recorded --source clone, and point the tests' TMPDIR at their basetemp #526’s empty work-directory leak and failed source-unclonable behavior by cleanup and recloning the recorded repository.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud