fix(web): preserve mobile app page when entering edit#6048
Conversation
lefarcen
added
size/L
Static gate runs actionlint with ShellCheck enabled, which flags literal Markdown backticks in single-quoted strings and repeated GITHUB_OUTPUT redirects. Scope the literal-string warnings and group output writes without changing workflow behavior. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks @Siri-Ray — the initial URL-to-srcDoc transition is thoughtfully covered, and the focused multi-page browser scenario passes. I found one merge-safe lifecycle issue where the captured entry snapshot can outlive that transition and override newer source-backed state; I’ve left the concrete reproduction and fix direction inline.
I verified the changed ranges, web typecheck, 234 focused web tests, 46 daemon range tests, diff hygiene, and the focused Playwright scenario. The live Preflight failure is the same six untouched daemon test typing errors already disclosed in the PR body.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Assert the unknown fetch payloads through Vitest's runtime structure matching so the test keeps validating error codes and created run IDs without unsafe property access. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks @Siri-Ray — the latest head fixes the workspace-gate test typing, and the focused daemon/web validation is green. The initial URL-to-srcDoc page handoff still works, but the previously noted merge-safe lifecycle concern remains on this head: the entry snapshot can outlive that handoff and overwrite newer source-backed state. I’ve kept the concrete reproduction and fix direction inline.
I verified every changed range, workflow lint, diff hygiene, web and daemon typechecks, 234 focused web tests, and 52 focused daemon tests. Thanks for the thoughtful bug fix and the quick CI cleanup. 🙏
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Keep the URL-to-srcDoc transition snapshot one-shot so later Manual Edit content reloads preserve newer source attributes and navigation state. Extend component and Playwright coverage through the exit, navigate, re-enter, and outer-HTML save lifecycle. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
|
Looper fixer round complete — 7471210
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks @Siri-Ray — the one-shot consumption fixes the stale replay after content reloads, and the expanded regression scenario is valuable. One merge-safe activation gap remains when the srcDoc transport has already been materialized: the state is captured, but a visibility-only switch can skip the only restore dispatch. I left the concrete lifecycle and fix direction inline.
I reviewed every changed range and verified actionlint, pnpm guard, web and daemon typechecks, 234 focused web tests, 52 focused daemon tests, diff hygiene, and the focused Playwright scenario. Thanks for the quick, thoughtful follow-up. 🙏
Cancel the URL-to-srcDoc handoff retries as soon as the user interacts with the active document, so the delayed restore cannot overwrite newer navigation or saved attributes. Cover the delayed callback window in the runtime bridge regression test. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
|
Looper fixer round complete — d03afea
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@Siri-Ray, thank you for the thoughtful iteration on the URL-to-srcDoc handoff. I reviewed every changed range and verified the focused web tests (234 passed), daemon tests (52 passed), web/daemon typechecks, guard, workflow lint, and diff hygiene. The latest head still has one reproducible blocking regression in the existing Manual Edit history suite, detailed inline. The earlier unresolved prewarmed-srcDoc activation thread at FileViewer.tsx:syncBridgeModes also remains applicable: the new retry cancellation prevents stale delayed replay, but it still does not dispatch the pending restore when activation is only a visibility swap. Thanks again for pushing this tricky lifecycle forward. 🙏
Dispatch and consume the captured URL-preview snapshot when an already-materialized srcDoc becomes active, because that visibility-only transition does not fire another iframe load event. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
|
Looper fixer round complete — cd58472
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@Siri-Ray, thank you for the careful follow-up on the prewarmed srcDoc path—the new activation restore and one-shot consumption coverage are both useful. I reviewed every changed range and verified guard, actionlint, web and daemon typechecks, 235 changed-area web tests, and 52 focused daemon tests. One current-head regression still blocks the existing Manual Edit history suite, detailed inline; the fix reply on the prior thread addressed prewarming but not this asynchronous activation failure. Thanks again for working through this tricky iframe lifecycle. 🙏
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| } | ||
| const activationFileIdentity = previewFileIdentityRef.current; | ||
| manualEditActivationPendingRef.current = true; | ||
| void capturePreviewRuntimeState(urlPreviewIframeRef.current) |
There was a problem hiding this comment.
Blocking: preserve the existing Manual Edit activation contract while waiting for the runtime-state capture. This changed line defers enterManualEditMode() until the capture response or the 500 ms timeout, so any edit-selection message emitted during that interval is handled while manualEditMode is still false and is discarded. On this exact head, corepack pnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/components/FileViewer.manual-edit-history.test.tsx fails five of six existing scenarios at panelState.props remaining null; the two newly changed web test files pass, so this is specific to the uncovered activation path. This matters because the PR currently breaks the required Web workspace test lane and can lose a user’s first interaction during the pending handoff. Please either introduce an explicit pending/active transition that queues or rejects follow-up actions without dropping them, or update the activation/fixture contract so capture-capable frames acknowledge the request and callers wait for the visible active state before sending selections. Then restore all six history scenarios and run the full Web workspace test lane.
Teach the Manual Edit history fixture to answer the URL preview runtime-state capture request and wait for the srcDoc transport to become explicitly active before sending edit selections. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
Complete the URL preview runtime-state capture response and wait for explicit srcDoc activation across the remaining Manual Edit regression fixtures, preserving their original interaction assertions. Generated-By: looper 0.11.0 (runner=fixer, agent=codex)
|
Looper fixer round complete — 46b300d
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@Siri-Ray Thank you for the careful iteration on this tricky iframe lifecycle. I reviewed every changed range and verified the runtime-state capture/restore handoff, one-shot consumption across prewarmed and rebuilt srcDoc transports, and the updated activation fixtures. Guard, workflow lint, web and daemon typechecks, 254 focused web tests, 52 focused daemon tests, diff hygiene, and the focused multi-page Playwright scenario all pass; the final regression coverage is especially strong. 🙏
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
Visual regression reviewHead:
0 changed · 0 unchanged · 10 new without baseline · 1 failed Capture or diff failures
New cases without baselines
Visual diff is advisory only and does not block merging. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@Siri-Ray Thank you for the careful work on this tricky iframe lifecycle. I reviewed every live changed range on the merge-synced head and verified that the URL-to-srcDoc runtime-state handoff, prewarmed activation, one-shot restore consumption, and interaction-safe retry logic remain identical to the previously approved feature patch. The current Web job also shows the changed Manual Edit suite green (14 tests passed); its remaining failures are broad base-sync assertions outside this PR's changed ranges. The focused daemon coverage, workflow-only lint fixes, and multi-page regression scenario are well scoped. Nice work carrying this through the follow-up cycle. 🙏
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.3 participants
Why
While editing a multi-page mobile prototype, navigating away from the home page and then clicking Edit unexpectedly returned the preview to the home page. This interrupted the designer's current context and made page-specific edits start from the wrong screen.
The preview normally runs as a URL-loaded iframe, but Manual Edit requires the bridge-enabled
srcDoctransport. Switching transports created a fresh document and discarded mutable runtime UI state such as the active page, hidden states, form values, and scroll positions.What users will see
Clicking Edit while viewing a non-home page in a multi-page mobile prototype now keeps that page visible. Exiting Edit also preserves the page, and the prototype's navigation continues to work normally.
Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-pack/tools-prflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies)srcDoctransport switchScreenshots
No visual appearance or entry-point change. This PR preserves runtime navigation state during the existing Edit transition; the behavior is covered by the red/green Playwright scenario below.
Bug fix verification
e2e/ui/app-manual-edit.test.ts[P0] manual edit mode preserves the current page in a multi-page mobile appscenario failed before the source change on the feature-line baseline, with the preview returning to Home after entering Edit.Validation
corepack pnpm install --frozen-lockfilecorepack pnpm guardcorepack pnpm --filter @open-design/web typecheckcorepack pnpm exec vitest run -c vitest.config.ts tests/project-file-range.test.tsinapps/daemon— 46 passedapps/webVitest coverage for runtime-state capture, transport activation, bridge injection, and restoration — 6 passedcorepack pnpm exec playwright test -c playwright.config.ts ui/app-manual-edit.test.ts --grep "preserves the current page in a multi-page mobile app" --workers=1— 1 passedgit diff --check origin/feat/workspace-team...HEADcorepack pnpm typecheckreaches and passes the relevant Web and E2E workspaces, then fails on six pre-existingpayload is of type 'unknown'errors in the untouchedapps/daemon/tests/run-create-workspace-gate.test.ts.