fix(mount): cache scaffold reads per generation - #433
Conversation
Session-Id: 01a02017-d3b0-7e20-87dd-bf1b9408ee54
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/mountfuse/static_scaffold_cache_test.go">
<violation number="1" location="internal/mountfuse/static_scaffold_cache_test.go:66">
P3: In TestStaticScaffoldConcurrentReadsCoalesce, readFileFunc calls t.Fatalf from a reader goroutine, not the test goroutine. FailNow is only reliable on the test goroutine; calling it from other goroutines can leave goroutines running and is discouraged. Since only one path is ever passed this branch never fires, so it's a cleanliness nit rather than a behavior bug. Hoist the path check to the test body instead.</violation>
</file>
<file name="internal/mountfuse/fs.go">
<violation number="1" location="internal/mountfuse/fs.go:339">
P1: When websocket invalidation arrives during a scaffold fetch, the next read joins the old load and its completion permanently recaches the pre-invalidation content. Retire in-flight loads at the generation boundary or tag results with a generation and discard responses started before invalidation.</violation>
<violation number="2" location="internal/mountfuse/fs.go:352">
P2: When the first FUSE reader is canceled while other readers still wait, the shared `ReadFile` uses the canceled leader context and returns `context.Canceled` to every waiter. Give each coalesced load an independent lifecycle context instead of binding the remote request to one reader.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| delete(s.fileCache, remotePath) | ||
| } | ||
| if existing, ok := s.fileLoads[remotePath]; ok { |
There was a problem hiding this comment.
P1: When websocket invalidation arrives during a scaffold fetch, the next read joins the old load and its completion permanently recaches the pre-invalidation content. Retire in-flight loads at the generation boundary or tag results with a generation and discard responses started before invalidation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/mountfuse/fs.go, line 339:
<comment>When websocket invalidation arrives during a scaffold fetch, the next read joins the old load and its completion permanently recaches the pre-invalidation content. Retire in-flight loads at the generation boundary or tag results with a generation and discard responses started before invalidation.</comment>
<file context>
@@ -303,12 +311,68 @@ func (s *fsState) getFile(remotePath string) (mountsync.RemoteFile, bool) {
+ }
+ delete(s.fileCache, remotePath)
+ }
+ if existing, ok := s.fileLoads[remotePath]; ok {
+ s.cacheMu.Unlock()
+ select {
</file context>
| s.fileLoads[remotePath] = load | ||
| s.cacheMu.Unlock() | ||
|
|
||
| file, err := s.client.ReadFile(ctx, s.workspaceID, remotePath) |
There was a problem hiding this comment.
P2: When the first FUSE reader is canceled while other readers still wait, the shared ReadFile uses the canceled leader context and returns context.Canceled to every waiter. Give each coalesced load an independent lifecycle context instead of binding the remote request to one reader.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/mountfuse/fs.go, line 352:
<comment>When the first FUSE reader is canceled while other readers still wait, the shared `ReadFile` uses the canceled leader context and returns `context.Canceled` to every waiter. Give each coalesced load an independent lifecycle context instead of binding the remote request to one reader.</comment>
<file context>
@@ -303,12 +311,68 @@ func (s *fsState) getFile(remotePath string) (mountsync.RemoteFile, bool) {
+ s.fileLoads[remotePath] = load
s.cacheMu.Unlock()
+
+ file, err := s.client.ReadFile(ctx, s.workspaceID, remotePath)
+ if err == nil {
+ file.Path = normalizeRemotePath(file.Path)
</file context>
| } | ||
| } | ||
|
|
||
| func TestStaticScaffoldConcurrentReadsCoalesce(t *testing.T) { |
There was a problem hiding this comment.
P3: In TestStaticScaffoldConcurrentReadsCoalesce, readFileFunc calls t.Fatalf from a reader goroutine, not the test goroutine. FailNow is only reliable on the test goroutine; calling it from other goroutines can leave goroutines running and is discouraged. Since only one path is ever passed this branch never fires, so it's a cleanliness nit rather than a behavior bug. Hoist the path check to the test body instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/mountfuse/static_scaffold_cache_test.go, line 66:
<comment>In TestStaticScaffoldConcurrentReadsCoalesce, readFileFunc calls t.Fatalf from a reader goroutine, not the test goroutine. FailNow is only reliable on the test goroutine; calling it from other goroutines can leave goroutines running and is discouraged. Since only one path is ever passed this branch never fires, so it's a cleanliness nit rather than a behavior bug. Hoist the path check to the test body instead.</comment>
<file context>
@@ -0,0 +1,152 @@
+ }
+}
+
+func TestStaticScaffoldConcurrentReadsCoalesce(t *testing.T) {
+ const scaffoldPath = "/discovery/google-mail/LAYOUT.md"
+ started := make(chan struct{})
</file context>
1 participant
Related: AgentWorkforce/relayfile-cloud#168
Summary
/discovery/**andLAYOUT.mdcontent for the lifetime of a mount generationRegression controls
Verification
go test -race ./internal/mountfuseNo merge or deployment performed.