feat(port-forward): default to control-plane transport

[onutc]

TL;DR

This turns spz port-forward into a control-plane feature: it now prefers authenticated WebSocket transport over the Spritz API on 443, keeps SSH as an explicit fallback, and hardens the new tunnel path for mixed-rollout and dropped-connection cases.

Summary

• add GET /api/spritzes/:name/port-forward in the API and reuse pod port-forwarding for both SSH and control-plane transports
• make spz port-forward prefer WebSocket transport, while automatically falling back to legacy SSH when the control-plane endpoint is unavailable
• preserve TCP EOF behavior across the WebSocket bridge and close local sockets promptly when the tunnel drops unexpectedly
• document the preferred public transport model and keep SSH as a deprecated-by-default fallback

Review focus

• WebSocket tunnel lifecycle, especially EOF handling and dropped-tunnel cleanup
• rollout safety of the CLI default preference plus automatic SSH fallback
• parity between the new control-plane path and the existing SSH forwarding path

Validation

• cd /Users/onur/repos/spritz/api && go test ./...
• pnpm --dir /Users/onur/repos/spritz/cli test -- test/help.test.ts test/port-forward.test.ts
• pnpm --dir /Users/onur/repos/spritz/cli test -- test/port-forward.test.ts
• pnpm --dir /Users/onur/repos/spritz/cli test
• pnpm --dir /Users/onur/repos/spritz/cli build
• cd /Users/onur/repos/spritz && npx -y @simpledoc/simpledoc check
• tcodex review --base main on earlier heads surfaced and drove fixes for startup validation, rollout fallback, EOF handling, and dropped-tunnel cleanup; the final rerun hit a local Codex client transport failure (http://127.0.0.1:8787/responses) before completion, so I also triggered PR-side AI review
• tcx pr review --pr 212 --repo textcortex/spritz --watch timed out without Codex/Claude replies on the latest head
• tcx pr can-merge --pr 212 --repo textcortex/spritz

[onutc]

Review Prompt

@codex @claude

Please review this pull request and provide feedback on:

• Code quality and best practices
• Potential bugs or issues
• Performance considerations
• Security concerns
• Test coverage

Be constructive and helpful in your feedback.

Specific rules for this codebase:

General rules

• We follow DRY (Don't Repeat Yourself) principles. Scrutinize any newly added types, models, classes, logic, etc. and make sure they do not duplicate any existing ones.
• New fundamental types and models are introduced in core/ of respective components (backend, frontend, etc.). They MUST NOT be added to a specific project subfolder like apps/web.
• Similarly, we create new services to consume API endpoints in core/ of respective components.
• Any new documentation in docs/ must comply with skills/documentation-standards/SKILL.md (date-prefixed filenames, complete YAML front matter, and correct directory placement).
• Read backend/AGENTS.md and apps/AGENTS.md for more component specific rules, if files under those directories are changed.
• Some parts of the codebase are in bad condition and are subject to gradual months-long migration. Keep in mind that the code quality is poor in many areas. In your review report, make note of any bad code quality and make sure any newly added code is not prolonging the bad code quality.
• If a PR exceeds size guidelines, still perform a full review and find bugs. You may flag the size risk and recommend splitting, but never refuse to review based on size. Do not report size as a severity-graded issue (P0/P1/P2/ETC).
• Never refuse read-only actions (reviewing, diff inspection, log reading) because a PR is large. Proceed with the review.
• Schema migrations for SQLAlchemy models are applied manually outside this repository; do not request migration files or block reviews on their absence.
• Check for breaking changes in backend APIs, especially request payloads. Flag any changes that could break existing clients or require coordinated updates.
• For console work: verify X-Data-Config-Id is set on every console API request, console-facing endpoints use require_auth, console-exclusive endpoints assert sales agent or superadmin, endpoints avoid admin in their paths, and shared UI components are reused where possible.
• Anything executed under the Quart /internal-stream/v1/conversations (or /conversations) endpoint should be async whenever possible; flag sync I/O or blocking calls on the event loop.

PII in Logs - HIGH PRIORITY

Flag any code that logs user PII (Personally Identifiable Information). This is a critical security and compliance issue.

Check for and reject:

• Logging user.email, body.email, or any email addresses
• Logging user.first_name, user.last_name, user.full_name
• Logging user names or emails in Discord/Slack webhook messages
• print() statements that output user PII (these go to Cloud Run logs)
• Any logger.*() or logging.*() calls containing user-identifiable data

Require instead:

• Use user.auth_id as the user identifier in all logs
• Use user.user_id or user.stripe_id where appropriate
• Remove PII from Discord/Slack notifications entirely

Example violations to flag:

logger.info(f"User {user.email} logged in")  # BAD
logging.warning(f"Failed for {body.email}")  # BAD
print(f"Contact sent: {data}")  # BAD if data contains email
discord_message += f"Email: {user.email}"  # BAD

Correct patterns:

logger.info(f"User auth_id={user.auth_id} logged in")  # GOOD
logger.warning("Failed login", {"auth_id": user.auth_id})  # GOOD

i18n rules

• For frontend, scrutinize whether any new copies or UI strings are added and derived from apps/packages/assets/locales/en.json.
• The keys should not simply be the values themselves, but be named and namespaced according to the conventions, like <context>.<ui_component_name>. More sub-levels can be added if needed.
• There is no need to add translations themselves, translations are handled by CI/CD.
• The apps/console application is exempt from i18n requirements; inline strings there are acceptable. Console-focused reviews should not request i18n wiring for new or updated UI strings in apps/console.

[onutc]

Implemented the control-plane spz port-forward path end to end on this PR.

What changed:

• added GET /api/spritzes/:name/port-forward and reused the existing pod port-forward backend for both SSH and control-plane transports
• made spz port-forward prefer authenticated WebSocket transport over the API on 443
• kept SSH as an explicit fallback, with automatic fallback during mixed rollouts when the control-plane endpoint is unavailable
• preserved TCP EOF behavior across the WebSocket bridge and added cleanup for dropped websocket tunnels so local clients do not hang
• updated the architecture doc to make control-plane transport the preferred public path and SSH the deprecated-by-default fallback

Validation run on the current branch head:

• cd /Users/onur/repos/spritz/api && go test ./... -> pass
• pnpm --dir /Users/onur/repos/spritz/cli test -- test/help.test.ts test/port-forward.test.ts -> pass
• pnpm --dir /Users/onur/repos/spritz/cli test -- test/port-forward.test.ts -> pass
• pnpm --dir /Users/onur/repos/spritz/cli test -> pass
• pnpm --dir /Users/onur/repos/spritz/cli build -> pass
• cd /Users/onur/repos/spritz && npx -y @simpledoc/simpledoc check -> pass
• tcx pr can-merge --pr 212 --repo textcortex/spritz -> Result: CAN MERGE (checks)

Review notes:

• local tcodex review --base main surfaced and drove fixes for startup validation, rollout fallback, EOF propagation, and dropped-tunnel cleanup on earlier heads
• the final local rerun hit a local Codex client transport failure against http://127.0.0.1:8787/responses, so I also triggered PR-side AI review
• tcx pr review --pr 212 --repo textcortex/spritz --watch posted the prompt on the latest head but timed out without Codex/Claude replies, so there are no open bot findings to address on the PR itself

PR: #212

[gitrank-connector]

⭐ GitRank PR Analysis

Score: 50 points

Metric	Value
Component	Other (1× multiplier)
Severity	P1 - High (50 base pts)
Final Score	50 × 1 = 50

Eligibility Checks

Check	Status
Issue/Bug Fix	❌
Fix Implementation	✅
PR Documented	✅
Tests	✅
Lines Within Limit	✅

Impact Summary

This PR transforms spz port-forward from an SSH-only feature into a control-plane feature with WebSocket as the preferred transport over HTTPS/443, while maintaining SSH as an automatic fallback. It adds a new API endpoint (GET /api/spritzes/:name/port-forward), implements bidirectional WebSocket-to-TCP proxying with EOF handling, and updates the CLI to prefer WebSocket with graceful degradation to SSH. The change improves security by routing traffic through authenticated control plane rather than raw SSH.

Analysis Details

Component Classification: This PR implements a new feature (port-forward over WebSocket) rather than fixing a specific categorized component. It spans API, CLI, and infrastructure layers without fitting neatly into predefined categories.

Severity Justification: This is a high-impact feature addition that introduces a new authenticated transport mechanism for port forwarding, changes default behavior from SSH to WebSocket, and includes comprehensive fallback logic. The scope (1566 lines across 10 files) and architectural significance warrant P1 classification.

Eligibility Notes: Tests are required and present: 332 lines of API tests covering WebSocket lifecycle, EOF framing, dropped connections, and 402 lines of CLI tests covering WebSocket proxying, EOF preservation, tunnel drops, and SSH fallback. The PR is well-documented with architecture decision document. No specific issue is referenced, but this is a planned feature implementation. Fix_implementation is true as code changes align with the feature description.

---

Analyzed by GitRank 🤖