Skip to content

Add isolated SwiftPM scratch placement and guarded cache cleanup - #524

Open
morluto wants to merge 8 commits into
repoprompt:mainfrom
morluto:agent/421-422-cache-lifecycle
Open

Add isolated SwiftPM scratch placement and guarded cache cleanup#524
morluto wants to merge 8 commits into
repoprompt:mainfrom
morluto:agent/421-422-cache-lifecycle

Conversation

@morluto

@morluto morluto commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Why

Worktrees accumulate independent multi-gigabyte .build directories. Developers cannot place those caches under one controlled root or safely identify abandoned caches.

Sharing one mutable scratch directory across divergent branches would trade disk pressure for correctness risk. This PR uses conservative worktree isolation, then builds cleanup on the same cache identity model.

Closes #421 and #422.

What changes

Placement

Adds REPOPROMPT_DEVELOPER_SWIFTPM_SCRATCH_ROOT. Conductor derives an isolated path using repository, worktree, toolchain, destination, target triple, and configuration identity.

The existing exact-path REPOPROMPT_SWIFTPM_SCRATCH_PATH remains supported. Unset behavior stays checkout-local.

Inventory and cleanup

One model owns effective path, identity, size, age, eligibility, and skip reasons. Cleanup defaults to dry-run. Mutation requires --apply --confirm and immediately revalidates current/dirty/active/live-bound state, confinement, symlinks, and identity.

Symlinked cache paths are skipped without resolving and deleting their targets. Cleanup never deletes worktree sources or Git metadata.

Compatibility

Packaging, local installation, and Xcode MCP workflows use the same scratch authority. The Xcode MCP compatibility link is refreshed after successful builds so it cannot retain an older executable.

Deliberate scope

This does not share one exact build directory across divergent worktrees. Isolation comes first; broader artifact sharing needs separate evidence.

Review order

  1. Scripts/cache_inventory.py identity model
  2. conductor and packaging propagation
  3. cleanup planning/revalidation
  4. safety tests
  5. Xcode/local-install compatibility wiring

Verification

  • cache inventory/cleanup tests
  • conductor self-tests
  • packaging-path focused tests
  • guardrails
  • contribution commit/push preflights

morluto added 4 commits July 12, 2026 02:49
- Add Scripts/cache_inventory.py to resolve SwiftPM scratch paths by
  repo/worktree + toolchain + target triple + configuration.
- Wire scratch identity into conductor build/package/test/run/relaunch and
  install-debug-cli operations, and into package_app.sh and
  xcode_developer_workflow.sh.
- Expand/resolve exact paths and developer-root paths, include SWIFT_TARGET_TRIPLE
  in cache key, and resolve/exclude symlinked managed worktree roots.
- Drop the cache-identity sentinel; keyed directories are identified by repo-hash
  prefix.
- Fix local production install path to use the package_app .build/release
  compatibility path and resolve it through symlinks.
- Update test harnesses to match the new symlinked layout.
- Add plan_build_cache_cleanup and execute_build_cache_cleanup to
  cache_inventory.py with dry-run/apply/confirm and --limit support.
- Re-validate each directory before rmtree: must still exist, not be a symlink,
  not have an active SwiftPM .lock, not be live-bound to the worktree, remain
  within the resolved scope (repo root, developer root, or exact path), and the
  worktree must not have become dirty.
- Wire the "cache cleanup" subcommand into conductor (parser, OperationRegistry,
  internal runner, help) with default limit=None.
- Add Scripts/test_cache_inventory.py and include it in conductor-selftest.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@baron

baron commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Current-main refresh on exact head 1113bcbd52e445502f368931438be1828bf332fe: the isolated SwiftPM scratch/cleanup goal is still relevant, but the branch conflicts in the current Makefile/conductor authority and historical shards 2/4 are red. Please refresh onto current conductor semantics and rerun the safety, cleanup, and full relevant CI lanes before substantive re-review.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Audit disposition — destructive-safety blocker (2026-08-14)

The exact scratch path is canonicalized with resolve(strict=False) before cleanup inventory and symlink classification. That erases the fact that the configured path was a symlink, so the later is_symlink() guard can inspect the resolved target and permit deleting it—contrary to the PR’s stated safety contract.

Please retain the original path, lstat every path component immediately before deletion, reject any symlink/reparse transition, and revalidate target identity and confinement under the authorized root. The cache identity should also include actual Swift/Xcode/SDK, architecture, and deployment-target identity rather than only environment paths that can remain stable across upgrades. I would not merge cleanup code with this path-provenance loss.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Deep-review assessment — 2026-08-14

Disposition: destructive-safety blocker. resolve_swiftpm_cache_identity resolves an exact configured scratch path before inventory. If the configured path is a symlink, that loses the lexical/symlink identity; later is_symlink() checks see the resolved target and the target can become eligible for shutil.rmtree. This contradicts the PR’s promise that symlinked cache paths are skipped without following them.

Preserve the original path, use lstat/openat-style no-follow checks for the leaf and every ancestor at both plan and apply time, and delete only the exact verified directory entry. Also strengthen cache identity with actual compiler/toolchain and SDK/deployment-target versions; hashing environment paths alone will not distinguish an upgraded Xcode installed at the same location.

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.

Opt-in developer-controlled SwiftPM scratch root for conductor builds across worktrees

2 participants