Repository navigation
fix: show accurate page reference errors and allow retry - #666
Merged
Merged
Conversation
Reviewer's GuidePage-reference metadata failures now retain their original identity and render accurate missing, trash, deletion, permission, authentication, or temporary-failure states. A versioned metadata hook subscribes before lookup, ignores stale requests, supports sync recovery and authoritative Retry, and cleans up safely; tests and Storybook examples cover the complete state and race-condition matrix. Sequence diagram for page-reference metadata loading and retrysequenceDiagram
participant MentionPage
participant MetadataHook as usePageReferenceMetadata
participant Events as EventEmitter
participant AppLayer as AppBusinessLayer
participant ViewService
MentionPage->>MetadataHook: loadViewMeta(pageId)
MetadataHook->>Events: subscribe before lookup
MetadataHook->>AppLayer: loadViewMeta(pageId)
AppLayer->>ViewService: get or refresh metadata
alt metadata available
ViewService-->>AppLayer: View
AppLayer-->>MetadataHook: View
MetadataHook-->>MentionPage: render page reference
else metadata error
ViewService-->>AppLayer: original lookup error
AppLayer-->>MetadataHook: error
MetadataHook-->>MentionPage: render mapped error state
end
opt temporary failure
MentionPage->>MetadataHook: retry()
MetadataHook->>AppLayer: loadViewMeta(pageId, authoritative)
AppLayer->>ViewService: refresh metadata
ViewService-->>AppLayer: View
AppLayer-->>MetadataHook: View
MetadataHook-->>MentionPage: replace error with page reference
end
State diagram for page-reference metadata statusstateDiagram-v2
[*] --> Loading
Loading --> Available: View returned
Loading --> Missing: ViewMetadataError(not_found)
Loading --> InTrash: ViewMetadataError(in_trash)
Loading --> Deleted: PageNotFound/Gone error
Loading --> Denied: permission error
Loading --> Unauthorized: unauthorized error
Loading --> TemporaryFailure: other lookup error
TemporaryFailure --> Loading: retry()
TemporaryFailure --> Available: authoritative refresh succeeds
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/components/editor/components/leaf/mention/usePageReferenceMetadata.ts" line_range="53" />
<code_context>
+ ? await loadViewMeta(pageId, undefined, { authoritative: true })
+ : await loadViewMeta(pageId);
+
+ if (requestVersion.current === version) acceptView(view);
+ } catch (error) {
+ if (requestVersion.current === version) {
</code_context>
<issue_to_address>
**Retargeted lookup overwrites cached title**
When the component retargets while a lookup is pending and that lookup resolves after the new target commits but before the old passive effect cleanup runs, the old request passes the version check and calls `acceptView`, which invokes `onPageMentionNameResolved`; in rich-text contexts that writes to the shared page-name cache, so a stale title for the previous target can overwrite a newer cached title.
Fence responses against the current target synchronously before calling `acceptView` or its external callback.
</issue_to_address>| ? await loadViewMeta(pageId, undefined, { authoritative: true }) | ||
| : await loadViewMeta(pageId); | ||
|
|
||
| if (requestVersion.current === version) acceptView(view); |
There was a problem hiding this comment.
🟡 Medium · Retargeted lookup overwrites cached title
When the component retargets while a lookup is pending and that lookup resolves after the new target commits but before the old passive effect cleanup runs, the old request passes the version check and calls acceptView, which invokes onPageMentionNameResolved; in rich-text contexts that writes to the shared page-name cache, so a stale title for the previous target can overwrite a newer cached title.
Fence responses against the current target synchronously before calling acceptView or its external callback.
Prompt for AI agents
In `src/components/editor/components/leaf/mention/usePageReferenceMetadata.ts` at line 53:
**Retargeted lookup overwrites cached title**
When the component retargets while a lookup is pending and that lookup resolves after the new target commits but before the old passive effect cleanup runs, the old request passes the version check and calls `acceptView`, which invokes `onPageMentionNameResolved`; in rich-text contexts that writes to the shared page-name cache, so a stale title for the previous target can overwrite a newer cached title.
Fence responses against the current target synchronously before calling `acceptView` or its external callback.
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.
Description
A failed page-reference metadata lookup currently renders as "No Access" even when the server reports a missing record or a temporary service failure. Preserve the original lookup error and distinguish missing, trashed, deleted, permission-denied, sign-in-required, and temporary-failure states.
Temporary failures now expose Retry using an authoritative metadata refresh. Subscribe before the initial lookup and fence pending requests so folder updates can recover a reference and stale responses cannot overwrite a newer title or a different target. Add Storybook examples for every state. Preserve readable same-page block content when an optional metadata lookup fails, including equation references in detached editor surfaces.
This fixes status display and recovery behavior; it does not recreate a missing server record or change permissions.
Validation
latest-amd64independently for Cloud, Worker, Search and AI. Report image IDs/version/revision labels instead of failing when independently published services have different source commits. YAML parsing, Bash syntax and a dry-run report with four different revisions passed. All 16 backend shards passed the new reporting step and started Docker services; form-timeline-bdd passed.Existing local fixture BDD
The
@local-ref-pagesscenario is opt-in withRUN_REF_PAGES_TEST=1, requires a loopbackBASE_URLandREF_PAGES_TEST_PASSWORD, and reuses the existing accounts, document, references, and trash state. It does not seed, modify, repair, or clean up page data or permissions.Coverage limit: this BDD checks existing permissions and recoverable trash, not permanent deletion or live permission changes. Manual testing found that revoking access after a title has resolved still requires refresh; granting access recovers the title live.
Checklist
Summary by Sourcery
Show accurate page-reference failure states and provide safe retry and recovery behavior without obscuring readable content.
New Features:
Bug Fixes:
Enhancements:
CI:
Tests: