Repository navigation
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes the flow in #61009 (git init and the first commit with nothing in between). It misses a common variant, though, so the bug can still happen.
I ran the issue's script with each checkout's CLI from source. On the base, location.get keeps the directory-hash id and session list returns []. With this PR, the location reports the root-commit id and both sessions are listed. Then I changed the script to fetch /api/location once between git init and the commit. The location is rebuilt as a git location with project id global, and since it's no longer markerless it's never checked again. After the commit it still reports global, and session list returns []. This happens whenever an agent or a client opens the location between the two steps, for example an agent running git init and git commit as separate tool calls. Details inline.
Also inline: the rebuild runs the location's shutdown (pending permissions, forms and RPC are closed, and a shutdown event is published) whenever the markers change, and every borrow of a markerless location now walks up to the filesystem root.
Tests in location-layer and location-activity pass, and typecheck:tsc passes. The new test only covers init and commit with no borrow in between. A case that borrows between git init and the commit would catch the problem above.
Public API: unchanged.
| build.close = lifecycle.shutdown | ||
| // Remember whether this graph resolved a VCS identity, and which marker it saw. | ||
| // A markerless graph is the only one that can become wrong later. | ||
| build.markerless = Context.get(exit.value, Location.Service).vcs === undefined |
There was a problem hiding this comment.
Only markerless graphs are ever checked again. After git init and before the first commit, Project.resolve returns vcs git with id global (no root commit yet). If anything borrows the location in that window, it's rebuilt as a git graph with markerless = false, and after the commit it never updates. I reproduced this: an extra GET /api/location between git init and git commit leaves the location on global, and opencode session list returns [] again. Could the check compare the project identity instead of whether a marker exists? For example, record the resolved id and re-resolve when it was global or markerless, or react to Worktree.Event.Resolved/previous/adopted for this location's project.
There was a problem hiding this comment.
Fixed. The map now records the resolved Project id instead of markerless-ness. A graph is re-resolved before the next borrow when its id was global or it resolved no VCS, and it is invalidated only when that id actually changes. Your reproduction (init → borrow → first commit) now ends on the commit id, and packages/core/test/location-layer.test.ts covers that exact sequence: the borrow between init and commit resolves global, then the next borrow reports Project.ID.make(head). Graphs that pinned a VCS identity are still untouched.
| // opencode already opened). Nothing re-resolves a cached graph, so a markerless one is | ||
| // dropped as soon as the markers above it change and the next borrow rebuilds it with the | ||
| // identity its sessions are stored under. | ||
| const refresh = (key: Location.Ref) => |
There was a problem hiding this comment.
Two concerns with doing this on every borrow:
droprunslifecycle.shutdown, which closes pending permissions, forms and RPC and publishesLocationEvent.Shutdown. That's the same as an explicit reload. When the agent itself runsgit initin a markerless folder, the nextInstance.provideborrow during that turn would shut down the live location under the running session. Please check that case, or move the rebuild to a point where the location is not busy.refreshwalks up to the filesystem root with synchronousexistsSynccalls on everyget/contextEffect/contextEffectOptionof a markerless location, including eachInstance.provide. That blocks on slow or network filesystems (see the Windows work in fix(core): speed up location boot and VCS routes on Windows #60569). A trigger based on events (project resolution changing the id) would avoid both problems.
There was a problem hiding this comment.
Both addressed.
- Not on a busy location: refresh returns early while the RcMap entry has borrowers (
refCount > 0) or is retained by an open socket, and the detach is a plaininner.invalidate— nodrop/lifecycle.shutdown. The old graph keeps its permissions/forms/RPC and is released when its last borrower lets go, so the step that runsgit inithands off at its boundary instead of being shut down mid-turn. Added a test that a borrowed location keeps its identity until the borrow is released. - No walk, no sync block: the probe is two async
fs.accesscalls for.git/.hgin the location directory itself, and only for graphs whose identity was provisional. A marker above the directory was already visible when the graph resolved (otherwise it would not have been markerless), so there is no upward walk. The real resolution only runs once a marker appears, or forglobal, whereProject.resolvehits the git fingerprint cache.
To re-resolve without widening the map layer dependencies, the location graph exposes resolveProject through LocationLifecycle, which already owns Project.Service. bun test test/location-layer.test.ts: 26 pass; the other location tests and bun run typecheck:tsc pass.
e4ef655 to
7c08ce6
Compare
There was a problem hiding this comment.
Both earlier points are addressed:
- A location that had no VCS, or that resolved as
global(repo with no commit yet), is now re-resolved before the next borrow. I re-ran the #61009 script on the base and with this PR, plus a variant that reads/api/locationbetweengit initand the first commit. With the PR, the location ends on the root-commit id andopencode session listshows both sessions. On the base it returns[]. - Borrowed or retained locations are left alone, the rebuild invalidates without running
lifecycle.shutdown, and the check is two async file probes in the folder itself.
The location-layer, location-activity and location-retain tests pass, and typecheck:tsc passes.
Public API: unchanged. The only addition is resolveProject on the internal LocationLifecycle service.
One small gap remains (inline): a parent folder becoming a repo after a subfolder was opened is not picked up.
| if (held && held.refCount > 0) return Effect.void | ||
| return Effect.gen(function* () { | ||
| // Without a VCS the identity only changes once a marker appears in this directory. A | ||
| // marker above it was already visible when the graph resolved, so there is no walk here. |
There was a problem hiding this comment.
A marker above the folder is only seen if it existed when the graph was built. If someone opens proj/src and then runs git init in proj, this check never finds a marker, so the location keeps its directory-hash id, the same #61009 symptom for a subfolder. Not blocking, but cheap to handle: compare against Project.root with async fs.access. That is one walk, and only for graphs with no VCS.
There was a problem hiding this comment.
Handled in c20ce8d. For a graph with no VCS, refresh now calls Project.root with the async fs.access probe instead of looking only at the folder, so a marker that appears at or above the location invalidates it. Graphs that resolved a VCS still skip the walk entirely.
Added a regression test for your case: open proj/src (no repo anywhere), run git init in proj, and the next borrow resolves the git identity — Project.ID.global until the first commit pins the root commit, same as the location opened directly in the repo.
bun test test/location-layer.test.ts: 27 pass, 0 fail. The other location tests and bun run typecheck:tsc pass.
7c08ce6 to
c20ce8d
Compare
There was a problem hiding this comment.
Thread 3 is addressed: a location with no VCS now uses Project.root with async file checks, so a repo created in a parent folder after a subfolder was opened is picked up, and the new test covers it. The changes for threads 1 and 2 are still in place.
I re-ran the #61009 script on the base and with this PR, plus the variant that reads /api/location between git init and the first commit. With the PR, the location ends on the root-commit id and opencode session list shows both sessions. On the base it returns []. The location-layer, location-activity and location-retain tests pass, and typecheck:tsc passes.
Public API: unchanged.
No new concerns.
A location graph is cached for its whole life and resolves its project once, so opening a folder before `git init` pinned the markerless identity and its sessions kept being reported under it. Record the resolved project id and re-resolve before a borrow only when the identity could not be pinned yet: a directory without a VCS marker, or a repository without a commit or remote (`global`). The graph is invalidated only when that id changes. Without a VCS, Project.root decides whether a marker appeared at or above the location with async probes, which also covers opening a subfolder before its parent becomes a repo. The check never runs while the location is borrowed or retained, and the detach is a deferred invalidation instead of `lifecycle.shutdown`, so a running step keeps its graph until it hands off. Snapshot capture is gated on the location's VCS, so re-resolving also restores it after the first commit; undo/redo no longer rolls back only the conversation.
c20ce8d to
f7c56fa
Compare
|
Added a regression test for the second symptom reported in #61009 today: the stale location also disables snapshot capture, because Local check on the Also noting #61249 ( |
There was a problem hiding this comment.
The new commit only adds a test, which checks that snapshot capture works again after the folder becomes a git repo. The fix itself is unchanged since my last review, where the #61009 script passed with this PR and failed on the base. location-layer tests pass, including the new one, typecheck:tsc passes, and the PR still merges cleanly into the current base.
Public API: unchanged.
No new concerns.
Issue for this PR
Closes #61009
Type of change
What does this PR do?
A location graph caches its Project, so opening a folder before
git initpinned the markerless identity:opencode session listreturned nothing because sessions were stored under the new VCS identity.The map now records the resolved Project id. Graphs whose identity could not be pinned yet (no VCS marker, or no commit or remote, where the id is
global) are re-resolved before the next borrow, and invalidated only when it changes. Sessions are listed again and snapshot capture resumes.The check never disturbs a live Location: graphs without a VCS compare
Project.rootwith async probes (so a repo above it is found too), borrowed or retained graphs are skipped, and the detach is a deferred invalidation instead oflifecycle.shutdown, so running steps hand off at a boundary.How did you verify your code works?
Tests in
packages/core/test/location-layer.test.tscover thegit init-> borrow -> first commit sequence (aglobalidentity mid-way, then the commit id), a parent folder becoming a repo, a borrowed location keeping its identity until release, and snapshot capture recovering afterwards. Without the fix the first test fails withReceived: undefined.bun test test/location-layer.test.ts: 28 pass, 0 fail.bun run typecheck:tscpasses.Checklist
Added by the opencode bot (AI-generated).
Related: #57985, #55133