Repository navigation
Keep the in-memory hosted engine off the disk for Go workspaces - #2031
Draft
Mikola Lysenko (mikolalysenko) wants to merge 12 commits into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 12 commits into
Mikola Lysenko (mikolalysenko) wants to merge 12 commits into
Conversation
Go discovery read only a go.work at the project root and only members inside the root. With GOWORK unset it ignored an ancestor go.work and read the root go.mod in module mode, so in a member of a parent workspace it could attest a go.mod replace that the workspace overrides, doesn't `use`, or makes inert by version selection. Hosted and vendored modes resolve the workspace go itself would use and refuse to write such a replace. On disk, discovery now loads the governing workspace with the same go_workspace::load_with loader hosted mode uses: GOWORK (environment, then the go env -w file), else the nearest go.work at or above the root, with `use` members anywhere. Out-of-root members' requires and replaces now count toward selection and conflicts. In-root members are still re-read through the view, so recognition and parse diagnostics stay the same. A wired Socket replace that this workspace keeps from the build, at exactly the points where hosted mode's host_conflict check refuses, stays a ref (list, rollback and remove still find it) but is marked unattested with the new UnattestedKind::GoWorkspaceOverride. The same applies to a replace in the root go.work while GOWORK makes go build another one. vex_sources reports these under the existing `go_workspace_override` reason, the one vex's own go.mod gate already uses. A replace that workspace selection makes inert is still diagnosed, as in module mode. A GOWORK that go rejects still withholds every ref as patched_ref_invalid. In memory there is no directory to search from and no environment the project builds under. The root go.work and its in-root members are the workspace there, as the in-memory hosted engine reads them, and the same conflict gate applies. GOWORK keeps its old effect in memory and GOFLAGS is not read. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Version-selection inertness in replaced_purl, and the hosted selection checks, ran before Workspace::bypass. A Socket replace that the governing go.work also refuses (not used, overridden by a go.work replace, member conflict) was dropped as inert with a diagnostic instead of being kept as a GoWorkspaceOverride ref, so list, rollback and remove could not find wiring that re-running scan or vendor would never clear. Compute the bypass first and skip the inertness checks for a bypassed replace; it stays a ref and is reported unattested. Found by Cursor Bugbot on #2011. Co-Authored-By: Claude <[email protected]>
This fixes the confirmed /code-review high findings on #2011. Each fix has a test that fails on the previous commit. - A Go workspace mark now applies to one file only. The new UnattestedKind::file_scoped is true only for GoWorkspaceOverride. vex_sources drops the bypassed ref from the candidate's basis and vetoes the patch only when no live ref of it remains. wiring_conflicts ignores bypassed refs. So a root go.work replace that go never reads no longer withholds the root go.mod's mirror of it. A leftover member pin that Socket's own go.work replace overrides no longer turns into a wiring conflict that also withholds the go.work patch, which is what vex.rs's WorkHosted exemption intends. - A replace of a module that is itself a `use` member is diagnosed as inert again, both on disk and in memory, hosted and vendored. This check now runs before the workspace bypass. - SumPins now also reads the sum files the loader already read: the governing go.work.sum and each out-of-root member's go.sum. They are used only when they lie inside the git repository. - In memory, workspace conflicts are evaluated in go_workspace::PathSpace::Memory. Paths are compared lexically against the view's files and never against the filesystem or the working directory. conflict/work_conflict delegate to the new *_in variants, so disk behavior is unchanged. - The snapshot probe list is now built by one shared helper (workspace_probes), spelled the way resolve_with spells it. - A root go.work that go doesn't read is no longer judged unparseable, and its wiring stays an unattested ref. - The go_workspace_override omission phrase now covers every case. - read_loaded_workspace uses members_within. The view re-reads only the in-root members' go.mod, which recognition and parse diagnostics need. The duplicate go.sum and go.work.sum reads are gone. - Tests that use a nested project now create a .git, so they keep working once reads are bounded to the repository root (#2025). Co-Authored-By: Claude Opus 5.5 <[email protected]>
The hosted engine hands the golang rewriter the Go workspace it read (HostedGoWorkspace). In memory its paths are root-relative, but the rewriter still judged them with the disk checks: replace_host, rooted_at, selection and host_conflict canonicalized against the process's cwd and probed `<cwd>/go.mod`. So an in-memory project whose root module go.work does not `use` got "nowhere to go" (no go.mod) when the server's cwd had none, and the member answer when it did. HostedGoWorkspace now carries `files`: in memory, the root-relative files that exist (go.work, a readable root go.mod, each in-root member's readable go.mod), filled by the engine as discovery lists them. The rewriter routes every workspace check through `PathSpace::Memory` when it is set, via new `_in` variants of `replace_host`, `selection`, `host_conflict` and `conflict` on GoWorkspace / Loaded. Disk behaviour is unchanged (the old names delegate with `PathSpace::Disk`). `GoWorkspace::selection` also displayed a main-module member through the disk display_path; discovery's build-list graph now carries the workspace's PathSpace so in-memory discovery uses `selection_in` too. Co-Authored-By: Claude Opus 5.5 <[email protected]>
2984629 dropped every workspace-bypassed ref from wiring_conflicts, so a leftover member pin that Socket's go.work replace overrides would not withhold the go.work patch. That also caught the root go.mod. A GOWORK=off build from the root reads the root go.mod, so a different patch there still disagrees with go.work about what the build links. The behavior is the same reasoning ReplaceHost::mirrors_root_go_mod uses. e2e_vex_lockfile golang_go_mod_and_go_work_disagreeing_attest_neither went red in coverage (2): it got go_workspace_override plus go_version_not_selected skips, not one wiring_conflict skip. Count bypassed refs in the root go.mod as wiring again. Member go.mod pins are still exempt, as a_bypassed_leftover_pin_is_no_wiring_conflict requires. Co-Authored-By: Claude <[email protected]>
… fix/hosted-memory-go-pathspace
- GoWorkspace carries its own `files` (in memory: the root-relative
files that exist; `None` on disk) and every method routes through
`GoWorkspace::paths()`. The `_in(PathSpace, ..)` variants and their
Disk-default wrappers are gone, so no caller can judge an in-memory
workspace against the disk or the cwd. HostedGoWorkspace::files and
its paths() go with them; the rewriter's code is back to the base's.
- One builder for the in-memory workspace, go_workspace::read_memory,
over a MemoryFiles reader: the hosted engine reads through
read_advisory, discovery through its guarded reads (diagnosing an
unparseable member go.mod). Both list a file in `files` when it
exists (a link, an entry without content, a go.mod go cannot parse
all count), checked without reading it, and both drop an unparseable
member go.mod's text.
- members_within no longer counts `../out` or an absolute member as
inside the empty in-memory root (strip_prefix("") matched them), so
the NoModule hint names in-root members only.
- PathSpace::from_files builds the space from an optional file list.
- The cwd test moved to hosted::engine's tests; read_go_workspace is
private again. The redirect fixtures list the root go.mod whenever
the project has one, as read_memory does.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Second review round on Go workspace discovery. - A Socket replace of a module that is itself a `use` member (every replace of it is inert, and the writers refuse it with `WorkspaceConflict::MainModule`, which a re-run never clears) was dropped with a diagnostic, so rollback, remove and list lost wiring still in the file. It stays a ref, marked `GoWorkspaceOverride` with the same "is inert" detail (owner decision 4). - go.sum / member sums / go.work.sum were read only when a hosted ref existed, so a sum left by a reverted patch was no longer recognized and a dead ledger claim could be re-derived from its raw text (rule 11). They are recognized either way. - The plan both attested a purl (the live go.work patch) and reported it skipped for a lockfile-only candidate whose only ref is a stale, bypassed leftover pin of another patch. That candidate is now dropped with a note when another candidate attests the purl. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… fix/hosted-memory-go-pathspace
Second review round on the in-memory Go workspace. The in-memory hosted engine dropped the text of a member go.mod go cannot parse, while the disk loader keeps it: a member declaring the patched module (every replace of it inert, `MainModule`) or replacing it was refused on disk but pinned over in memory. `MemoryFiles:: unparseable` now says whether the text is kept: the engine keeps it, discovery still drops it after its diagnostic. Co-Authored-By: Claude Opus 5.5 <[email protected]>
#2011 squash-merged as b69fc03. Its go_workspace.rs and discover/golang.rs on main equal the #2011 tip this branch already merged (ec87efa), so both conflicts resolve to this branch's side. Co-Authored-By: Claude <[email protected]>
Collaborator
Author
|
bugbot run Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
In memory, GoWorkspace::files counts a root go.mod that exists but has no text the rewriter holds (a link, an unreadable file), and conflict() reports such a root as "not a member" when go.work does not use it. The rewriter passes has_go_mod from its candidate texts, so replace_host saw no root module, chose go.work, and ran work_conflict: the replace went into go.work, where it patches the members but never the project. When the root is not a member, treat a root go.mod that the in-memory workspace lists as the root's module, as on disk, so go.mod hosts the replace and conflict() refuses it. Found by Bugbot on df219a6. Co-Authored-By: Claude <[email protected]>
Collaborator
Author
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 04a1a9d. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2011 (
fix/go-discovery-workspace), which has now landed on main as b69fc03; this PR targets main.Problem
The hosted engine hands the golang rewriter the Go workspace it read (
HostedGoWorkspace). In memory (ProjectView::Memory) its paths are root-relative with an empty root. The rewriter still judged them with the disk checks:replace_host,rooted_at,selectionandhost_conflictresolved paths against the process's working directory and probed<cwd>/go.mod. The in-memory engine's answer therefore depended on where the server process was started.Example: an in-memory project with a root
go.modand ago.workthat only hasuse ./b. If the cwd had nogo.mod, the rewriter refused with "nowhere to go: … holds neither go.work nor a go.mod". If the cwd had one, it gave the correct "does notusethis module" answer.Fix
GoWorkspacenow hasfiles: Option<BTreeSet<PathBuf>>. In memory it lists the root-relative files that exist; on disk it isNone. Every method goes throughGoWorkspace::paths()(PathSpace::from_files). The_in(PathSpace, ..)variants from Read the governing go.work in Go discovery #2011 and their Disk-default wrappers are removed, so no caller can judge an in-memory workspace against the disk. The rewriter's code is unchanged from the base.go_workspace::read_memorybuilds the in-memory workspace for both the hosted engine and discovery, through a smallMemoryFilesreader trait. The engine reads throughread_advisory; discovery uses its guarded reads, which diagnose an unparseable member go.mod. A file is infileswhen it exists, whether readable, a link, an entry without content, or a go.mod go cannot parse. Existence is checked without reading the file.GoWorkspace::selectionno longer shows a main-module member through the diskdisplay_path.I checked the rest of the in-memory engine's Go path: the memory branch of
read_go_workspacereads only through the view, andgo_build_list::select_inis pure. No other disk, cwd or env probes remain on that path.Review findings
filescounted readable, not existing, files; engine and discovery built it differently. Confirmed, fixed. The engine skipped a symlinked or content-less root go.mod. Discovery skipped an unparseable one. Both now go throughread_memory, which uses existence. The new testshosted::engine::tests::memory_go_workspace_never_probes_the_cwdandvex::discover::golang::tests::in_memory_workspace_files_are_the_files_that_existuse the same three projects: a valid, an unparseable and a symlinked root go.mod, withuse ./b. Both expect the samefilesand the "not a member" verdict. Under the old engine rule the symlinked case losesgo.modand fails. Note: in discovery the root go.mod's presence is only consulted for a parsed go.mod, so its old rule never changed a verdict; it is aligned anyway.members_withincounted../outas in-root in memory. Confirmed, fixed. Only all-Normalrelative paths count as within the root, so absolute members are excluded too.memory_members_within_excludes_members_outside_the_rootfails without the fix (["a", "../out", "/abs", ""]) and also checks the NoModule member hint.memory_go_filesignored a root go.mod outside the members. Fixed.go_workspace_keyandmemory_go_filesnow takeroot_go_mod.with_root_go_workpasses whether the fixture has one.PathSpaceon the workspace. Done. See Fix above.read_memoryreturns the workspace and its in-root members. Discovery derives its sum files from those members.PathSpace::from_files. Added, used byGoWorkspace::paths. Discovery'sWorkspace::pathsandHostedGoWorkspace::pathsare gone..is_some(). Fixed. It is now an existence check. The engine's check also countsunreadableentries.read_go_workspacepub(crate)only for a test. Fixed. The test moved intohosted::engine's tests and the function is private again.Tests
cargo test -p socket-patch-core --lib -- hosted:: vex::discover:: vendor::go golang: 912 passed.--test hosted_memory_engine: 49 passed.--test hosted_memory_parity: 42 passed.cargo clippy --locked --workspace --all-features -- -D warningsandcargo fmt --all -- --checkare clean.Brought current with #2011, plus round-2 review findings (cloud routine)
Merged the updated #2011 in. It merged cleanly. The core Go and hosted tests (657) and
hosted_memory_parity(42) pass.Review findings, round 2 (
/code-review highon2ff156a)MainModule), or replaces it, was refused on disk but pinned over in memory.MemoryFiles::unparseablenow says whether the text is kept: the engine keeps it, and discovery still drops it after its diagnostic. Test:memory_go_workspace_never_probes_the_cwd(extended). It fails with the engine dropping the text.Declined:
normalize_for_readof the root go.mod. It only matters for pathological double-BOM or quote-nesting cases. Follow-up.exists_no_followcounts an implied in-memorygo.mod/directory. It only changes which refusal is named.read_sumtrait method, the duplicated member reader and the private doc link. Cleanup only.Tests (cloud run):
cargo test -p socket-patch-core --lib -- vex::discover:: utils::go hosted:: vendor::gogives 837 passed, golden included. CLI--test hosted_memory_paritygives 42 passed. clippy and fmt are clean.Main merged after #2011 landed (burn-down agent)
df219a66merges main at b69fc03. main'svendor/go_workspace.rsandvex/discover/golang.rsare byte-identical to the #2011 tip (ec87efaf) this branch had already merged, so both conflicts resolve to this branch's side.cargo test -p socket-patch-core --lib -- vex::discover:: utils::go hosted:: vendor::go: 841 passed;cargo check -p socket-patch-core --all-targetsclean.Note
Medium Risk
Changes Go workspace conflict and replace-host decisions for in-memory projects; behavior is aligned with disk semantics but touches patch redirect and discovery paths.
Overview
In-memory golang patching previously used disk path checks (including probing the process cwd for
go.mod), so workspace conflict answers could differ depending on where the server started. This PR makes the in-memory workspace self-contained.GoWorkspacenow carries an optionalfilesset listing root-relative paths that exist in memory.PathSpace::from_filesandGoWorkspace::paths()route all conflict, replace-host, and member logic through lexical comparisons whenfilesis set, removing the*_in(PathSpace, …)APIs so callers cannot mix disk and memory semantics.A shared
go_workspace::read_memorybuilder (via theMemoryFilestrait) loads in-memory workspaces for both the hosted engine (AdvisoryGoFiles) and discovery (ViewGoFiles), counting existence (links, unreadable entries, unparseablego.mod) rather than only readable content. The engine still retains unparseable membergo.modtext for conflict weighing, matching disk behavior.members_withinin memory now excludes paths with..or absolute components.replace_hosttreats an existing but unreadable rootgo.modas a real root module. Tests assert engine and discovery agree and that memory never probes cwd.Reviewed by Cursor Bugbot for commit 04a1a9d. Configure here.
Generated by Claude Code