feat: [ENG-2706] integrate HITL into Changes tab and Configuration toggle

[ncnthien]

Summary

• Surface agent-authored curate operations directly in the WebUI Changes tab with an inline metadata panel per file (no more separate review inbox).
• Replace approve/reject buttons with implicit decisions: staging an agent file approves it, discarding rejects it (which restores from .brv/review-backups/).
• Add an Agent supervision toggle to the Configuration tab (binary enable/disable; persists via the existing review:getDisabled / review:setDisabled events).
• Sidebar gets a pulsing amber dot for high-impact pending agent files; the dot disappears once the file is staged or discarded.
• Multi-diff view is restyled as a vertical list of free-floating rounded cards (one per file), with the agent panel anchored at the top of each card and per-file sticky filename headers.

Server

• New transport event review:listOperations returning all completed curate-log entries (or [] when supervision is disabled). No schema migration, no field renames.
• Handler reuses the new projectContextTreeFilePath helper for relative-path projection across handleDecideTask, handleListOperations, and handlePending.

Client

• New React Query hook useGetAgentChanges (3s polling), pure joinAgentMeta utility for the working-tree × curate-log join, and splitReasonPrefix for [dream/action] → dream:action rendering.
• decidePendingFiles in ChangesPanel groups pending files by taskId and fires one review:decideTask per task in parallel; runs after the corresponding vc:add / vc:reset so a failed stage leaves review state untouched.
• New HitlSettingsPanel on the Configuration tab.

Test plan

• All new unit tests pass (review-handler-list-operations, join-agent-meta, should-show-agent-pulse, split-reason-prefix).
• Manual QA per ENG-2809 — 12 cases covering low/high-impact agent edits, implicit approve/reject, multi-file view, Configuration toggle round-trip, CLI/WebUI state sync.
• Lint and typecheck clean (npm run lint, npm run typecheck).
• Manual smoke: with supervision on, run `brv curate` against a project; verify Changes tab surfaces the operation with the expected badges and dot.

Closes ENG-2706

[github-actions]

Claude finished @ncnthien's task in 4m 3s —— View job

---

PR review — HITL into Changes tab and Configuration toggle

Overall: 4 / 5 — clean architecture, good test coverage on the new utilities/handler, but one correctness issue contradicts a behavior claim in the PR description.

• Read PR context and changed file list
• Reviewed server-side changes (review-handler, transport events)
• Reviewed client-side changes (changes-panel, new API hooks, components)
• Reviewed new utilities (join-agent-meta, split-reason-prefix, should-show-agent-pulse)
• Reviewed tests for correctness and coverage
• Posted inline comments for specific issues

Summary

The change cleanly folds HITL into the existing Changes tab without a separate review inbox — the "implicit decision via stage/discard" model is intuitive and the per-file FileMetaPanel (op type + impact + reason) is well-scoped. The new review:listOperations event and joinAgentMeta/splitReasonPrefix/shouldShowAgentPulse utilities are small, pure, and well-tested. The HitlSettingsPanel round-trips through the existing review:getDisabled/setDisabled events without schema churn.

Issues flagged (see inline comments)

Correctness

• 🐞 changes-panel.tsx:136–149 — handleStageFile / handleStageAll wrap vc:add in runAction, which swallows errors. decidePendingFiles('approved') then runs unconditionally, so a failed stage still marks the agent op approved. This directly contradicts the PR description's "a failed stage leaves review state untouched" claim.
• ⚠️ review-handler.ts:249 — limit: 200 silently caps which curate-log entries reach the Changes tab; older agent metadata on still-modified files just stops appearing with no signal. Worth documenting or replacing with a per-path dedupe at the store level.

Consistency / UX

• changes-panel.tsx:263–278 — useReviewDecideTask doesn't invalidate vc:status, so rejecting a pending agent file leaves a stale entry in the Changes panel until the next 3 s poll.
• review-handler.ts:238–273 — handleListOperations drops previousSummary while handlePending keeps it. Pick one and apply consistently.

Design

• types.ts:22–32 — getEffectiveImpact treats reviewStatus === 'pending' as 'high' impact, conflating two orthogonal axes (impact vs. review status). Recommend separating the pulse-dot heuristic from impact derivation.
• file-meta-panel.tsx:56–86 — isHigh and isHighPending are redundant given the above coupling; reads as accidentally working.

Performance

• get-agent-changes.ts:19 — 3 s polling alongside vc:status doubles the request rate per project. Consider push-driven invalidation via the existing ReviewEvents.NOTIFY event, or scope the query key by selectedProject.

