Preview proposal changes on saved board objects across experiences - #2872
Conversation
# Conflicts: # docs/product/WORKSPACE_OVERHAUL_VALIDATION.md
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Board proposal overlays are implemented in #2872, stacked on #2870. Local full/scoped/browser evidence is in the PR; no board writes or approval/apply were added. Independent review has no HIGH/CRITICAL finding. MEDIUM follow-through: synchronously invalidate an in-flight preview on Close, rather than waiting for the route unmount, and add a close-during-load regression. This is a short UI race and is tracked with the other #2808 interaction refinements instead of expanding the current review cycle. Source interaction #2870 full suites passed. Its MEDIUM deep-offset SQL scan cost is being addressed in the next source scalability slice, alongside the original-library metadata-query follow-through; upload413 error fidelity is also in progress. The umbrella remains open. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b262c8ecc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function closeProposalPreview() { | ||
| proposalMarkers.value = {} | ||
| const query = { ...route.query } | ||
| delete query.proposalId | ||
| void router.replace({ query }) |
There was a problem hiding this comment.
When the user clicks Close while load() is awaiting its two GETs, this handler clears only the parent marker map and waits for the asynchronous router.replace() to unmount the child. A response that lands first still matches the child's unchanged generation and can emit markers again after they were cleared; if navigation is delayed or aborted, those supposedly closed markers persist. Invalidate the child request synchronously as part of closing rather than relying on route-driven unmount.
Useful? React with 👍 / 👎.
| <router-link | ||
| v-if="activeProposal.boardId && !isArchivedHistory && ['PendingReview', 'Approved'].includes(normalizeProposalStatus(activeProposal.status))" | ||
| class="tk-meta" | ||
| :to="{ path: `/workspace/boards/${activeProposal.boardId}`, query: { proposalId: activeProposal.id } }" | ||
| >Preview on board</router-link> |
There was a problem hiding this comment.
For a PendingReview or Approved proposal whose expiresAt has passed, this condition still exposes Preview on board even though the Review code already classifies it as read-only and BuildProposalDiffAsync rejects expired proposals. The destination therefore offers only a refresh that fails. Add the existing isProposalExpired(activeProposal) guard here and the equivalent proposal.isExpired/expiry guard in ReviewProposalDetails.vue:172.
Useful? React with 👍 / 👎.
| <h2>Proposed board changes</h2> | ||
| <button type="button" :disabled="loading || !available" @click="load">{{ loading ? 'Checking proposal…' : 'Refresh board preview' }}</button> | ||
| <RouterLink :to="{ path: '/workspace/review', query: { boardId: board.id }, hash: `#proposal-${proposalId}` }">Open Review</RouterLink> | ||
| <button type="button" @click="emit('close')">Close preview</button> |
There was a problem hiding this comment.
When Italian or Spanish is selected, this new board panel remains entirely in English because its labels and explanatory copy are hardcoded rather than read through vue-i18n. Board and Review are already extracted surfaces, and ADR-0054 requires future user-visible strings on them to be added to all three catalogs, so this introduces a mixed-language core flow. Add board/review catalog keys for the panel and the new Review links instead of embedding English literals.
Useful? React with 👍 / 👎.
b262c8e
into
codex/2808-overhaul-integration
1 participant
Proposals could be previewed in Chat and Review, but users still had to match the described changes to board objects themselves. Preview on board now opens a checked, read-only layer over the saved board in both renderers and all four experiences. Existing card/column targets are marked, while the diff describes new or hidden objects; saved board data remains unchanged.
The panel verifies board/proposal identity, effective revision, status and update time across the checked diff and operation payload. Execution parameter IDs take precedence over display IDs. Account, board, refresh/access failures and a short server-relative freshness window retract the preview. Open Review returns to the exact proposal; approval and Apply stay in Review. This continues #2808 and is stacked on #2870.
Verification: full frontend 6,368 passed with three existing skips before final target/access refinements; final scoped 49 passed, and 63 combined overlay/audio tests passed after integrating the corrected parent. Typecheck, production build, scoped ESLint, doc links and governance passed. A real API Chromium journey passed in15.8seconds across Classic/Studio/Companion/Unified, both renderers and375px, with no overflow or serious/critical axe findings, unchanged saved cards/columns, zero browser mutation calls, explicit close, Review navigation and403 recovery. Initial synthetic fixture errors were corrected and failed logs retained. Only validation append text conflicted during the parent merge; both sections were retained and runtime matches the independently reviewed feature commit.
Independent Luna review found no HIGH/CRITICAL defect. MEDIUM follow-through: Close currently relies on route unmount to invalidate a pending request, leaving a brief response/route race; a synchronous close invalidation regression is tracked under #2808. No provider quality, physical-device, actual Apply or production deployment acceptance is claimed. OUTSTANDING_TASKS.md owner decisions remain unchanged.