Repository navigation
fix(workspace): defer installs and make dependency cleanup visible - #398
lastobelus wants to merge 8 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89f7ac2df3
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1578c5569
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 338b9b383e
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
338b9b3 to
68c29f1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68c29f1fac
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2e3cdfe to
4001050
Compare
68c29f1 to
e8c77c3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8c77c3b5c
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| export const StorageCleanupReportEntry = Schema.Struct({ | ||
| kind: Schema.Literals(["worktree", "browser-artifacts", "logs"]), | ||
| kind: Schema.Literals(["worktree", "dependencies", "browser-artifacts", "logs"]), |
There was a problem hiding this comment.
When an older web, desktop, or mobile client connects to an updated remote server and dependency cleanup emits this new "dependencies" value, that client's strict three-literal schema rejects the entire server.getStorageCleanupReport subscription or cleanup-run response. The existing storageCleanupRun capability does not distinguish this response version, so version-skewed clients can report cleanup as failed even though it ran; preserve an older wire representation or add versioned capability/response handling before emitting the new literal.
AGENTS.md reference: AGENTS.md:L73-L75
Useful? React with 👍 / 👎.
Keep worktree creation dependency-free, and run locked installation through a managed Action before dependency-backed work. Carry-Group: build-ci Carry-Observation: Setup links environment files; preparation intentionally does not attest to the integrity of every existing package file.
Include dependency outcomes and size estimates, cover configured managed folders, and refresh live provider ownership without losing unfinished shutdown protection. Carry-Group: incubator Carry-Observation: The cleanup report, provider ownership guard, and removal eligibility are one Maintenance behavior; keep their validation together.
Use the repository test runner and the Action reporting convention so dependency preparation is covered by the standard CI suites. Carry-Group: build-ci
Canonicalize dependency ownership and active-work guards while rejecting worktree symlinks and changed locations. Keep aggregate recovery labeled as an estimate even when report entries are truncated. Carry-Group: incubator
Resolve the primary helper inside Node while preserving the target checkout and child exit status. Sequence dependency preparation and CI without shell-specific command chaining. Carry-Group: build-ci
Read and validate the target manifest’s exact pnpm version before its frozen-lockfile install. Older checkout pins no longer inherit the primary checkout’s version requirement. Carry-Group: build-ci
Describe the target manifest’s pnpm version requirement when preparing older worktrees. Carry-Group: build-ci
…ents Report dependencies as worktree content with optional open-ended resource detail and explicit reasons. Older report readers keep the existing categories, while current clients show dependency labels and estimates. Carry-Group: incubator
e8c77c3 to
a3610ac
Compare
1 participant
New worktrees attempted an eager install before their environment setup, and cleanup reports omitted dependency removals. This keeps setup cheap, adds managed on-demand preparation before CI, and reports dependency outcomes and estimated reclaimed sizes. The setup and preparation launchers resolve helper paths inside Node and sequence CI without shell-specific expansion or chaining. Preparation reads the target checkout's exact pnpm version pin and lockfile, including on older branches.
Dependency cleanup now includes configured and previous worktree folders, including symlinked parent folders. Worktree symlinks are still refused, and shared ownership and active-work guards use the canonical location. Reports keep the existing three wire categories and describe dependencies through optional open resource detail and explicit reasons, so existing clients can read them. Aggregate recovery figures remain labeled as estimates even when individual report entries are capped. Provider protection refreshes when ownership changes, ignores stale persisted sessions, and retains live or unfinished runtimes even when attachment persistence failed. Thread inactivity rules and the existing safety checks remain in place.
Validation: focused cleanup/dependency regression tests, provider lifecycle tests, dependency-free bootstrap tests, scoped server/web/contracts type checks, and targeted lint passed. An isolated browser pass verified that a dependency-only rule enables Delete now, reports the removal, and preserves the worktree and tracked source.
The declared launchers were also exercised through zsh with paths containing spaces and preparation/CI failure propagation. The report fix passed 306 cleanup/settings tests, including historical report decoding and unknown resource details, targeted lint, and exact-head Quick CI. The isolated browser pass was repeated with the actual dependency-removal response. A PowerShell process was unavailable for local Windows execution.
Before: dependency-only cleanup disabled the manual button. After: it is available and the removal is reported.
The aggregate estimate label after the review fixes:
Implemented with GPT-6.1-Sol through the Codex harness.