Style / Tests

• split-reason-prefix.ts — action regex [^\]]+ is greedy and silently accepts [a/b/c] as prefix=a:b/c. Tighten or document.
• review-handler-list-operations.test.ts — solid coverage; a follow-up test for multiple ops on the same file across entries would lock the flatten-and-let-client-dedup contract.

Test plan results

• Did not run npm test / npm run typecheck in this review pass — recommend running before merge.

Strengths

• ✅ Pure utilities with focused unit tests (joinAgentMeta, splitReasonPrefix, shouldShowAgentPulse).
• ✅ Server handler isolates per-file failures with Promise.allSettled, batches log updates by logId, and notifies onResolved best-effort.
• ✅ projectContextTreeFilePath helper centralizes the absolute→relative projection across three handlers.
• ✅ getEffectiveImpact keeps DELETE always-high (matches existing CLI behavior).
• ✅ TUI/server import boundary preserved; no webui/ → server/ cross-imports.
  · feat/ENG-2706

[github-actions]

Claude finished @ncnthien's task in 3m 39s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (follow-up pass)

Overall: 4.5 / 5 — the follow-up commit cleanly addresses every blocker and correctness/consistency issue from the prior review. Remaining open items are all polish/follow-ups, not merge-blockers.

• Read PR diff vs origin/main and confirm what changed since last review
• Re-check the flagged correctness bug in changes-panel.tsx (failed stage should leave review state untouched)
• Re-check review-handler.ts (limit:200, previousSummary asymmetry)
• Re-check types.ts / file-meta-panel.tsx impact-vs-pending coupling
• Re-check get-agent-changes.ts polling + query key scoping
• Re-check split-reason-prefix.ts regex
• Re-check useReviewDecideTask invalidation of vc:status
• Review new tests added
• Post inline comments and a summary with score (0–5)

What this round fixed (from the previous review)

Prior issue	Status	Where
🐞 Failed vc:add still marked agent op approved	✅ Fixed via try/catch + early return	changes-panel.tsx:136-145, 150-161
useReviewDecideTask didn't invalidate vc:status on reject	✅ Fixed	execute-review-decide-task.ts:32
getEffectiveImpact conflated pending with high	✅ Decoupled; pulse heuristic moved to shouldShowAgentPulse	types.ts:22-26, file-meta-panel.tsx, should-show-agent-pulse.ts
split-reason-prefix regex accepted nested slashes ([a/b/c])	✅ Tightened to [^/\]]+ with a new test	split-reason-prefix.ts:1, test at split-reason-prefix.test.ts:41-44

Still open (carryover, follow-up-worthy, not blocking)

• ⚠️ review-handler.ts:249 — limit: 200 still uncommented / unchanged. Worth a one-line comment or removing the cap and relying on client-side dedup. (inline)
• ⚠️ review-handler.ts:273 vs handlePending:300 — previousSummary asymmetry persists. Defensible since the Changes tab gets diff content via vc:diff, but worth a clarifying comment. (inline)
• ⚠️ get-agent-changes.ts:19 — 3 s polling alongside vc:status polling. Mutation-side invalidation now covers snappy UX, so this could be replaced with ReviewEvents.NOTIFY subscription (the daemon already emits it; mirror src/tui/features/tasks/hooks/use-task-subscriptions.ts:112). (inline)
• 🧪 review-handler-list-operations.test.ts — covers flatten across entries on different paths; missing the explicit "same file across multiple entries" test that locks in the flatten-and-let-client-dedup contract. (inline)

Strengths

• ✅ Targeted follow-up commit — only touches the 6 files needed for the review feedback. No drive-by changes.
• ✅ Pulse-dot heuristic now lives in a dedicated, well-tested utility — reads much better at the call sites.
• ✅ Regex fix is paired with a regression test (does not match prefixes with nested slashes in the action segment).
• ✅ try/catch rewrite of the stage handlers reads cleanly; PR description claim and code now agree.
• ✅ Import boundary preserved; no webui/ → server/ cross-imports.
• ✅ TUI/server NOTIFY subscriber pattern is available to swap in later for polling.

Test plan results

