sync(desktop): propagate finalized runtime into transport epic - #1979
sync(desktop): propagate finalized runtime into transport epic#1979integry wants to merge 6 commits into
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 1953-epic-desktop-transport-api without dropping client/instance-pairing API 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 EvaluationThis integration sync is ready to merge into ✅ Defense-in-depth deep-link handling — ✅ Startup delivery is preserved — ✅ Packaged CSS and layout are verified — The renderer build rejects uncompiled Tailwind output, while the packaged smoke test checks representative selectors and real 1280×820 control bounds. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Bound the pending-link queue
S2: 🟢 Add a mounted-router testA future integration test mounting ScoreThe implementation is focused, well-tested, and has no verified code blocker or current-head failure. The remaining checks are pending rather than failing. Score: 9/10 🤖 Review Details
🤖 Review by ProPR |
Full Test Suite ResultsValidation failed during: Full test suite.
View Test Output |
|
✅ Applied the requested follow-up changes in commit 6fe54e8 Summary of ChangesFixed the flaky full-suite failure in DesktopExperience.test.tsx. The test now opens instance management through the title-bar control, avoiding an unrelated shortcut-listener timing race. Production behavior is unchanged. Validation:
No commit created. 🤖 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
…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
….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\nReview exact shared runtime head 48f8581 against the transport epic base. Re-audit canonical deep-link normalization across every decoding layer, prohibited /login and /desktop/pairing routes, reserved parameters, external/protocol-relative/traversal forms, startup buffering, renderer isolation, deterministic DesktopExperience coverage, and confirm the VAPID fixture padding is test-only and mathematically correct. Treat any validate-one-representation/return-another behavior or security 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: 5464613320✓ |
|
/review\n\nRetry after the transient missing-worktree failure. Review exact SHA 48f8581 against the transport epic base only. Re-audit canonical deep-link validation/output equivalence, prohibited auth/pairing routes, reserved parameters, external/protocol-relative/traversal forms, deterministic UI coverage, renderer isolation, and safe test-only VAPID scalar padding. No code modifications; report blockers and score. |
|
❌ Failed to apply follow-up changes requested by @integry An error occurred while processing your request: Comment ID: 5464690693✓ |
|
/review Review exact SHA 48f8581 against transport epic base 1953. Confirm canonical deep-link validation/output equivalence; encoded delimiter/traversal and auth/pairing paths are rejected; normal task query/hash works; renderer isolation and deterministic UI coverage remain; VAPID padding is safe test-only scope. No code changes. Report blockers and score. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR correctly integrates the Electron runtime, hardened dashboard deep links, compiled renderer styling, and deterministic packaged-layout coverage into the transport epic. It needs one localized correction before merge: deep-link validation is performed against the input representation, while a potentially longer canonical representation is returned. ✅ Layered route protection — Encoded delimiters and traversal are decoded and checked at every layer; external/protocol-relative paths, ✅ Renderer coverage remains isolated — Layout inspection runs only for packaged smoke tests, and the build verifies that representative Tailwind and dashboard selectors were compiled. ✅ VAPID padding is test-only and correct — The change is confined to Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Revalidate the canonical deep link
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe implementation is otherwise focused, well-tested, and supported by passing current checks, but the explicit canonical validation/output invariant still has a reproducible edge-case failure. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
|
/fix F1 Fix only the exact canonical serialization invariant on shared runtime head 48f8581. Serialize the candidate before return; reject if the canonical href exceeds 2,048 code units or contains prohibited controls; require dashboardPathFromDeepLink(canonicalCandidate) to succeed and equal the dashboard path validated from the input. Never validate one representation and deliver another. Add desktop and renderer regressions for a raw under-limit open path containing enough spaces/non-ASCII characters that WHATWG percent-serialization expands beyond 2,048, plus a near-boundary canonical value that remains valid. Preserve every encoded-delimiter/traversal/auth rejection and normal tasks query/hash behavior. Run desktop/UI focused tests, typechecks, package/smoke, full suite, and git diff --check; publish the exact branch head. |
|
✅ Applied the requested follow-up changes in commit d99f006
Summary of ChangesImplemented F1 only on base head Changes:
Verification:
Only the three scoped files are modified. Per instruction, I did not commit or push; the system must publish the resulting branch head. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…8dcb68971e6a0e6`. Implemented F1 only on base head `48f85811addf4bccfd390961e8dcb68971e6a0e6`. Changes: - [security.ts](/tmp/git-processor/worktrees/integry/propr/pr-1979-followup-2026-08-29T20-41-29/apps/desktop/src/security.ts) now checks the serialized canonical candidate’s length/controls and requires its dashboard path to equal the validated input path before returning it. - Added desktop and renderer regressions for percent-expansion beyond 2,048 and an exactly 2,048-character valid canonical link. - Existing traversal, encoded-delimiter, auth/pairing, query, and hash tests remain passing. Verification: - Desktop tests: 28 passed. - Focused renderer tests: 5 passed. - Desktop and UI typechecks: passed. - Production package: passed. - `git diff --check`: passed. - Packaged smoke: environment-blocked because no usable Chromium sandbox helper is installed/configured. - Full suite: reached file 155/320 before blocking on unavailable Redis (`127.0.0.1:6379`); neither Redis nor Docker is installed. Only the three scoped files are modified. Per instruction, I did not commit or push; the system must publish the resulting branch head. PR: #1979 Comment by: @integry (ID: 5464771154) Model: gpt-5.6-sol
|
/review Final review of exact runtime head d99f006. Verify canonical serialization is length/control checked and revalidated before delivery; raw and canonical dashboard paths must be equal. Reproduce 673-space candidate length 2046 accepted and 674-space candidate length 2049 rejected. Re-audit encoded delimiter/traversal, login/pairing/reserved/external/protocol-relative/control rejection, normal task query/hash navigation, startup buffering, renderer isolation, deterministic tests, and test-only VAPID padding. No code changes. Report blockers and score only after current-head checks. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR is merge-ready within scope, conditional only on the pending full test suite completing successfully. The authoritative current-head status has no failures, eight passing checks, and one pending check. ✅ Canonical equivalence enforced — ✅ Deep-link security preserved — Recursive decoding and path-scope validation reject encoded traversal/delimiters, controls, external and protocol-relative targets, login/pairing routes, and reserved parameters while preserving normal ✅ Runtime boundaries are well covered — Startup links remain buffered until dashboard readiness, renderer navigation receives only validated paths, packaged CSS/layout checks are deterministic, and VAPID scalar padding is correctly right-aligned to 32 bytes within test-only code. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe implementation satisfies the requested runtime and security invariants with focused regression coverage and no known current-head failure. The remaining full-suite check is still in progress but does not identify a code blocker. Score: 9/10 🤖 Review Details
🤖 Review by ProPR |
Integration-only sync of the finalized Electron runtime into the desktop transport/auth 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 #1977 the exact reviewed runtime/CSS/deep-link foundation. It must merge only into 1953-epic-desktop-transport-api, never main.