feat: playwright sugar poc with resilient navigation - #39
Merged
Conversation
📝 WalkthroughWalkthroughThis PR updates CI/docs workflow, replaces the LabEmbed docs iframe with a new PlayExplorer demo (component, tokenizer, helpers, CSS), integrates lab builds into docs, adds deep-link/demo URL options, introduces clickToURL helper, refactors Play/Director/Outcome APIs (async locators, recheck config, skip predicates, RunOptions), removes SyncStrategy, updates exports, and adjusts tests/docs. ChangesAll Changes (single coherent cohort)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
rickcedwhat-ai
force-pushed
the
feat/poc-resilience
branch
3 times, most recently
from
May 24, 2026 21:39
8ef596d to
157e408
Compare
rickcedwhat
approved these changes
May 24, 2026
rickcedwhat-ai
force-pushed
the
feat/poc-resilience
branch
from
May 24, 2026 21:45
157e408 to
54fc91e
Compare
Owner
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/play.ts (1)
237-246:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve async locators before building attempt outcomes.
On Line 244, function locators are invoked but not awaited. With async locator factories, this can pass unresolved promises into outcome matching.
Suggested fix
- const resolvedOutcomes: Outcome[] = act.outcomes.map(o => ({ + const resolvedOutcomes: Outcome[] = await Promise.all(act.outcomes.map(async o => ({ name: o.name, isSuccess: o.isSuccess, ...(o.isTimeoutOutcome && { isTimeoutOutcome: true }), ...(o.isActionErrorOutcome && { isActionErrorOutcome: true }), ...(o.onOutcome && { onOutcome: o.onOutcome }), ...(o.locator !== undefined && { - locator: typeof o.locator === 'function' ? (o.locator as Function)(page, ctx) : o.locator, + locator: + typeof o.locator === 'function' + ? await Promise.resolve((o.locator as AsyncLocatorFn)(page, ctx)) + : o.locator, }), - })); + })));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/play.ts` around lines 237 - 246, The resolvedOutcomes mapping calls locator factories synchronously (in act.outcomes => ...) which can return promises; change the logic that builds resolvedOutcomes so locator factories are awaited (e.g., make the surrounding function async and use await for typeof o.locator === 'function' ? await (o.locator as Function)(page, ctx) : o.locator, or collect with Promise.all when mapping) so no unresolved Promise gets stored in the Outcome.locator field; update any caller of resolvedOutcomes to handle the async/await change accordingly.
🧹 Nitpick comments (2)
docs/guide/getting-started.md (1)
102-102: ⚡ Quick winConsider improving readability by splitting this dense paragraph.
The single-line paragraph packs three distinct concepts (fixture location, Play explorer features, local setup docs) with heavy bold formatting. Breaking it into multiple sentences or a short list would improve scannability.
♻️ Proposed refactor for clarity
-The repo ships a **Sugar Lab** fixture (`lab/`) exercised by **`tests/director.spec.ts`**. [Sugar Lab](/guide/sugar-lab) includes an interactive **Play explorer** (code + illustrative UI + branching modals). **`lab/README.md`** explains running the real SPA locally. +The repo ships a **Sugar Lab** fixture (`lab/`) used by integration tests in **`tests/director.spec.ts`**. The [Sugar Lab](/guide/sugar-lab) page features an interactive **Play explorer** that visualizes code execution, UI state, and branching outcomes. See **`lab/README.md`** for instructions on running the SPA locally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/guide/getting-started.md` at line 102, Split the dense single-line paragraph into multiple shorter sentences or a 2–3 item list: (1) state the fixture location and the test that exercises it (reference "lab/" and "tests/director.spec.ts"), (2) describe the Play explorer features (reference "Play explorer": code, UI, branching modals) as a separate sentence, and (3) mention the local SPA setup docs in "lab/README.md" as its own sentence; also reduce heavy bolding by only emphasizing the most important terms rather than every phrase.docs/.vitepress/theme/components/PlayExplorer.vue (1)
323-327: ⚡ Quick winAdd keyboard-triggered popover behavior for flow snippets.
Snippet previews are hover-only right now. Adding focus/blur handlers (and linking trigger/tooltip with ARIA) would make this explainer usable without a mouse.
Also applies to: 369-372, 343-345, 353-355, 394-395, 410-417
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/.vitepress/theme/components/PlayExplorer.vue` around lines 323 - 327, The hover-only popover triggers (e.g., the element using methods flowPopOpen('nav', $event) and flowPopScheduleHide in PlayExplorer.vue) need keyboard accessibility: add focus and blur handlers alongside `@mouseenter/`@mouseleave (e.g., `@focus`="flowPopOpen('nav', $event)" and `@blur`="flowPopScheduleHide") on each snippet trigger, ensure the trigger has tabindex="0" if not natively focusable, and link the trigger and popover with ARIA attributes (aria-haspopup="dialog"/"tooltip", aria-expanded bound to the popover state, and aria-controls pointing to the popover id). Repeat the same updates for the other instances noted (lines around 343–345, 353–355, 369–372, 394–395, 410–417) so keyboard users can open/close the flow popovers and screen readers can associate triggers with their tooltips.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-checks.yml:
- Around line 60-70: Update the docs-site job to harden security: replace
unpinned action refs (actions/checkout@v4, actions/setup-node@v4,
pnpm/action-setup@v4) with their corresponding commit SHAs for the intended v4
releases, add a minimal permissions block scoped to only the needed permissions
for this job, and set persist-credentials: false on the actions/checkout step to
avoid credential leakage; locate these changes within the docs-site job
definition (job name "docs-site" and the steps referencing actions/checkout,
actions/setup-node, and pnpm/action-setup) and apply the SHA pins and permission
block accordingly.
In `@docs/.vitepress/theme/components/play-explorer/createTour.ts`:
- Line 60: Change the incorrect call Outcomes.timeout(3_000) to use the proper
API Outcomes.timeout() (or Outcomes.timeout('name') if a name is desired) and
update the accompanying documentation text that mentions a “3s timeout
wait/race” so it refers to Play's attempt timeout option { timeout: 3000 }
instead of implying a numeric argument to Outcomes.timeout; search for other
occurrences of Outcomes.timeout(3_000) and the descriptive phrases (the two
related text blocks in the same file) and update both the code call and the
explanatory sentences to reflect the correct API shape and the { timeout: ... }
usage.
In `@docs/.vitepress/theme/components/PlayExplorer.vue`:
- Around line 148-152: The resetTour function currently forces outcome.value =
'success', which makes the viewer role land in an invalid/unreachable state;
change resetTour so outcome.value is set to a neutral/default (e.g., 'empty' or
null) instead of 'success' and ensure any role-switch handler that re-applies
this (the code that sets outcome on role changes) respects the role — for
example, when role.value === 'viewer' leave outcome as 'empty' (or clear it) and
only set 'success' for roles that can reach that outcome; update both resetTour
and the role-switching block that currently re-applies outcome to use this
conditional behavior.
- Line 601: The displayed step uses a zero-based phaseIndex (rendering "Step
0"); update the user-facing label in PlayExplorer.vue to use a one-based value
by showing phaseIndex + 1 (or compute a displayIndex = phaseIndex + 1) in the
template where the eyebrow <p class="eyebrow">{{ `Step ${phaseIndex} · ${phase}`
}}</p> is rendered so the first step reads "Step 1" while leaving the underlying
phaseIndex logic unchanged.
In `@docs/api/director.md`:
- Line 63: The docs reference a non-existent API SyncStrategies.withReload() —
update the example to use the actual exported sync strategy API from the
codebase (or remove the call) by locating the real export in source (search for
any exported sync-related symbol) and replacing SyncStrategies.withReload() with
the correct exported identifier and invocation so the documentation matches the
current public API and the example compiles against the code.
In `@src/clickToURL.ts`:
- Around line 14-33: The clickToURL function currently lets trigger.click() run
uncontrolled and accepts negative maxRetries; fix by validating opts (set
maxRetries = Math.max(0, opts.maxRetries || 5) and ensure timeout > 0), then
enforce the global timeout budget by computing remaining = timeout - (Date.now()
- startTime) before each attempt and using that remaining ms as a hard cap for
both the click and wait phases (i.e., race trigger.click() and
page.waitForURL(expectedUrl, { timeout: Math.min(subTimeout, remaining), ... })
against a timeout promise that rejects after remaining ms); if remaining <= 0,
throw the overall timeout error immediately. Reference symbols: clickToURL,
opts, maxRetries, timeout, startTime, trigger.click(), page.waitForURL().
In `@src/play.ts`:
- Around line 195-201: The current logic evaluates act.skip predicates even when
a prior runError has occurred, which can produce unwanted side effects; modify
the skip-determination in the play loop so that you short-circuit on runError
before calling any act.skip function: where shouldSkip is computed for an act
(variables act.skip, shouldSkip, lastOutcome), first check if runError is truthy
and set shouldSkip = true without invoking act.skip, otherwise evaluate the
existing predicate logic (i.e., only call typeof act.skip === 'function' ?
act.skip(ctx, lastOutcome) : act.skip when runError is falsy).
- Around line 280-282: The cleanup skip check treats function predicates as
always-true because it only checks truthiness of act.skip; update the
conditional in the cleanup loop to evaluate the predicate when it's a function
(and await it if async) and otherwise use the boolean value. In other words,
replace the plain if (act.skip) check with a check that calls act.skip() when
typeof act.skip === 'function' (awaiting the result if necessary) and skips only
when that evaluated result is true; reference the act.skip predicate used in the
cleanup handling to locate where to change this logic.
---
Outside diff comments:
In `@src/play.ts`:
- Around line 237-246: The resolvedOutcomes mapping calls locator factories
synchronously (in act.outcomes => ...) which can return promises; change the
logic that builds resolvedOutcomes so locator factories are awaited (e.g., make
the surrounding function async and use await for typeof o.locator === 'function'
? await (o.locator as Function)(page, ctx) : o.locator, or collect with
Promise.all when mapping) so no unresolved Promise gets stored in the
Outcome.locator field; update any caller of resolvedOutcomes to handle the
async/await change accordingly.
---
Nitpick comments:
In `@docs/.vitepress/theme/components/PlayExplorer.vue`:
- Around line 323-327: The hover-only popover triggers (e.g., the element using
methods flowPopOpen('nav', $event) and flowPopScheduleHide in PlayExplorer.vue)
need keyboard accessibility: add focus and blur handlers alongside
`@mouseenter/`@mouseleave (e.g., `@focus`="flowPopOpen('nav', $event)" and
`@blur`="flowPopScheduleHide") on each snippet trigger, ensure the trigger has
tabindex="0" if not natively focusable, and link the trigger and popover with
ARIA attributes (aria-haspopup="dialog"/"tooltip", aria-expanded bound to the
popover state, and aria-controls pointing to the popover id). Repeat the same
updates for the other instances noted (lines around 343–345, 353–355, 369–372,
394–395, 410–417) so keyboard users can open/close the flow popovers and screen
readers can associate triggers with their tooltips.
In `@docs/guide/getting-started.md`:
- Line 102: Split the dense single-line paragraph into multiple shorter
sentences or a 2–3 item list: (1) state the fixture location and the test that
exercises it (reference "lab/" and "tests/director.spec.ts"), (2) describe the
Play explorer features (reference "Play explorer": code, UI, branching modals)
as a separate sentence, and (3) mention the local SPA setup docs in
"lab/README.md" as its own sentence; also reduce heavy bolding by only
emphasizing the most important terms rather than every phrase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a56143d5-3a6c-4338-8dab-e42f3ea5100d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.claude/launch.json.github/workflows/pr-checks.yml.gitignoreROADMAP.mddocs/.vitepress/config.tsdocs/.vitepress/theme/components/LabEmbed.vuedocs/.vitepress/theme/components/PlayExplorer.vuedocs/.vitepress/theme/components/play-explorer/createTour.tsdocs/.vitepress/theme/components/play-explorer/tsHighlight.tsdocs/.vitepress/theme/index.tsdocs/.vitepress/theme/play-explorer-layout.cssdocs/api/director.mddocs/api/sync-strategy.mddocs/guide/getting-started.mddocs/index.mdlab/.npmrclab/README.mdlab/package.jsonlab/src/DatasetsPage.tsxlab/src/urlBootstrap.tspackage.jsonsrc/attemptAction.tssrc/clickToURL.tssrc/director.tssrc/index.tssrc/outcomes.test.tssrc/outcomes.tssrc/play.test.tssrc/play.tssrc/syncStrategy.tstests/director.spec.ts
💤 Files with no reviewable changes (4)
- docs/.vitepress/theme/components/LabEmbed.vue
- src/syncStrategy.ts
- .claude/launch.json
- src/outcomes.test.ts
rickcedwhat-ai
force-pushed
the
feat/poc-resilience
branch
from
May 24, 2026 22:56
54fc91e to
63f86f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/outcomes.ts (1)
5-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale JSDoc for timeout behavior.
The comment still describes
after-based timeout derivation, but this API no longer supportsafter.💡 Proposed fix
/** * An outcome as configured by the user — `locator` may be a page-bound function - * resolved by Play before calling attemptAction. `after` is consumed by Play - * to derive the `timeout` parameter. + * resolved by Play before calling attemptAction. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/outcomes.ts` around lines 5 - 8, The JSDoc for the Outcome type is stale: it mentions deriving the `timeout` from `after` and that `after` is consumed by Play; update the comment for the Outcome (and any references to `locator`, `attemptAction`, and `Play`) to remove mention of `after` and clearly state that `timeout` is provided directly by the API (or how timeouts are now specified), e.g., describe that `locator` may be a page-bound function resolved before `attemptAction` and that `timeout` is used directly to control waits.
🧹 Nitpick comments (1)
src/play.ts (1)
16-18: ⚡ Quick winUpdate the public docs for predicate-based
skip.
ActOptions.skipnow accepts a callback, but the supplieddocs/api/play.mdsnippet still documents this as{ skip?: boolean }. Shipping the type change without the doc update leaves consumers on the old contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/play.ts` around lines 16 - 18, The docs still describe ActOptions.skip as a boolean but the type ActOptions (in src/play.ts) now allows skip to be a predicate function (skip?: boolean | ((ctx: PlayCtx, lastOutcome?: PlayOutcome) => boolean)); update docs/api/play.md to reflect this new contract: change the type signature and example snippets to show both boolean and predicate usage, document the predicate parameters (PlayCtx, optional lastOutcome: PlayOutcome) and the expected boolean return, and include a brief example of returning true/false based on ctx or lastOutcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/.vitepress/theme/components/PlayExplorer.vue`:
- Around line 261-266: The picksSummary computed builds bits typed as { k:
string; v: string }[] but outcome is a ref of type OutcomeChoice | null (set to
null in resetTour), causing a nullable type mismatch; update the bits typing to
allow null (e.g., { k: string; v: string | null }[]) or normalize outcome.value
when constructing bits (e.g., use outcome.value ?? ''), so the entry for Outcome
rehearsal in the picksSummary computed (and any other places using
outcome.value) matches the declared types.
In `@src/clickToURL.ts`:
- Around line 28-34: The Promise.race branch can leave trigger.click and
page.waitForURL running after the global timeout; change the inner async branch
in clickToURL to be cancellable: create an AbortController per attempt, pass its
signal to trigger.click and page.waitForURL (or wrap those calls to reject on
signal), and when the global-timeout promise fires call controller.abort() so
the losing branch is interrupted before the next retry; ensure the abort
rejection is handled separately from other errors so retries proceed normally.
In `@src/director.ts`:
- Around line 122-124: The ensureExists flow currently calls await
this._runPlay(playbook, 'create', params, { indent: 1 }) and ignores its result;
change it to capture the result (e.g., createResult) and fail fast if
createResult.isSuccess is false by returning the failure or throwing an error
consistent with ensureExists's contract. Specifically, in ensureExists after
calling this._runPlay(..., 'create', ...) store the returned value, check
createResult.isSuccess, and if false propagate the failure (or throw a
descriptive error) instead of continuing, referencing existsResult,
createResult, this._runPlay, playbook, and params to locate and fix the logic.
In `@src/play.ts`:
- Around line 195-203: The skip predicate invocation (act.skip(ctx,
lastOutcome)) is currently executed outside the per-act try/catch, so if it
throws it escapes the act wrapper; wrap the call to act.skip in a try/catch
inside the per-act handling (the block that computes shouldSkip using runError,
act.kind, act.skip) and on catch set shouldSkip = true (or set runError/record
the error) and rethrow or log using the same "[label > actName]" act-failure
wrapper so the thrown predicate is handled exactly like an act failure; apply
the same change to the second occurrence around lines where act.skip is checked
later (the 282–285 area) to ensure both predicate evaluations are protected by
the per-act try/catch and error-wrapped consistently.
In `@tests/director.spec.ts`:
- Around line 65-72: The cleanup's wait currently uses page.getByText('Updated
dataset').first().waitFor() which may match the earlier rename toast; update the
cleanup to wait for the specific toast from the revert operation (for example by
targeting the row's actions/rename flow or using the toast locator for the most
recent occurrence) — locate the rename flow in the .cleanup callback (the
page.locator(...).getByRole('button', { name: 'Row actions' }) click,
getByRole('menuitem', { name: 'Rename' }), getByRole('textbox'), and 'Save'
button sequence) and replace the brittle first().waitFor() with a selector that
uniquely identifies the cleanup revert toast (e.g., using last() or a scoped
locator tied to the row/newName or an explicit wait for a toast that contains
both 'Updated dataset' and the dataset name) so the wait only resolves after the
revert finishes.
---
Outside diff comments:
In `@src/outcomes.ts`:
- Around line 5-8: The JSDoc for the Outcome type is stale: it mentions deriving
the `timeout` from `after` and that `after` is consumed by Play; update the
comment for the Outcome (and any references to `locator`, `attemptAction`, and
`Play`) to remove mention of `after` and clearly state that `timeout` is
provided directly by the API (or how timeouts are now specified), e.g., describe
that `locator` may be a page-bound function resolved before `attemptAction` and
that `timeout` is used directly to control waits.
---
Nitpick comments:
In `@src/play.ts`:
- Around line 16-18: The docs still describe ActOptions.skip as a boolean but
the type ActOptions (in src/play.ts) now allows skip to be a predicate function
(skip?: boolean | ((ctx: PlayCtx, lastOutcome?: PlayOutcome) => boolean));
update docs/api/play.md to reflect this new contract: change the type signature
and example snippets to show both boolean and predicate usage, document the
predicate parameters (PlayCtx, optional lastOutcome: PlayOutcome) and the
expected boolean return, and include a brief example of returning true/false
based on ctx or lastOutcome.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 81333ad8-7228-4472-8388-ff0f21c57169
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.claude/launch.json.github/workflows/pr-checks.yml.gitignoreROADMAP.mddocs/.vitepress/config.tsdocs/.vitepress/theme/components/LabEmbed.vuedocs/.vitepress/theme/components/PlayExplorer.vuedocs/.vitepress/theme/components/play-explorer/createTour.tsdocs/.vitepress/theme/components/play-explorer/tsHighlight.tsdocs/.vitepress/theme/index.tsdocs/.vitepress/theme/play-explorer-layout.cssdocs/api/director.mddocs/api/sync-strategy.mddocs/guide/getting-started.mddocs/index.mdlab/.npmrclab/README.mdlab/package.jsonlab/src/DatasetsPage.tsxlab/src/urlBootstrap.tspackage.jsonsrc/attemptAction.tssrc/clickToURL.tssrc/director.tssrc/index.tssrc/outcomes.test.tssrc/outcomes.tssrc/play.test.tssrc/play.tssrc/syncStrategy.tstests/director.spec.ts
💤 Files with no reviewable changes (4)
- .claude/launch.json
- docs/.vitepress/theme/components/LabEmbed.vue
- src/syncStrategy.ts
- src/outcomes.test.ts
✅ Files skipped from review due to trivial changes (8)
- .gitignore
- lab/README.md
- docs/guide/getting-started.md
- ROADMAP.md
- docs/index.md
- docs/.vitepress/config.ts
- docs/api/sync-strategy.md
- docs/.vitepress/theme/play-explorer-layout.css
rickcedwhat
pushed a commit
that referenced
this pull request
May 27, 2026
* fix: address code review issues from PR #39 - outcomes.ts: remove stale after reference from JSDoc - director.ts: fail-fast when create play does not succeed in ensureExists - play.ts: guard skip predicate calls with try/catch in main and cleanup loops - clickToURL.ts: add AbortController per attempt to interrupt losing branch - tests/director.spec.ts: use scoped last() locator for cleanup toast wait - PlayExplorer.vue: widen picksSummary bits type to string | null - docs/api/play.md: document ActOptions.skip predicate form with example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): ensure play execution short-circuits on failure outcomes * feat: strict typing for director methods and race-with-buffer detect algorithm * feat: standardize wrapper names in logger and add optional name overloads to nav, prep, cleanup * chore: everything is working perfectly and playbooks are completely clean * feat(poc): add recheckSync sync options and poc run script * feat: implement collect mode on Director and update test suite * test: improve unit tests and refine sync skip predicate in cleanup --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 28, 2026
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
2 participants
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.
This PR introduces a resilient clickToURL implementation to handle dynamic SPA navigation safely, improves the director sync logic, and correctly matches dynamic URLs based on path segments to handle dynamic project IDs safely. It also includes the PoC tests and playbooks for org-level project updates.
Summary by CodeRabbit
New Features
Documentation
Refactor