• Did not run npm test / npm run typecheck in this pass — recommend running locally before merge (PR description's checkboxes are still unchecked).

Verdict

4.5 / 5 — ready to merge once lint/typecheck are clean and the manual QA from ENG-2809 is run. The four open items above are reasonable follow-ups (perf polish + handler doc + one extra test), none are correctness-critical.
· feat/ENG-2706

[github-actions]

Claude finished @ncnthien's task in 3m 50s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (round 3)

Overall: 4 / 5 — the deep-link migration is well-scoped and adequately tested at the unit level, but ProjectGuard has a real cold-start race that defeats the auto-select feature for the exact flow it was built for. Fixable in <10 lines.

• Identify what changed since the last review (commit 131dc8d)
• Review the WebUI auto-select project flow (project-guard.tsx, resolve-auto-select-project.ts)
• Review the deleted review HTTP handler / new transport routing
• Review new base64url utilities (server + webui)
• Verify previous-round feedback still holds
• Post review summary with score

What changed since the last pass (131dc8d)

• Deleted src/server/infra/http/review-api-handler.ts + review-ui.ts (~1 KLoC) and their test file. The standalone /review HTTP endpoint is gone — review now uses the WebUI Changes tab end-to-end.
• New buildReviewUrl(port, projectPath) + symmetric base64url encoders (server: Buffer, browser: TextEncoder+btoa). Cross-encoder agreement is asserted in tests.
• ReviewEvents.NOTIFY.reviewUrl and pendingReview.reviewUrl are now optional (only emitted when WebUI is running).
• ProjectGuard reads ?project=<base64url> and auto-selects via the new resolveAutoSelectProject resolver.

---

Issues flagged

🐞 Correctness — project-guard.tsx:36-46 cold-start race wipes the deep-link param before it's applied

The effect unconditionally deletes ?project=... whenever urlParam is truthy, regardless of whether projects has loaded yet. useGetProjectList is gated on isConnected (see project-guard.tsx:19-23), so on a cold tab open the timeline is:

1. Render 1: isConnected=false, projects=undefined, urlParam="ABC..." → candidate=undefined (line 27–34 short-circuits on projects).
2. Effect commits: candidate is falsy so no setSelectedProject; the if (urlParam) branch still fires and clears the URL.
3. Eventually projects load — but urlParam is already undefined. resolveAutoSelectProject falls through to projectCwd (or, with a persisted selectedProject from Zustand partialize at transport-store.ts:91, returns undefined and the persisted selection wins).

Net effect: clicking the CLI-printed …/changes?project=XYZ link from a fresh tab lands the user on the persisted/projectCwd project, not XYZ — exactly the flow this commit is meant to enable. The unit tests for resolveAutoSelectProject don't catch this because they exercise the resolver in isolation, not the guard's effect timing.

Minimal fix — only clear once the resolver has actually had a chance to run:

useEffect(() => {
  if (candidate && candidate !== selectedProject) {
    setSelectedProject(candidate)
  }
  if (urlParam && projects) {       // gate on projects being loaded
    const next = new URLSearchParams(searchParams)
    next.delete('project')
    setSearchParams(next, {replace: true})
  }
}, [candidate, projects, selectedProject, urlParam, searchParams, setSearchParams, setSelectedProject])

A regression test that mounts <ProjectGuard> with isConnected=false and a URL param, then flips to isConnected=true with a matching project, would lock this in. Fix this →

🐞 Regression — use-activity-logs.ts:236 will render review: undefined

content += `\n\n⚠ ${pendingCount} ${fileLabel} review: ${reviewUrl}`

reviewUrl is now optional (ReviewNotification.reviewUrl?: string, tasks-store.ts:28). The two other consumers (push-flow.tsx:48, format-status.ts:116) guard with reviewUrl ? but this one doesn't. When the daemon emits a notify with no WebUI running, the TUI activity log will display the literal string review: undefined. Either guard the same way, or drop the URL fragment when missing:

content += `\n\n⚠ ${pendingCount} ${fileLabel} review` + (reviewUrl ? `: ${reviewUrl}` : '')

Fix this →

⚠️ Nit — localhost vs 127.0.0.1 mismatch

buildReviewUrl (build-review-url.ts:4) uses http://localhost:..., but the WebUI server binds to TRANSPORT_HOST = '127.0.0.1' (src/server/constants.ts:43, IPv4-only — comment says "Use IP address for better sandbox compatibility"). On dual-stack systems where localhost resolves to ::1 first, browsers will retry over IPv4 (gracefully), but in sandboxed/Docker contexts that lack the fallback this can hang. The previous code used http://127.0.0.1:... for exactly this reason. Suggest staying on 127.0.0.1 to match the bind address.

⚠️ Minor — missing searchParams in the effect dep array

project-guard.tsx:46 lists setSearchParams but not searchParams, even though new URLSearchParams(searchParams) reads from it inside the effect. Functionally OK today because setSearchParams triggers a re-render that re-runs the effect with the fresh closure, but it'll trip react-hooks/exhaustive-deps if the rule is enabled here.

---

Carryovers from prior rounds (still open, still non-blocking)

• review-handler.ts:249 — limit: 200 cap kept un-commented (per author's no-comment policy + the existing review threads as documentation).
• get-agent-changes.ts:19 — 3 s polling kept to match useGetVcStatus; push-driven invalidation via ReviewEvents.NOTIFY tracked as follow-up.
• review-handler.ts:273 — previousSummary projection asymmetry intentional (CLI vs WebUI consumers).
• review-handler-list-operations.test.ts — "same file across multiple entries" lock-in test deferred.

All four are reasonable to address in a follow-up PR.

---

Strengths

• ✅ Clean deletion: removing review-api-handler.ts + review-ui.ts + tests is ~1.6 KLoC of net reduction. No dead branches left.
• ✅ Symmetric base64url encoders, with a cross-encoder agreement test (base64url.test.ts) that includes CJK / Cyrillic / emoji / reserved URL chars / Windows backslashes / UNC paths.
• ✅ resolveAutoSelectProject precedence is sensible (URL > existing selection > cwd) and the resolver is correctly defensive: try/catch around decodeBase64Url, allowlist check against knownPaths. Tampered URLs can't push the user to an unknown project.
• ✅ ReviewEvents.NOTIFY.reviewUrl made optional with a clear inline comment on the schema (review-events.ts:23-24) — good signal-to-noise.
• ✅ push-handler.ts / status-handler.ts simplification removes the buggy fallback to transport.getPort() (the transport port never served the review UI).
• ✅ feature-handlers.ts:241 correctly drops the reviewUrl: '' placeholder from the resolved-broadcast payload now that the field is optional.

Test plan

• Did not run npm test / npm run typecheck in this pass. Recommend running both before merge, and adding the cold-start regression test described above.

---

Verdict

4 / 5 — one real bug in the new deep-link path (cold-start race wipes the URL param) and one TUI display regression (review: undefined). Both are small fixes. Once those land — plus the optional localhost/127.0.0.1 consistency tweak — this is merge-ready.
· feat/ENG-2706

[github-actions]

Claude finished @ncnthien's task in 1m 31s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (round 4)

Overall: 4.5 / 5 — both real bugs from round 3 are cleanly fixed. Two nits and a missing regression test carry over; none are merge-blockers.

• Identify what changed since last review (commit d8f51b7)
• Re-check project-guard.tsx cold-start race
• Re-check use-activity-logs.ts:236 reviewUrl: undefined
• Check buildReviewUrl host (localhost vs 127.0.0.1)
• Spot-check tests added this round
• Post final summary with score

What this round fixed (from the previous review)

Prior issue	Status	Where
🐞 Cold-start race wiped ?project= before resolver ran	✅ Fixed via if (!projects) return guard + projects in dep array	project-guard.tsx:36-48
🐞 TUI activity log rendered literal review: undefined when reviewUrl absent	✅ Fixed with ternary guard, matches push-flow.tsx:48 / format-status.ts:116 pattern	use-activity-logs.ts:236

The fix on project-guard.tsx is correct. Because if (urlParam) return null (line 50) holds the render output to null until the URL param is consumed, the effect commits across renders and only triggers setSelectedProject / setSearchParams once projects is non-undefined — the cold-start deep-link path now reaches resolveAutoSelectProject with the URL param intact.

Still open (carryover, non-blocking)

• ⚠️ localhost vs 127.0.0.1 in build-review-url.ts:4 — buildReviewUrl still emits http://localhost:${port}, but the WebUI server binds to TRANSPORT_HOST = '127.0.0.1' (src/server/constants.ts:43, comment notes "Use IP address for better sandbox compatibility"). On dual-stack hosts where localhost resolves to ::1 first, browsers fall back gracefully, but sandboxed/Docker contexts without IPv6 can hang. Trivial one-character fix. Fix this →
• ⚠️ Missing searchParams in effect dep array (project-guard.tsx:48) — effect reads searchParams at line 44 but only lists setSearchParams in deps. Functionally OK today (every setSearchParams call mutates urlParam, which is in deps), but react-hooks/exhaustive-deps will flag it if enabled here.
• 🧪 No regression test for the cold-start race itself — resolve-auto-select-project.test.ts exercises the resolver in isolation (good coverage: URL > selection > cwd, malformed input, non-ASCII, Windows paths). The bug that was just fixed lived in the guard's effect timing, not the resolver, so it remains untested. A small RTL test that mounts <ProjectGuard> with isConnected=false + a URL param then flips isConnected=true (with a matching project) would lock the regression. Fix this →

Strengths

• ✅ Surgical commit — only the 2 files needed for the round-3 feedback. No drive-by changes.
• ✅ Activity-log fix is consistent with the two other reviewUrl consumers (push-flow.tsx:48, format-status.ts:116).
• ✅ Cold-start fix is minimal (one early return + one dep entry); no broader refactor of the guard's flow.
• ✅ Resolver-level tests cover the interesting precedence cases (URL > selection > cwd), malformed base64url, and unicode/Windows paths.

Test plan

• Did not run npm test / npm run typecheck / npm run lint in this pass. Recommend running before merge alongside the manual QA from ENG-2809.

Verdict

4.5 / 5 — round-3 bugs cleanly resolved. The remaining open items (localhost → 127.0.0.1, dep array exhaustive-deps, and a guard-level regression test) are reasonable polish/follow-ups. Once lint / typecheck are green and the host nit is decided on, this is merge-ready.
· feat/ENG-2706

[ncnthien]

Addressed both correctness findings in d8f51b73:

Finding	Fix
🐞 Cold-start race in ProjectGuard — URL strip fired before projects loaded, wiping the deep-link param before the resolver could use it	Added if (!projects) return early-out in the effect, and added projects to deps. URL stays in place until projects are loaded.
🐞 use-activity-logs.ts:236 rendering "review: undefined" when reviewUrl is optional and absent	Guarded with a ternary, mirroring push-flow.tsx and format-status.ts.

Nits deferred:

• localhost vs 127.0.0.1 — intentional. Matches the existing brv webui CLI output (line 46 of src/oclif/commands/webui.ts). The dual-stack concern is theoretical; we don't run inside --network none sandboxes, and modern browsers fall back to IPv4. Happy to revisit if we hit a real failure.
• Missing searchParams in effect deps — intentional. React Router returns a new URLSearchParams instance per render, so including it would cause the effect to fire on every render. The body only reads searchParams inside if (urlParam) and urlParam IS in the deps, so the closure is fresh whenever the branch fires. react-hooks/exhaustive-deps isn't enabled in this project's ESLint config.

Carryovers (all noted non-blocking by the bot, deferred to follow-up PRs):

• limit: 200 curate-log cap
• 3s polling on useGetAgentChanges (matches useGetVcStatus)
• previousSummary projection asymmetry (CLI vs WebUI)
• Multi-op-same-file handleListOperations test

[github-actions]

Claude finished @ncnthien's task in 1m 17s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (round 5)

Overall: 4.5 / 5 — this push is just a merge from origin/main. No PR-relevant code changed since round 4. Round-4 verdict stands.

• Identify what changed since last review (commit d8f51b7 / merge d13300a)
• Verify round-4 carryover items
• Spot-check tests
• Post review summary with score

What changed since round 4

Only the merge commit d13300a ("Merge remote-tracking branch 'origin/main' into feat/ENG-2706"), which pulled in the unrelated a03ad67 (#617, shared-rules disambiguation). Confirmed via:

$ git diff d8f51b7..HEAD --stat -- ':!src/server/infra/connectors' ':!test/unit/infra/connectors'
(empty)

So nothing in the HITL surface area was touched. The previous round's fixes are still in place:

• ✅ project-guard.tsx:36-48 — early if (!projects) return plus projects in deps; cold-start race no longer wipes ?project= before resolver runs.
• ✅ use-activity-logs.ts:236 — ternary guard (+ (reviewUrl ? \: ${reviewUrl}` : '')) matches the push-flow.tsx/format-status.tspattern; no more literalreview: undefined`.

Round-4 nits — acknowledging the author's responses

• localhost vs 127.0.0.1 — Fair defense. Confirmed brv webui (src/oclif/commands/webui.ts:46) emits http://localhost:${webuiPort} too, so the new buildReviewUrl is internally consistent with what the CLI already prints. The IPv6/dual-stack sandbox concern remains theoretical for this project's deployment surface. Accepted.
• searchParams not in effect deps — Fair defense. react-router-dom's useSearchParams() returns a fresh URLSearchParams instance every render, so including it in deps would re-fire the effect on every render. The body only reads searchParams inside if (urlParam), and urlParam is in the deps, so the closure is fresh whenever the branch fires. react-hooks/exhaustive-deps isn't enabled in this repo's ESLint config, so no lint pressure either. Accepted.

Carryovers (acknowledged as follow-up scope)

All four are reasonable to defer:

• review-handler.ts:249 — limit: 200 cap, kept uncommented per the no-comments policy.
• get-agent-changes.ts:19 — 3 s polling matches useGetVcStatus; push-driven invalidation via ReviewEvents.NOTIFY is tracked as follow-up.
• review-handler.ts:273 — previousSummary asymmetry intentional (CLI vs. WebUI consumers).
• review-handler-list-operations.test.ts — "same file across multiple entries" lock-in test deferred to a future PR that expands handler coverage.

Test plan

Did not run npm test / npm run typecheck / npm run lint in this pass (no PR-side code changed since round 4). PR description's checkboxes are still unchecked — recommend running them locally plus the manual QA from ENG-2809 before merge.

Verdict

4.5 / 5 — round-4 carries forward. The merge is clean (no conflicts touching the HITL surface). Once lint / typecheck are green and manual QA is run, this is merge-ready.
· feat/ENG-2706

[cuongdo-byterover]

lgtm, after @ncnthien fix the UX of switch button, we can merge

[github-actions]

Claude finished @ncnthien's task in 1m 46s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (round 6)

Overall: 4.5 / 5 — round-6 commit is a one-line dependency bump (@campfirein/byterover-packages 1.0.2 → 1.0.3). No PR-relevant TypeScript / handler / WebUI logic changed. The round-4 / round-5 verdict carries forward.

• Identify what changed since last review (commit b950332)
• Inspect new diff vs. last reviewed commit
• Verify carryover items
• Post review summary with score

What changed since round 5

$ git diff d8f51b7..HEAD --stat -- ':!src/server/infra/connectors' ':!test/unit/infra/connectors'
 package-lock.json | 4 ++--
 package.json      | 2 +-

A single commit, b950332 "feat: [ENG-2706] upgrade package to 1.0.3":

• package.json — @campfirein/byterover-packages bumped from github:#1.0.2 → github:#1.0.3.
• package-lock.json — resolution updated from bbd9b88... → 991078c....

The src/server/infra/connectors/** deltas in the stat header came in via the unrelated a03ad67 merge from main (PR #617, shared-rules disambiguation) on d13300a, not from this commit. Confirmed — no HITL surface changes since round 4's fixes.

Findings on this commit

⚠️ Minor — submodule pointer not bumped alongside the package

The submodule at packages/byterover-packages still points to f42f9bd, while the npm-resolved package now points to 991078c (the 1.0.3 tag). Per CLAUDE.md (Web UI section), BRV_UI_SOURCE=submodule|package controls which source Vite reads — build:ui / dev:ui:package use npm, build:ui:submodule / dev:ui use the submodule. The two sources are therefore intentionally allowed to diverge, but if 1.0.3 carries UI-affecting changes (the feat: commit type implies it does), local dev sessions using dev:ui will render an older snapshot than what ships from build:ui.

Not a blocker for the merge, but worth bumping the submodule to the 1.0.3 tag in the same commit going forward, or at least running git submodule update --remote packages/byterover-packages and committing the pointer before release. Fix this →

🧪 Opaque to review

The actual contents of 1.0.3 aren't visible from this PR — the bot can't open the published bundle to verify nothing breaks the new HitlSettingsPanel toggle, FileMetaPanel sparkles/badges, or the multi-diff free-floating card layout. Recommend the manual QA from ENG-2809 include a smoke pass of the Changes tab and the Configuration tab after this bump.

Carryovers (all acknowledged as follow-up scope)

These remain open from round 1 / round 3, all reasonable to defer:

• review-handler.ts:249 — limit: 200 cap (kept un-commented per no-comments policy; thread documents the rationale).
• get-agent-changes.ts:19 — 3 s polling matches useGetVcStatus; push-driven invalidation via ReviewEvents.NOTIFY is tracked as follow-up.
• review-handler.ts:273 — previousSummary projection asymmetry intentional (CLI vs WebUI consumers).
• review-handler-list-operations.test.ts — "same file across multiple entries" lock-in test deferred.
• build-review-url.ts:4 — localhost vs 127.0.0.1 defended (matches existing brv webui output, dual-stack concern theoretical).
• project-guard.tsx:48 — searchParams not in effect deps; defended on instance-identity grounds.

Verdict

4.5 / 5 — round-5 carries forward unchanged. Only outstanding nit on this commit is the submodule pointer not tracking the bumped package version. Once lint/typecheck/npm test are green and the ENG-2809 manual QA is run (ideally against the new 1.0.3 bundle), this is merge-ready. Also acknowledging @cuongdo-byterover's earlier review note: the UX of the supervision switch button still needs the requested polish before merging per their thread.
· feat/ENG-2706

[github-actions]

Claude finished @ncnthien's task in 2m 26s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (round 7)

Overall: 4.5 / 5 — round 7 is a pure merge from main (no PR-side commits). The HITL surface is untouched since round 6 and round-4's cold-start fix is intact. Round-6 verdict carries forward unchanged.

• Identify what changed since round 6 (commit b950332)
• Re-check carryover items (limit 200, 3s polling, previousSummary, localhost vs 127.0.0.1, submodule pointer)
• Look at any newly merged code from main that intersects the HITL surface
• Spot-check tests
• Post final summary with score

What changed since round 6

Only the two merge commits from main (bad0fa3, fbc9535) landing the new billing/team work (PR #661, Release/3.13.0):

$ git log b950332..HEAD --oneline --no-merges -- \
    ':!src/server/infra/billing' ':!test/unit/infra/billing' \
    ':!src/oclif/lib/billing-line.ts' ':!src/oclif/lib/format-billing-line.ts' \
    ':!src/oclif/lib/insufficient-credits.ts' ':!src/webui/features/provider' \
    ':!test/commands/providers' ':!src/server/templates'
bad0fa3 feat: [ENG-2706] merge origin/main into feat/ENG-2706

(the lone result is the merge commit itself — every other delta belongs to the billing/team feature inherited from main).

$ git diff b950332..HEAD -- \
    src/server/infra/transport/handlers/review-handler.ts \
    src/server/utils/build-review-url.ts \
    src/webui/features/vc \
    src/tui/features/activity/hooks/use-activity-logs.ts \
    src/tui/features/tasks/stores/tasks-store.ts
(empty)

So zero changes to the HITL-specific surface. The only project-guard.tsx delta vs round-6 HEAD is + <ProjectAssociationInitializer /> (project-guard.tsx:56), pulled in by the merge alongside the new src/webui/features/project/components/project-association-initializer.tsx. That component is billing/team work (associates the selected project with the user's pinned team for usage tracking) and doesn't intersect with the HITL surface. Verified the cold-start fix from round 4 is intact at project-guard.tsx:37-49 (if (!projects) return early-out + projects in deps).

Findings on this round

Nothing new. No HITL-related code changed; no merge conflicts visibly resolved into the HITL surface; the deep-link guard still passes the round-4 inspection.

Carryovers (all acknowledged as follow-up scope by the author)

Unchanged from prior rounds; all reasonable to defer:

• review-handler.ts:249 — limit: 200 cap; defended (intentional bounded snapshot, joinAgentMeta dedups per path).
• get-agent-changes.ts:19 — 3 s polling matches useGetVcStatus; push-driven invalidation via ReviewEvents.NOTIFY tracked as follow-up.
• review-handler.ts:273 — previousSummary projection asymmetry intentional (CLI brv review pending vs WebUI inline panel).
• review-handler-list-operations.test.ts — "same file across multiple entries" lock-in test deferred.
• build-review-url.ts:4 — localhost vs 127.0.0.1; defended (matches existing brv webui CLI output).
• project-guard.tsx:49 — searchParams not in effect deps; defended on instance-identity grounds.
• 🧪 Submodule pointer drift (round 6) — packages/byterover-packages still points to f42f9bd while package.json now resolves @campfirein/byterover-packages#1.0.3 (991078c). BRV_UI_SOURCE=submodule|package flows use different snapshots in local dev. Worth bumping in a follow-up if 1.0.3 carries UI-affecting changes. Fix this →

Outstanding pre-merge items (from prior rounds, not raised by this round)

• 👁️ @cuongdo-byterover flagged earlier that the UX of the supervision switch button still needs the requested polish before merging (round 6 thread).
• ✅ lint / typecheck / npm test — PR description's checkboxes are still unchecked. Recommend running them locally before merge along with the manual QA from ENG-2809.

Strengths (unchanged from prior rounds)

• ✅ Pure utilities with focused unit tests (joinAgentMeta, splitReasonPrefix, shouldShowAgentPulse).
• ✅ Symmetric base64url encoders with cross-encoder agreement test (CJK / Cyrillic / emoji / Windows paths).
• ✅ resolveAutoSelectProject precedence (URL > existing selection > cwd) with allowlist defence against tampered URLs.
• ✅ TUI/server import boundary preserved; no webui/ → server/ cross-imports.
• ✅ Cold-start deep-link race fix from round 4 remains intact post-merge.

Verdict

4.5 / 5 — no PR-side commits this round; the merge from main is clean and doesn't touch the HITL surface. Once the supervision-switch UX polish lands (per @cuongdo-byterover's earlier thread), lint/typecheck/npm test are green, and the ENG-2809 manual QA is run, this is merge-ready.
· feat/ENG-2706

[github-actions]

Claude finished @ncnthien's task in 1m 46s —— View job

---

PR review — HITL into Changes tab and Configuration toggle (round 8)

Overall: 4.5 / 5 — PR was reopened with no PR-side commits since round 7. HEAD is still fbc9535, the carryover items remain unchanged, and the round-4 cold-start fix and reviewUrl guard are intact. Verdict carries forward.

• Identify what changed since last review (round 7 — merge fbc9535)
• Re-check the HITL surface (review-handler, changes-panel, project-guard, build-review-url)
• Verify carryover items
• Spot-check tests & submodule pointer
• Post review summary with score

What changed since round 7

Nothing. git log fbc9535..HEAD returns empty:

$ git rev-parse HEAD
fbc9535d1804b21f82958eb8730e037bd20e9fee

This trigger fired because the PR was reopened, not because new code landed. Reconfirmed the round-4 cold-start fix at project-guard.tsx:37-49 (if (!projects) return early-out + projects in deps), and the reviewUrl ternary guard at use-activity-logs.ts:236 — both still in place.

Carryovers (unchanged, all acknowledged as follow-up scope)

#	Where	Status	Notes
1	review-handler.ts:249	Defended	limit: 200 intentional bounded snapshot; joinAgentMeta dedups per path client-side. No comment per repo convention; this thread is the documentation.
2	get-agent-changes.ts:19	Defended	3 s polling matches useGetVcStatus for consistency. Push-driven invalidation via ReviewEvents.NOTIFY tracked as follow-up.
3	review-handler.ts:273	Defended	previousSummary asymmetry intentional (CLI brv review pending vs WebUI inline panel).
4	review-handler-list-operations.test.ts:274	Deferred	"same file across multiple entries" lock-in test scheduled for next coverage-expansion PR.
5	build-review-url.ts:4	Defended	localhost matches existing brv webui CLI output (src/oclif/commands/webui.ts:46); dual-stack concern theoretical for this project's deployment surface.
6	project-guard.tsx:49	Defended	searchParams deliberately not in deps — react-router-dom's useSearchParams() returns a fresh instance each render; urlParam already in deps suffices and react-hooks/exhaustive-deps isn't enabled here.
7	packages/byterover-packages pointer	⚠️ Still open	Submodule still at f42f9bd while package.json resolves @campfirein/byterover-packages#1.0.3 (991078c). BRV_UI_SOURCE=submodule|package flows therefore render different snapshots in local dev. Worth bumping in a follow-up. Fix this →

Outstanding pre-merge items (from prior rounds, not raised by this round)

• 👁️ @cuongdo-byterover flagged in round 6 that the UX of the supervision switch button still needs the requested polish before merging.
• ✅ lint / typecheck / npm test — PR description's checkboxes are still unchecked. Recommend running them locally before merge along with the manual QA from ENG-2809 against the bumped 1.0.3 bundle.

Strengths (unchanged from prior rounds)

• ✅ Pure utilities with focused unit tests (joinAgentMeta, splitReasonPrefix, shouldShowAgentPulse).
• ✅ Symmetric base64url encoders with cross-encoder agreement test (CJK / Cyrillic / emoji / Windows paths).
• ✅ resolveAutoSelectProject precedence (URL > selection > cwd) with allowlist defence against tampered URLs.
• ✅ TUI/server import boundary preserved; no webui/ → server/ cross-imports.
• ✅ Cold-start deep-link race fix from round 4 remains intact post-merge.

Verdict

4.5 / 5 — no PR-side changes this round. Once the supervision-switch UX polish lands (per @cuongdo-byterover's earlier thread), the submodule pointer is bumped to match 1.0.3, lint/typecheck/npm test are green, and the ENG-2809 manual QA is run, this is merge-ready.
· feat/ENG-2706