Repository navigation
fix(core): save a location's project only after its folder is found - #59001
andreivince wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good. I ran the reproduction from #57988 against a server built from source, on the base and with this change. On the base, a 404 location request for a missing folder still saves a project, and it survives a restart. With this change, nothing is saved, and the project appears once the folder exists. The new tests fail without the fix and pass with it; the core location and project tests and tsc in packages/core and packages/server pass.
Public API is unchanged: no HTTP routes, SDK, events or config change.
The new node that saves the project depends on FileSystem, so a missing folder fails the boot before anything is written. Workerd keeps saving projects, and activity tracking from session events stays in LocationLifecycle. Resolving the project twice per boot (once without saving in Location, then again to save it) costs little thanks to the git-discovery cache. No blocking concerns.
There was a problem hiding this comment.
The new commit only merges dev; the pull request's own changes are unchanged. I re-ran the #57988 reproduction on the new head: requests for the missing folder still return 404 and save nothing, and the project appears once the folder exists. The server tests in fetch.test.ts and workerd.test.ts pass, and tsc passes in packages/core. Public API is unchanged. No new concerns.
There was a problem hiding this comment.
The new commit only merges dev (including #58997); the pull request's own changes are unchanged and combine cleanly with #58997's test changes. I re-ran the #57988 reproduction on the new head: requests for the missing folder still return 404 and save nothing, and the project appears once the folder exists. The server tests in fetch.test.ts and workerd.test.ts pass, and tsc passes in packages/core and packages/server. Public API is unchanged. No new concerns.
Issue for this PR
Closes #57988
Type of change
What does this PR do?
A request for a folder that no longer exists already returns 404, but booting the location saved its Project first, so the missing folder showed up in
GET /api/projectand stayed there after a restart. Existing rows for missing folders also gottime_activebumped on every such request.Locationnow resolves the Project without saving it, and a newLocationProjectnode saves it and records activity onceFileSystemhas accepted the folder. Profiles whoseFileSystemdoesn't check the host, like workerd, still save projects, and creating a session in a missing folder is unchanged. This moves where the Project gets saved during a location boot, so a maintainer's view on the placement is welcome. The Desktop "recently closed" side of the issue isn't covered here.#58997 stops session routes from booting a Location at all; this PR only changes what a Location boot saves, so the two don't overlap in behavior (they touch
instance.tsandfetch.test.tsin different places).How did you verify your code works?
New tests in
packages/server/test/fetch.test.ts(a missing folder is not listed, and is listed once it exists) andpackages/server/test/workerd.test.ts(workerd locations are still saved), plus atime_activecheck in the existing missing-folder test. The fetch test and thetime_activecheck fail on dev and pass now. I also ran the issue's steps against a realopencode serve, including a restart.bun run testandbun typecheckpass inpackages/serverandpackages/core, apart from ripgrep and bash tests that fail on this Mac on dev too.Screenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #51197, #57987, #50951