sync(desktop): propagate finalized runtime into UX epic - #1980
Conversation
…the #1960/#1961 placeholder boundary. Implemented the packaged-renderer CSS fix without changing the #1960/#1961 placeholder boundary. Key changes: - Desktop Vite now loads `propr-ui`’s PostCSS pipeline using native cross-platform paths. - Tailwind resolves its config/content relative to `propr-ui`. - Production builds fail if emitted CSS contains `@tailwind`/`@apply` or lacks `.h-5`, `.space-y-5`, `.bg-primary-500`, or `.dashboard-card`. - Packaged smoke now measures the 1280×820 window, logo bounds, controls, help text, button, and runtime footer spacing. Validation passed: - Production desktop package - Emitted CSS inspection - Desktop/UI typechecks - 24 Electron tests - 21 DesktopExperience tests - Web UI production build - `git diff --check` The sandboxed launch was attempted but blocked before window creation because this non-root container cannot configure Electron’s root-owned `4755` sandbox helper or create a user namespace. The smoke remains strict and does not use `--no-sandbox`; CI already provisions the helper correctly. PR: #1971 Comment by: @integry (ID: 5464159479) Model: gpt-5.6-sol
Implemented F3 only; S1 remains untouched. - Hardened `propr://open` validation in [security.ts](/tmp/git-processor/worktrees/integry/propr/pr-1971-followup-2026-08-29T19-10-30/apps/desktop/src/security.ts:25), rejecting external/protocol-relative paths, traversal, controls, malformed encoding, login/pairing routes, and hosted-flow parameters. - Added ordered startup buffering and post-load hash routing in [desktop-deep-link.ts](/tmp/git-processor/worktrees/integry/propr/pr-1971-followup-2026-08-29T19-10-30/propr-ui/src/desktop-deep-link.ts:3), integrated at [desktop.tsx](/tmp/git-processor/worktrees/integry/propr/pr-1971-followup-2026-08-29T19-10-30/propr-ui/src/desktop.tsx:145). - Added `/tasks`, startup-buffer, post-load, and unsafe-input regressions. Validation: - Electron tests: 26/26 passed - Focused UI tests: 5/5 passed - Desktop/UI typechecks: passed - UI lint: passed - Production package: passed - `git diff --check`: passed - Sandboxed layout/API-origin smoke: attempted twice, but the container cannot launch Electron because generated `chrome-sandbox` is `node:node 0755`; Electron requires root ownership and mode `4755`. No sandbox-disabling workaround was used. PR: #1971 Comment by: @integry (ID: 5464313572) Model: gpt-5.6-sol
|
/review Integration-only review. Confirm this exact already-reviewed runtime head can merge cleanly into 1951-epic-desktop-experience-uxs without dropping UX/setup-engine/desktop chooser behavior or changing the target to main. Source validation and 9/10 no-blocker review are documented in the PR body. |
|
/review |
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR cleanly integrates the Electron runtime foundation, including deterministic Tailwind compilation, packaged-renderer layout validation, and buffered dashboard deep links. It needs one localized security correction before merge and must remain targeted to ✅ Strong build validation — The renderer build fails when CSS is absent, Tailwind directives remain, or representative desktop/dashboard selectors are missing. ✅ Meaningful packaged smoke coverage — The smoke test verifies real 1280×820 layout bounds alongside sandbox, preload bridge, and profile API proofs. ✅ Well-covered deep-link lifecycle — Tests exercise startup buffering, post-load delivery, malformed inputs, reserved parameters, and restricted routes. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Encoded delimiter bypasses route restrictions
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe integration is otherwise focused and well tested, with no reported current-head failures, but the new deep-link validator has a reproducible restricted-route bypass that must be corrected before merge. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
|
/fix F1 Fix only the canonical deep-link discrepancy on the shared runtime head. The exact path /tasks%23/%2e%2e/login (and analogous encoded ? delimiter variants) must never validate one decoded representation and return/navigate a different prohibited representation. Derive the returned route from one canonical representation, or validate every representation actually returned, so /login, /login/*, /desktop/pairing, reserved auth/hosted-flow parameters, protocol-relative/external paths, traversal, encoded controls, and malformed/repeated encodings all fail closed. Add regressions for encoded # and ? delimiters combined with single/double-encoded dot segments, plus normal /tasks query/hash routing. Preserve startup buffering and the existing 2,048-byte/decoding bounds. Run desktop security tests, focused UI navigation tests, typechecks, production package, and sandboxed smoke. This source branch also feeds transport sync #1979 and packaging #1972; make no unrelated changes and do not merge to main. |
|
✅ Applied the requested follow-up changes in commit 87a6e62
Summary of ChangesImplemented F1 only.
Changed: Verification:
No commit or unrelated changes were made. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…st.tsx](/tmp/git-processor/worktrees/integry/propr/pr-1979-followup-2026-08-29T19-43-24/propr-ui/src/desktop/DesktopExperience.test.tsx:293). Fixed the flaky full-suite failure in [DesktopExperience.test.tsx](/tmp/git-processor/worktrees/integry/propr/pr-1979-followup-2026-08-29T19-43-24/propr-ui/src/desktop/DesktopExperience.test.tsx:293). The test now opens instance management through the title-bar control, avoiding an unrelated shortcut-listener timing race. Production behavior is unchanged. Validation: - UI suite: 70 files, 500 tests passed - UI typecheck: passed - `git diff --check`: passed No commit created. PR: #1979 Comment by: @github-actions[bot] (ID: 5464467107) Model: gpt-5.6-sol
|
!propr The reported F1 repair commit 87a6e62 did not reach the PR branch because the concurrent CI-only commit 6fe54e8 won the shared-head update. GitHub still reports exact head 6fe54e8, and the encoded-delimiter bypass remains there. Reapply the already-developed canonical deep-link repair and its tests on top of current live head 6fe54e8, preserving the CI test change. Verify the published PR head actually contains the fix for /tasks%23/%2e%2e/login and encoded ? variants before reporting completion. Run the desktop/UI tests and typechecks, package/smoke, and make no unrelated changes. |
|
✅ Applied the requested follow-up changes in commit c524a9e Summary of ChangesReapplied the canonical deep-link repair on exact head Changed only: Verification:
Packaged smoke was attempted but the non-root worker cannot configure the required root-owned setuid Chromium sandbox helper and lacks Xvfb. The application correctly refused to launch without sandboxing. Per instruction, I did not commit or push. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…54e8667a648429c5e4080d44bb0d5ff936abe`. Reapplied the canonical deep-link repair on exact head `6fe54e8667a648429c5e4080d44bb0d5ff936abe`. Changed only: - [security.ts](/tmp/git-processor/worktrees/integry/propr/pr-1980-followup-2026-08-29T19-52-01/apps/desktop/src/security.ts:32) - [security.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1980-followup-2026-08-29T19-52-01/apps/desktop/src/security.test.ts:77) - [desktop-deep-link.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1980-followup-2026-08-29T19-52-01/propr-ui/src/desktop-deep-link.test.ts:30) Verification: - Desktop: 27/27 passed - Focused UI: 25/25 passed - Full UI: 500/500 passed - Desktop/UI typechecks: passed - Production package: passed - Exact encoded `#` and `?`, single/double-encoded traversal variants: all returned `null` - Normal `/tasks?status=open#recent`: preserved - CI-only `DesktopExperience.test.tsx` change: preserved - Final diff: three requested files only Packaged smoke was attempted but the non-root worker cannot configure the required root-owned setuid Chromium sandbox helper and lacks Xvfb. The application correctly refused to launch without sandboxing. Per instruction, I did not commit or push. `git ls-remote` therefore still reports the published head as `6fe54e8`; post-publication verification can only occur after the system creates and publishes its automatic commit. PR: #1980 Comment by: @integry (ID: 5464512567) Model: gpt-5.6-sol
Full Test Suite ResultsValidation failed during: Full test suite.
View Test Output |
|
✅ Applied the requested follow-up changes in commit 48f8581 Summary of ChangesFixed the full-suite flake in notificationManagementRoutes.test.ts. Node occasionally emits a 31-byte ECDH private scalar by omitting a leading zero. The fixture now pads it to VAPID’s required 32-byte width, preventing valid mismatched-key cases from being misclassified as malformed. No production code changed. Validation:
The published PR head is A local full-suite rerun was unavailable because this worker has neither Docker nor Redis. Sandboxed smoke was also blocked by the environment’s missing root-owned Chromium sandbox helper and Xvfb. No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
….test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1980-followup-2026-08-29T19-58-19/packages/api/test/notificationManagementRoutes.test.ts:65). Fixed the full-suite flake in [notificationManagementRoutes.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1980-followup-2026-08-29T19-58-19/packages/api/test/notificationManagementRoutes.test.ts:65). Node occasionally emits a 31-byte ECDH private scalar by omitting a leading zero. The fixture now pads it to VAPID’s required 32-byte width, preventing valid mismatched-key cases from being misclassified as malformed. No production code changed. Validation: - Previously failing test: 3/3 passed - Desktop tests: 27/27 passed - Focused UI tests: 25/25 passed - Desktop/UI typechecks: passed - Production package: passed - `git diff --check`: passed - Final diff: one test file only The published PR head is `c524a9e5`, contains the canonical deep-link fix, and still targets `1951-epic-desktop-experience-uxs`. A local full-suite rerun was unavailable because this worker has neither Docker nor Redis. Sandboxed smoke was also blocked by the environment’s missing root-owned Chromium sandbox helper and Xvfb. No commit was created. PR: #1980 Comment by: @github-actions[bot] (ID: 5464573988) Model: gpt-5.6-sol
|
/review\n\nRe-review exact shared runtime head 48f8581 after F1. Confirm /tasks%23/%2e%2e/login and encoded ? variants are rejected in every API/UI path, normal /tasks query/hash navigation remains intact, canonical output is the representation actually validated, the UI timing test is deterministic, and the VAPID fixture padding is test-only and correct. Treat any bypass or regression as a blocker. Do not modify code; report score and findings on this exact head. |
|
❌ Failed to apply follow-up changes requested by @integry An error occurred while processing your request: Comment ID: 5464613934✓ |
|
/review Retry the exact-head review after the transient missing-worktree infrastructure failure. Review SHA 48f8581 only. Confirm the canonical encoded-delimiter/traversal bypass is closed while normal /tasks query/hash navigation works, every returned representation was validated, deterministic UI coverage remains, and the VAPID padding is safe test-only scope. No code changes; report findings and score. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationSHA ✅ Canonical bypass closed — ✅ Navigation and UI coverage preserved — Normal ✅ VAPID padding safely scoped — The fixed-width left-padding occurs only in Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe changed security path returns the final canonical representation that passed validation, targeted regressions cover the reported bypass in both API and UI paths, and no current-head check is failing. The remaining checks are pending rather than failed. Score: 9/10 🤖 Review Details
🤖 Review by ProPR |
2611e5f
into
1951-epic-desktop-experience-uxs
Integration-only sync of the finalized Electron runtime into the desktop UX sub-epic.
Source d5465a3 passed independent clean Node 22 audits, desktop/UI typechecks, 26 Electron tests, focused deep-link tests, production packaging, sandboxed 1280x820 visual smoke, all current-head CI, and final Ultrafix review 9/10 with no blockers. This PR exists only to give #1978 the exact reviewed runtime/CSS/deep-link foundation. It must merge only into 1951-epic-desktop-experience-uxs, never main.