Skip to content

docs: frontend audit round-2 follow-up & validations (tracks #2575)#2577

Open
rmusser01 wants to merge 3 commits into
devfrom
feat/frontend-audit-round2-followup
Open

docs: frontend audit round-2 follow-up & validations (tracks #2575)#2577
rmusser01 wants to merge 3 commits into
devfrom
feat/frontend-audit-round2-followup

Conversation

@rmusser01

@rmusser01 rmusser01 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

What this is

Follow-up tracker for the frontend/extension audit remediation that landed in #2575 (merged to dev). This PR does not change runtime behavior — it adds a checklist (apps/FRONTEND_AUDIT_FOLLOWUP.md) that makes explicit:

  1. Validations still required — changes that shipped in fix(frontend): remediate frontend + extension audit findings #2575 but that could not be verified without a running app/backend, and
  2. Continued work — the follow-up tickets that were deliberately deferred, each with its reason.

Full details in the doc; the highlights:

⚠️ Needs live validation before these features are trusted on dev

  • WS auth moved out of the URL (R3 / task-12113): persona/voice/STT now send the token via the WebSocket subprotocol or a {type:"auth"} first message instead of the URL query string. Unit-tested + charset-fallback-guarded, but the handshake/timing were not validated against a running server. Smoke-test persona live, voice chat, streaming transcription, and extension STT (and confirm no token= in the WS URL). This is the single change most likely to break a user-facing feature — validate or revert first.
  • CSP dropped 'unsafe-inline' (H1 / task-12093): quick browser console check on the main routes + the dev error overlay to confirm nothing legit is blocked and the theme applies without a flash.

Continued work (deferred, with reasons — see the doc's table)

  • task-12102 — TS-strict migration (blocked on ~66 pre-existing type errors; a real cleanup, not a flag flip).
  • task-12108 — consolidate the 3 characterChatMode copies (the watchdog already shipped; full merge is a big refactor while chat is churning).
  • H1 follow-up — drop CSP 'unsafe-eval' (needs WASM/OCR browser verification); optional HSTS/report-uri/build-time hash.
  • task-12103 — remove the extension/routes mirror (must migrate ~22 parity tests first).

Not introduced here

One pre-existing usePersonaLiveControl test failure predates all of this (confirmed on the base).


Suggested order: do the voice/STT smoke test first, then decide whether to keep or revert R3 before tackling the larger deferred items.

🤖 Generated with Claude Code


Summary by cubic

Adds a checklist doc for the round-2 frontend audit follow-up. It lists the live validations (WS auth out of URL; CSP without 'unsafe-inline'), tracks deferred work (task-12113, task-12116, task-12108, task-12103), and updates all docs to reflect the TS-strict ticket renumbering to task-12116; no runtime changes.

Written for commit 5ec7168. Summary will update on new commits.

Review in cubic

Tracking doc for the work following the merged audit remediation (PR #2575):
the validations that need a live app/backend (WS token-out-of-URL smoke test;
CSP browser check) and the deferred follow-up tickets (TS-strict migration,
characterChatMode consolidation, CSP unsafe-eval, extension/routes removal).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: aa491990-166d-45d0-a03f-15fcc99f97ba

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/frontend-audit-round2-followup

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

docs: frontend audit round-2 follow-up validation checklist

📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Add a follow-up checklist for audit remediation merged in PR #2575.
• Call out live validations needed before trusting WS auth and CSP changes on dev.
• Track deferred follow-up tickets with rationale and known pre-existing failures.
High-Level Assessment

The following are alternative approaches to this PR:

1. Convert checklist into GitHub issues (one per validation/deferred item)
  • ➕ Assignability/ownership and due dates
  • ➕ Better progress tracking and notifications
  • ➕ Easier linking to follow-up PRs
  • ➖ More process overhead to create/maintain
  • ➖ Loses the single-page narrative/context and audit-specific ordering
2. Add the validation checklist to PR #2575 or the original audit doc only
  • ➕ Keeps audit context consolidated in one canonical document
  • ➕ Reduces the number of separate docs to discover
  • ➖ PR comments can be harder to find later
  • ➖ Audit doc may become too long and less action-oriented

Recommendation: Keep this PR’s approach (a dedicated follow-up checklist doc) because it’s low-overhead, action-oriented, and explicitly separates “must validate live” from “deferred work.” Consider additionally linking this doc from the main audit doc index/TOC (and optionally mirroring the highest-risk V1 validation as a GitHub issue) to improve discoverability and ownership.

Files changed (1) +76 / -0

Documentation (1) +76 / -0
FRONTEND_AUDIT_FOLLOWUP.mdAdd round-2 audit follow-up checklist and live validation plan +76/-0

Add round-2 audit follow-up checklist and live validation plan

• Introduces a tracking document for post-#2575 work, enumerating required live smoke tests (WS auth token-out-of-URL, CSP without unsafe-inline) and listing deferred follow-up tickets with deferral rationale. Also documents known pre-existing test failures not attributed to the audit remediation.

apps/FRONTEND_AUDIT_FOLLOWUP.md

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a tracking document (apps/FRONTEND_AUDIT_FOLLOWUP.md) for the validation and continued work following a frontend audit. The review feedback points out inconsistent file paths listed in the document and suggests standardizing them to be relative to the repository root.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +35 to +38
Files if a revert/patch is needed: `apps/packages/ui/src/services/persona-stream.ts`,
`services/tldw/voice-conversation.ts`, `hooks/useVoiceChatStream.tsx`,
`routes/hooks/usePersonaLiveSession.tsx`, `hooks/usePersonaLiveControl.tsx`,
`entries/background.ts` (STT section).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The file paths listed here are inconsistent. The first path (apps/packages/ui/src/services/persona-stream.ts) is relative to the repository root, whereas the subsequent paths (e.g., services/tldw/voice-conversation.ts, hooks/useVoiceChatStream.tsx) omit the apps/packages/ui/src/ prefix.\n\nTo avoid confusion and ensure consistency, all paths should be specified relative to the repository root:\n\nmarkdown\nFiles if a revert/patch is needed: `apps/packages/ui/src/services/persona-stream.ts`,\n`apps/packages/ui/src/services/tldw/voice-conversation.ts`, `apps/packages/ui/src/hooks/useVoiceChatStream.tsx`,\n`apps/packages/ui/src/routes/hooks/usePersonaLiveSession.tsx`, `apps/packages/ui/src/hooks/usePersonaLiveControl.tsx`,\n`apps/packages/ui/src/entries/background.ts` (STT section).\n

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 74 rules

Grey Divider


Remediation recommended

1. Wrong/ambiguous file paths 🐞 Bug ⚙ Maintainability
Description
The “Files if a revert/patch is needed” list contains truncated paths (missing
apps/packages/ui/src/…) and an ambiguous entries/background.ts, which can send someone to the
wrong file when reverting/patching the WS-auth changes.
Code

apps/FRONTEND_AUDIT_FOLLOWUP.md[R35-38]

+Files if a revert/patch is needed: `apps/packages/ui/src/services/persona-stream.ts`,
+`services/tldw/voice-conversation.ts`, `hooks/useVoiceChatStream.tsx`,
+`routes/hooks/usePersonaLiveSession.tsx`, `hooks/usePersonaLiveControl.tsx`,
+`entries/background.ts` (STT section).
Evidence
The doc lists shortened paths and a generic entries/background.ts, but the actual files in this
branch are under apps/packages/ui/src/..., and there are two different background entrypoints,
making the doc’s reference ambiguous.

apps/FRONTEND_AUDIT_FOLLOWUP.md[35-38]
apps/packages/ui/src/services/tldw/voice-conversation.ts[1-6]
apps/packages/ui/src/hooks/useVoiceChatStream.tsx[1-13]
apps/packages/ui/src/routes/hooks/usePersonaLiveSession.tsx[1-10]
apps/packages/ui/src/hooks/usePersonaLiveControl.tsx[1-16]
apps/extension/entrypoints/background.ts[1-1]
apps/packages/ui/src/entries/background.ts[1-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The follow-up doc’s revert/patch file list includes incorrect/truncated paths and an ambiguous `entries/background.ts`. This undermines the doc’s main purpose (fast validation/revert guidance) because engineers may waste time searching or patching the wrong location.

### Issue Context
In this repo, the referenced hooks/services live under `apps/packages/ui/src/...`, and there are multiple `background.ts` entrypoints (extension entrypoint vs UI implementation).

### Fix Focus Areas
- apps/FRONTEND_AUDIT_FOLLOWUP.md[35-38]

### Suggested change
Update the list to fully-qualified repo paths and disambiguate background entrypoints, e.g.:
- `apps/packages/ui/src/services/persona-stream.ts`
- `apps/packages/ui/src/services/tldw/voice-conversation.ts`
- `apps/packages/ui/src/hooks/useVoiceChatStream.tsx`
- `apps/packages/ui/src/routes/hooks/usePersonaLiveSession.tsx`
- `apps/packages/ui/src/hooks/usePersonaLiveControl.tsx`
- `apps/packages/ui/src/entries/background.ts` (extension re-exports via `apps/extension/entrypoints/background.ts`)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +35 to +38
Files if a revert/patch is needed: `apps/packages/ui/src/services/persona-stream.ts`,
`services/tldw/voice-conversation.ts`, `hooks/useVoiceChatStream.tsx`,
`routes/hooks/usePersonaLiveSession.tsx`, `hooks/usePersonaLiveControl.tsx`,
`entries/background.ts` (STT section).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Wrong/ambiguous file paths 🐞 Bug ⚙ Maintainability

The “Files if a revert/patch is needed” list contains truncated paths (missing
apps/packages/ui/src/…) and an ambiguous entries/background.ts, which can send someone to the
wrong file when reverting/patching the WS-auth changes.
Agent Prompt
### Issue description
The follow-up doc’s revert/patch file list includes incorrect/truncated paths and an ambiguous `entries/background.ts`. This undermines the doc’s main purpose (fast validation/revert guidance) because engineers may waste time searching or patching the wrong location.

### Issue Context
In this repo, the referenced hooks/services live under `apps/packages/ui/src/...`, and there are multiple `background.ts` entrypoints (extension entrypoint vs UI implementation).

### Fix Focus Areas
- apps/FRONTEND_AUDIT_FOLLOWUP.md[35-38]

### Suggested change
Update the list to fully-qualified repo paths and disambiguate background entrypoints, e.g.:
- `apps/packages/ui/src/services/persona-stream.ts`
- `apps/packages/ui/src/services/tldw/voice-conversation.ts`
- `apps/packages/ui/src/hooks/useVoiceChatStream.tsx`
- `apps/packages/ui/src/routes/hooks/usePersonaLiveSession.tsx`
- `apps/packages/ui/src/hooks/usePersonaLiveControl.tsx`
- `apps/packages/ui/src/entries/background.ts` (extension re-exports via `apps/extension/entrypoints/background.ts`)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

rmusser01 and others added 2 commits July 2, 2026 18:14
…sion)

A teammate independently used task-12102 on dev (character-greeting selection),
colliding with the audit's TS-strict ticket. Renumber mine to 12116 and update
the audit-doc references; leave the teammate's 12102 untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous commit captured only the file rename; the id: field and the
FRONTEND_AUDIT / FOLLOWUP references were missed. Update them so TASK-12116 is
internally consistent and no doc still points the config ticket at 12102.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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