feat(webapp): old URLs follow a moved file, and share links follow the file (BEA-81) - #130
Open
ssowonny wants to merge 1 commit into
Open
feat(webapp): old URLs follow a moved file, and share links follow the file (BEA-81)#130ssowonny wants to merge 1 commit into
ssowonny wants to merge 1 commit into
Conversation
…e file (BEA-81) There is no rename in beardrive: the scanner emits a put at the new path and a delete at the old, same device, same blob, one cycle. Everything keyed on a path therefore broke the moment a file moved — the viewer 404'd, history lost the file's own past versions, restore refused them, and a share link either 404'd or silently served whatever unrelated file later took its address. internal/webapp/moves.go derives the pairing from the ops the replay already walks, cached with the snapshot. Deliberately not a rename op: journal.Less and Replay are what every device converges to, and every already-shipped journal would still need the heuristic to read its own history. The two rules point in opposite directions on purpose. A viewer URL is an address, so a LIVE path always wins and only an empty one redirects. A share token is a promise about one file, so it follows the file even when a new one takes the old address — and 404s forever once the file is deleted. Nothing here writes an op or touches sync. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
TL;DR
The two rules point in opposite directions, on purpose
a.mdmoves todocs/a.md, and then a newa.mdappears at the old address:flowchart TB E["a.md -> docs/a.md,<br/>then a NEW a.md appears"] V["<div style='text-align:left'><b>/proj/a.md</b> (viewer)<br/>the NEW a.md wins<br/>no redirect: a live path<br/>always wins</div>"] S["<div style='text-align:left'><b>/s/<token></b> (share)<br/>docs/a.md wins<br/>the link follows the file<br/>it was minted for</div>"] E -- "an address" --> V E -- "a promise about one file" --> S classDef box fill:#88888822,stroke:#888888 class E,V,S boxA viewer URL is an address — whatever is at that address today is the right answer. A share token is a promise about one file, and must never resolve to a file it wasn't minted for.
There is no rename to read
internal/syncer's scan emits aputfor the path it has never seen and adeletefor the cache key it no longer sees — same cycle, same device, same blob.journal.Ophasputanddeleteand nothing else. So a move is only ever inferred, and everything keyed on a path broke the moment a file moved.New
internal/webapp/moves.goderives the pairing inside the replayRemoteSource.Filesalready runs, and caches it with the snapshot. No new op kind, no new storage, no extra journal read.A
deleteof A pairs with the first-everputof B when all hold:Op.DeviceAmbiguous → no pairing, no redirect. Silence beats a wrong destination.
This is deliberately not a
renameop. A rename op changesjournal.Less/Replay, which is what every device converges to — and every already-shipped journal would still need the heuristic to read its own history. Derivation is reversible; a new op kind isn't.Four call sites, one index
/file/render/download404 no such file: a.mdX-Bdrive-Canonical-Path: docs/a.md; the read is recorded against the canonical pathGET .../resolve?path=PermRead:{"to":…,"kind":"file"|"folder"}or 404/s/<token>history?path=restoreFolder redirects are derived from the file mappings (there are no folder ops) and are all-or-nothing: every file that was under
notes/must land onwiki/<same suffix>, nothing undernotes/still live, no non-move delete in the way. A partial match gets no folder redirect — the individual files still redirect on their own.What can break
web/docs/.../guides/agent-artifacts.md.Op.Timeis unvalidated peer JSON, and both the 30s window and thesh.Createdcomparison read it. A member who can write a journal can shape a pairing — but they can already put arbitrary bytes at the shared path, so this is not an escalation. The requirement is only that a hostileTimecannot loop or panic: every walk carries a visited set and a hop cap, andTestResolveForwardCyclepins it.ponytail:comment names the one O(n²) bucket (identical copies of a single file) and its upgrade path.journal.Less,Replay,scan,materializeand the CLI are untouched;TestRenameConvergesAsPutPlusDeleteininternal/synceris the test that keeps it that way.Deviations from the reviewed plan
Named rather than smuggled:
restore.gonow sorts before building the index.loadOpsreturns unsorted andbuildMoveIndexneedsjournal.Lessorder.old-guide.md→archive/moved-guide.md), because the redirect has no browser-level proof without one.history?path=filter this PR rewrites. It is the older issue; expect a textual conflict there, not a semantic one.What was run
go test ./...— passes, including 22 new cases ininternal/webapp/moves_test.go,shares_test.go,restore_test.goand the multi-device convergence guard ininternal/syncer/syncer_test.go.npm run e2e— 155 passed, 1 skipped, including two new specs: the moved file's old URL redirects and banners, and a genuinely unknown path still gets the not-found card.npm run build+frontend/check-dist.sh—internal/webapp/staticis committed and fresh.Screenshots
origin/main)A path that never existed is untouched — still the not-found card, no banner, no redirect:
Architecture changes
architecture/webapp-server.mdMoveSourcejoinsUploaderas an optional capability onSource—*RemoteSourceimplements it,DirSourcedoesn't, so the "no journals, no moves" exclusion falls out of the type assertion instead of needing a rule.volume's cachedsnapshotnow carries the derivedmoveIndexalongside its file listing, and three new value types hang off it:pathEvent(one moment a path stopped being its file), andsegment(the time window during which a path was the file being asked about).flowchart TB Source["<div style='text-align:left'><b>Source</b> «interface»<br/>+Files(ctx) map path→FileInfo<br/>+Open(ctx, path, fi) io.ReadCloser</div>"] DirSource["<div style='text-align:left'><b>DirSource</b><br/>+Dir string</div>"] RemoteSource["<div style='text-align:left'><b>RemoteSource</b><br/>+Backend remote.Backend<br/>+Device Identity<br/>-loadSourcedOps(ctx) []sourcedOp<br/>-appendOp(ctx, op)</div>"] volume["<div style='text-align:left'><b>volume</b><br/>-source Source<br/>-refresh time.Duration<br/><span style='background:#ef444455;padding:0 4px;border-radius:3px'>❌ <s>-snap *snapshot</s></span><br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ -snap *snapshot (files + moves)</span><br/>+snapshot(ctx)<br/>+invalidate()</div>"] MoveSource["<div style='text-align:left'><b>MoveSource</b> «interface»<br/>+FilesWithMoves(ctx) files, moveIndex</div>"] moveIndex["<div style='text-align:left'><b>moveIndex</b> «map path→[]pathEvent»<br/>+buildMoveIndex(sorted ops)<br/>+resolveForward(idx, files, p) viewer<br/>+resolveShare(idx, files, p, since) /s/<br/>+chainSegments(idx, p) []segment<br/>+resolveFolder(idx, files, dir)</div>"] pathEvent["<div style='text-align:left'><b>pathEvent</b><br/>+At the delete that ended it<br/>+To "" = deleted, not moved<br/>+ToAt destination's create</div>"] segment["<div style='text-align:left'><b>segment</b><br/>+Path<br/>+From, To window it WAS the file</div>"] Note["Derived in the replay Files already runs,<br/>cached with the snapshot.<br/>No new op kind, no extra journal read.<br/>journal.Less and Replay are untouched."] volume -- "source" --> Source DirSource -. implements .-> Source RemoteSource -. implements .-> Source RemoteSource -. "✅ implements" .-> MoveSource MoveSource -- "✅ returns" --> moveIndex volume -- "✅ cached with the snapshot" --> moveIndex moveIndex -- "✅ per path, in time order" --> pathEvent moveIndex -- "✅ chainSegments" --> segment moveIndex -.- Note classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class MoveSource,moveIndex,pathEvent,segment added class Note noteBox linkStyle 3 stroke:#22c55e,stroke-width:2px linkStyle 4 stroke:#22c55e,stroke-width:2px linkStyle 5 stroke:#22c55e,stroke-width:2px linkStyle 6 stroke:#22c55e,stroke-width:2px linkStyle 7 stroke:#22c55e,stroke-width:2pxarchitecture/webapp-frontend.mdBrowsergained one member and no new relationships. It decides a path is missing from/treealone and never fetches the file, so theX-Bdrive-Canonical-Pathheader would never have reached the browser — and a moved folder has no content fetch to hang a header on. The not-found branch asks/resolve?path=instead, on the miss path only.flowchart TB HubApp["HubApp"] VolumeApp["VolumeApp"] Browser["<div style='text-align:left'><b>Browser</b><br/>folder listing, file view<br/>per-view routes<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +moved: /resolve?path= on a tree miss only</span></div>"] nav["<div style='text-align:left'><b>nav</b><br/>+navigate(url, {replace})</div>"] Note["The happy path is unchanged:<br/>no /resolve request fires<br/>while /tree has the path."] HubApp --> Browser VolumeApp --> Browser Browser -- "replaceState to the destination" --> nav Browser -.- Note classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class Note noteBoxThe one number worth revisiting
The 30s / same-device / same-blob window is a heuristic chosen to be wider than one daemon cycle and narrower than a person's editing session. It has never met a real journal. If moves start declining to pair on a busy team's volume, that constant —
moveWindowinmoves.go— is where to look first.Build session
(only works on the machine this ran on)