[1985 by GPT-5.6 Sol] Support ProPR Connect tunnel endpoints seamlessly in desktop - #1988
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
… seamlessly Implemented by ProPR AI using gpt-5.6-sol model. Implementation completed successfully.
|
/ultrafix |
|
🔄 Ultrafix loop started (goal: 8/10, max cycles: 10) First action:
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit 572a133 Summary of ChangesFixed PR #1988’s build blockers:
Verified:
No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Fixed PR #1988’s build blockers: - Restored correct URL warning classification in [runtimeConfig.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-29T21-22-36/propr-ui/src/config/runtimeConfig.ts:100). - Compacted the Connect verification markup in [DesktopExperience.tsx](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-29T21-22-36/propr-ui/src/desktop/DesktopExperience.tsx:96) to satisfy the 400-line lint limit. Verified: - Failing CI regression: 66/66 passed - Focused UI/desktop tests: 88/88 passed - Client tests: 17/17 passed - UI typecheck, lint, and production build passed - Notification regression stage passed - Browser smoke tests: 4/4 passed - CLI package verification passed - `git diff --check` passed No commit was created. PR: #1988 Comment by: @github-actions[bot] (ID: 5464959244) Model: gpt-5.6-sol
|
/review Review exact Connect desktop head 572a133 against transport epic base as a pre-reconciliation security and UX gate. Audit strict bare managed tunnel origin parsing: HTTPS t-.propr.dev only, with no IDNA lookalike, userinfo, port, path, query, fragment, credential, or relay-token crossover. Verify one exact origin for compatibility, pairing, REST, and Socket.IO; API-returned same-origin browser entry versus hosted app.propr.dev approval redirect; explicit confirmation with no silent profile overwrite; stale tunnel and restart warnings; main-renderer secret boundaries; and preservation of arbitrary remote and localhost flows. Check async switch and unmount races, same-host reconnect, origin-change fresh pairing, URL or log or DOM leakage, accessibility, and bounded errors. This head intentionally predates secret-free CLI discovery and runtime deep-link reconciliation: report those as integration requirements, while distinguishing exact-head defects. No code changes. Give blocker IDs and a score. |
|
ℹ️ Analyzed the follow-up request by @integry Analysis SummaryGate result: fail — 4 blockers. Score: 4/10. Reviewed exact head Exact-head blockers
Expected reconciliation requirementsThese are not counted as exact-head defects:
Passed audit areasThe shared client keeps compatibility, REST, and Socket.IO on one normalized origin; arbitrary HTTPS remotes and loopback HTTP remain supported. Connection-attempt generation and serialized profile mutations cover the tested switch/unmount races. Confirmation controls and the new status announcement are accessible, and pairing approval remains explicit. Focused verification passed:
No tracked files were changed. No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/review Exact-head review only for 572a133. Use the detailed audit scope in comment 5465088388. Distinguish current-head blockers from the expected later discovery and runtime deep-link reconciliation. Do not modify code. Report blocker IDs and score. |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR provides a strong, well-tested trust boundary for ProPR Connect origins, but needs one localized pairing correction before merge. ✅ Strict origin verification — Raw-authority comparison prevents default-port, encoded-host, credential, Unicode/IDNA, path, query, fragment, and suffix-lookalike inputs from acquiring Connect trust. ✅ Consistent transport routing — REST, compatibility/status requests, and Socket.IO retain the exact normalized Connect origin and fixed transport paths. ✅ Bound approval contracts — The shared validator accepts only the matching hosted approval URL or exact same-origin browser route, without permitting secret-bearing parameters. All current-head checks pass. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Noncanonical Connect configuration starts broken pairing
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Complete runtime reconciliationDuring the intentionally later discovery and deep-link integration, route every API-returned approval URL through ScoreThe Connect parser, transport behavior, approval validator, UX indicator, and tests are otherwise strong, and all authoritative checks pass. The inconsistent explicit-port pairing path is localized but must be corrected before merge. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
|
/fix F1 Fix formal F1 plus all four independently reproduced exact-head blockers on 572a133. Do not merge or base-sync yet.
Keep the exact origin for compatibility, pairing, REST and Socket.IO; validate every API-returned approval URL immediately before main opens it once reconciliation adds the controller; tokens and device secrets remain main-only; origin changes require a fresh identity. Run Connect parsing, shared pairing, API auth, client, runtimeConfig, desktop UX, accessibility, launcher drift, typechecks, full suite, package guard and git diff --check. Preserve current green arbitrary remote, localhost, strict approval and same-host semantics. |
|
✅ Applied the requested follow-up changes in commit 0b7c296
Summary of ChangesImplemented F1 only.
Changed desktopAuthService.ts and desktopAuth.test.ts. Verification passed:
The full 321-file suite reached 176 files before hanging because Redis is unavailable at 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented F1 only. - Noncanonical reserved Connect URLs—including explicit `:443` and percent-encoded hosts—now fail before pairing creation. - Returns bounded `PAIRING_CONFIGURATION_INVALID` / HTTP 503 without reflecting configured input. - Valid Connect, arbitrary HTTPS remotes, lookalikes, and loopback behavior remain unchanged. - Added regressions confirming no pairing row is persisted. Changed [desktopAuthService.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-29T22-14-16/packages/api/desktopAuthService.ts:152) and [desktopAuth.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-29T22-14-16/packages/api/test/desktopAuth.test.ts:101). Verification passed: - 67 Connect/client/API tests - 24 desktop tests - 96 runtime-config/desktop UX tests - 10 launcher drift tests - Affected typechecks and API lint - CLI release-package guard - `git diff --check` The full 321-file suite reached 176 files before hanging because Redis is unavailable at `127.0.0.1:6379`; it was stopped after repeated connection failures. No merge, base sync, commit, or PR creation was performed. PR: #1988 Comment by: @integry (ID: 5465187212) Model: gpt-5.6-sol
|
!propr Implement the remaining independently reproduced blockers on exact head 0b7c296. Formal F1 API rejection is complete; do not redo or weaken it.
Run runtimeConfig and hosted-flow tests, shared pairing/client/API tests, desktop UX and accessibility tests, typechecks, lint, full suite, package guard and git diff --check. Preserve completed API F1, exact origin transport, arbitrary remote and localhost behavior, approval validation, and main-only token custody. No base sync or merge. |
|
✅ Applied the requested follow-up changes in commit 68abb05 Summary of ChangesImplemented the remaining blockers on exact head Key changes:
Validation passed:
The repository-wide runner reached 175/321 files, then hung on Redis-dependent tests because neither Redis nor Docker is available. A resumed run encountered the same environment blocker at No commit, merge, sync, or PR operation was performed. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…c0e83b082c4c75d8ac8894cd947aeb1`. Implemented the remaining blockers on exact head `0b7c2961ac0e83b082c4c75d8ac8894cd947aeb1`. Key changes: - Strict raw Connect shorthand validation; credentials, ports, encoding, Unicode, paths, whitespace, alternate casing, trailing dots, and lookalikes are rejected before flow persistence. - Reserved `t-*.propr.dev` attempts can no longer downgrade to generic HTTPS pairing/client behavior. - Malformed runtime configuration is bounded and redacted. The API client is constructed only after successful validation, otherwise a safe configuration screen renders. - Desktop logs, IPC errors, DOM messages, and profile presentation avoid raw endpoints and failure details. - Added managed-tunnel stale/restart recovery with Retry, Re-enter, Rediscover, and explicit confirmation before replacing a profile. - Added a secret-free optional rediscovery adapter seam. - Formal F1 API files were not modified. Validation passed: - Client/shared pairing: 20 tests - API desktop-auth/status: 39 tests - Desktop/Electron: 24 tests - Full UI: 507 tests across 70 files - Root, client, UI, and desktop typechecks - Root and UI lint - CLI package guard - `git diff --check` The repository-wide runner reached 175/321 files, then hung on Redis-dependent tests because neither Redis nor Docker is available. A resumed run encountered the same environment blocker at `llmMetrics.test.ts`. All affected and independently runnable suites pass. No commit, merge, sync, or PR operation was performed. PR: #1988 Comment by: @integry (ID: 5465256401) Model: gpt-5.6-sol
|
!propr Fix the exact-head Connect blockers on 68abb05. Do not merge or sync the transport base; #1961 auth and #1987 CLI discovery will be reconciled later.
Run shared/client/API/Electron/UI/launcher Connect suites, UI lint/typecheck/build, desktop typecheck/package, configured full suite, and git diff --check. Preserve bounded/redacted input and errors, lazy API construction, profile mutation serialization, approval URL binding, existing explicit-port pairing selector, and all non-Connect remote/loopback support. |
|
✅ Applied the requested follow-up changes in commit cdb9f41 Summary of ChangesImplemented the exact-head Connect follow-up without merging or syncing the transport base.
Verification:
No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…g or syncing the transport base. Implemented the exact-head Connect follow-up without merging or syncing the transport base. - Enforced byte-for-byte canonical Connect origins across shared, client, API, Electron, hosted UI, CLI, and launcher paths. Reserved nested/lookalike hosts cannot fall through as generic remotes. - Removed renderer discovery fallback from managed recovery. Trusted rediscovery now requires current-attempt fencing, matching opaque profile ID, strict endpoint validation, and explicit confirmation. - Added bounded/redacted confirmation copy and deterministic recovery regressions. - Split the affected UI modules to eliminate all six lint warnings without changing lint configuration. Verification: - Connect suites: passed - UI lint, typecheck, build: passed - Desktop typecheck and Electron package: passed - `git diff --check`: passed - Full suite: 320/321 runs passed; only `test/llmMetrics.test.ts` timed out waiting for Redis at `127.0.0.1:6379`, which is unavailable in this environment. No commit was created. PR: #1988 Comment by: @integry (ID: 5465471435) Model: gpt-5.6-sol
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR adds strong, centralized Connect endpoint classification, safer desktop error handling, recovery UX, and direct-origin transport routing. However, two localized validation inconsistencies remain, so it is not ready to merge yet. The full test suite is also still pending. ✅ Strict Connect classification — ✅ Transport behavior is well covered — Added tests verify that REST, status, and Socket.IO traffic use the Connect origin and expected paths directly. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F2: 🔴 Hosted gating does not reach API clients
F3: 🔴 Approval validator accepts rejected encodings
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe core implementation is thoughtful and extensively tested, but the two new trust-boundary inconsistencies must be corrected before merge. The remaining full-suite check is pending rather than failing. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
|
/fix F2 F3 |
|
✅ Applied the requested follow-up changes in commit cb6a954
Summary of ChangesImplemented only F2 and F3.
Verification passed:
🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented only F2 and F3. - Hosted API client creation is now blocked for missing or non-Connect configurations, while OAuth completion remains exempt: [runtimeConfig.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-30T00-13-37/propr-ui/src/config/runtimeConfig.ts:350). - Approval URLs now require canonical raw query names and values, rejecting encoded names/hosts: [desktopPairing.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-30T00-13-37/packages/shared/src/desktopPairing.ts:57). - Added API-client gating and cross-contract encoding regressions. Verification passed: - Shared build - Client tests: 21 passed - UI tests: 66 passed - UI typecheck and targeted lint - `git diff --check` PR: #1988 Comment by: @integry (ID: 5465683890) Model: gpt-5.6-sol
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR adds centralized ProPR Connect endpoint classification, approval-URL validation, hosted-tunnel state handling, and desktop recovery UX. The implementation is well tested and thoughtfully defensive, but three localized correctness gaps mean it needs minor changes before merge. ✅ Centralized endpoint trust policy — Connect parsing is shared across the client, desktop, API, CLI, and launcher, with extensive coverage for encoded hosts, ports, nested hosts, and Unicode lookalikes. ✅ Approval URLs are tightly bound — ✅ Invalid configuration fails safely — The UI avoids constructing an API client and renders bounded diagnostics without exposing injected credentials or private URL components. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F4: 🔴 Oversized Connect URL bypasses canonical validation
F5: 🔴 Encoded tunnel name falls through
F6: 🔴 Help failure reported as pairing failure
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe overall design and test coverage are strong, and all current checks pass, but the canonical endpoint and selector fall-through defects affect security-sensitive routing boundaries. All three blockers appear locally correctable without redesign. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
|
/fix F4 F5 F6 |
|
✅ Applied the requested follow-up changes in commit 0aa02e0
Summary of ChangesImplemented only F4, F5, and F6:
Verification passed:
No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented only F4, F5, and F6: - Added the 2048-character guard before parsing `API_PUBLIC_URL` in [desktopAuthService.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-30T00-37-29/packages/api/desktopAuthService.ts:156). - Detects decoded `tunnel` parameter names such as `%74unnel`, blocking stored/configured fallback in [hostedTunnelConfig.ts](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-30T00-37-29/propr-ui/src/config/hostedTunnelConfig.ts:42). - Connect pairing errors now apply only to authentication; help-launch failures retain the bounded help-specific message in [DesktopExperience.tsx](/tmp/git-processor/worktrees/integry/propr/pr-1988-followup-2026-08-30T00-37-29/propr-ui/src/desktop/DesktopExperience.tsx:221). - Added focused regressions for all three findings. Verification passed: - API pairing suite: 8 tests - Focused UI suites: 82 tests - Desktop experience suite: 22 tests - API and UI typechecks - API and UI lint - `git diff --check` No commit was created. PR: #1988 Comment by: @integry (ID: 5465785343) Model: gpt-5.6-sol
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR is ready to merge. It consistently establishes a strict Connect trust boundary while preserving ordinary HTTPS and loopback behavior. ✅ Canonical endpoint verification — Shared, client, desktop, CLI, and launcher paths reject noncanonical reserved-host spellings before they can fall back to ordinary remote handling. ✅ Approval URL binding — Pairing approvals are constrained to the expected hosted contract or the exact API-origin ✅ Strong regression coverage — Tests exercise encoded authorities, ports, nested hosts, Unicode lookalikes, stale recovery races, and secret-bearing failures; all current checks pass. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Wire the future pairing consumerWhen the credential/profile-generation stack referenced in the PR context becomes available, wire ScoreThe implementation is cohesive, security-conscious, thoroughly tested, and merge-ready with no verified regressions or failing checks. Score: 9/10 🤖 Review Details
🤖 Review by ProPR |
1 participant
AI Implementation Summary
Closes #1985
Branch:
1985/gpt-5.6-sol-support-propr-connect-tun-20260829-2108-al6Commits: ✅ Changes committed (67e2d2f)
AI Processing Completed
Execution Details:
Summary:
Implemented the integration-safe ProPR Connect support for issue #1985.
/browserfallback./api/status, and Socket.IO use the Connect origin directly.Because #1961’s
DesktopCredentialServiceand profile-generation stack are not present on this base, no parallel credential or pairing stack was introduced, as required by the issue.All affected typechecks and targeted suites pass: 94 tests across client, Electron, UI, API, and launcher contracts.
Detailed Logs:
01a04f5b-78cc-7962-98c0-b755289c34c9Log files stored at:
/tmp/claude-logs/issue-1985-2026-08-29T21-21-02-780Z-conversation.json/tmp/claude-logs/issue-1985-2026-08-29T21-21-02-780Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #1985.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!