Skip to content

docs: add privileged conversation debug architecture#152

Merged
onutc merged 16 commits intomainfrom
codex/privileged-conversation-debug-architecture
Mar 24, 2026
Merged

docs: add privileged conversation debug architecture#152
onutc merged 16 commits intomainfrom
codex/privileged-conversation-debug-architecture

Conversation

@onutc
Copy link
Copy Markdown
Member

@onutc onutc commented Mar 24, 2026

TL;DR

This adds an architecture doc for the long-term privileged conversation debug and test path in Spritz. It defines a control-plane-owned broker model instead of direct runtime access from engineer laptops or ad hoc scripts.

Summary

  • add a new architecture doc for privileged conversation debugging and testing
  • define the target broker, authz, audit, debug-session, and spz CLI model
  • link the doc from the README key-docs section

Review focus

  • whether the control-plane broker model is the right long-term direction
  • whether the authz and break-glass sections are scoped correctly
  • whether any existing ACP or conversation docs should be cross-linked further

Test plan

  • npx -y @simpledoc/simpledoc check
  • git diff --check

@onutc
Copy link
Copy Markdown
Member Author

onutc commented Mar 24, 2026

Final implementation report for e8b809c.

What changed:

  • Added the owner-scoped internal debug chat endpoint at POST /api/internal/v1/debug/chat/send.
  • Added spz chat send for instance- and conversation-targeted headless messaging.
  • Hardened the route so it requires both authenticated caller identity and a dedicated internal token header.
  • Kept the flow owner-scoped even for admin principals.
  • Added ACP prompt/cancel/permission handling, same-socket bootstrap+prompt flow, and cleanup-on-failure behavior.
  • Added regression coverage for stale-session repair, prompt cancellation, permission rejection, keepalive settling, and strict internal-header auth.
  • Kept the new architecture doc repo-agnostic by removing org-specific author metadata.

Validation run locally:

  • go test ./... in api: passed
  • pnpm --dir cli test: passed
  • pnpm --dir cli exec tsc -p tsconfig.json --noEmit: passed
  • git diff --check: passed
  • npx -y @simpledoc/simpledoc check: passed
  • codex review --base main: no remaining P0/P1 findings on the latest head

PR state:

  • No PR issue comments or review comments were present after the latest push.
  • Latest GitHub checks on e8b809c are green: cli-tests, docs-check, and all go-tests jobs.

Residual risk:

  • The last Codex pass still noted P2-level concerns around very long session/load replay windows versus the synchronous debug-chat request model. Those are not release-blocking for this PR, but they are the main remaining edge case if we want to harden this further.

@onutc
Copy link
Copy Markdown
Member Author

onutc commented Mar 24, 2026

Final report for 82028b6.

What changed:

  • Removed the optimistic local user-message insert from the chat page so ACP is the only writer of durable transcript messages.
  • Added a regression test that verifies an echoed user_message_chunk renders exactly one user bubble.
  • Documented the transcript-ownership invariant in the ACP chat architecture doc and added code comments near the relevant UI paths.

Validation run locally:

  • pnpm --dir ui test -- chat.test.tsx: passed
  • pnpm --dir ui typecheck: passed
  • pnpm --dir ui build: passed
  • git diff --check: passed
  • npx -y @simpledoc/simpledoc check: passed
  • codex review --base main: no remaining P0/P1 findings on the latest head

PR state:

  • No inline review comments are present on the latest head.
  • The only Codex findings from the full-branch review are two pre-existing P2 notes in the older internal debug-chat websocket flow.
  • Latest GitHub checks on 82028b6 are green.

Residual risk:

  • The duplicate-message UI bug is fixed by making ACP authoritative for durable transcript entries.
  • The remaining P2 review notes are about long replay / websocket-close edge cases in the internal debug-chat implementation and are not blockers for this UI fix.

@onutc onutc merged commit 197d6e2 into main Mar 24, 2026
6 checks passed
@onutc onutc deleted the codex/privileged-conversation-debug-architecture branch March 24, 2026 21:39
@gitrank-connector
Copy link
Copy Markdown

👍 GitRank PR Analysis

Score: 5 points

Metric Value
Component Other (1× multiplier)
Severity P3 - Low (5 base pts)
Final Score 5 × 1 = 5

Eligibility Checks

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

Impact Summary

This PR adds a comprehensive internal debug chat API endpoint and CLI command that allows authorized users to send messages to Spritz conversations through a control-plane broker model. It includes 3172 lines of changes across API handlers, CLI commands, comprehensive tests, and architecture documentation. The feature implements proper authorization, audit logging, and timeout handling for privileged conversation access.

Analysis Details

Component Classification: This PR introduces a new feature (privileged conversation debug API and CLI) rather than fixing a specific component. It spans multiple systems (API, CLI, documentation) making it best categorized as OTHER.

Severity Justification: This is a new feature addition for internal debugging/testing workflows, not a bug fix or critical functionality. It enhances developer experience but does not address a broken system or security vulnerability, warranting P3 (Low) severity.

Eligibility Notes: Tests are required and included: 852 lines of test code in internal_debug_chat_test.go and 360 lines in cli/test/chat-send.test.ts covering multiple scenarios (creation, existing conversations, auth failures, timeouts, permission handling). The PR is well-documented with clear description and architecture docs. This is a new feature requiring tests, which are comprehensively provided.


Analyzed by GitRank 🤖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant