Feat/eng 2697

[DatPham-6996]

Summary

• Problem: On a successful brv curate, the skill instructed the in-daemon agent to report only data.filePath. It never pointed users at the ByteRover dashboard where the saved topic is actually viewable, so users had no obvious way to see what was just curated.
• Why it matters: The dashboard already exists and the daemon serves it on http://localhost:7700 by default — but the skill never surfaced it, leaving a working capability undiscovered and the curate loop ending at "a file path" instead of "here's your knowledge, rendered."
• What changed: Updated the skill templates the curate agent reads at runtime. On done, the agent now hands the user the clickable dashboard URL (Contexts page) alongside filePath. Added brv webui to SKILL.md's Quick Reference + command list, an "Example Session Responses" JSON walkthrough and a "View What You Saved" section to curate.md, plus a new Common Mistakes row. Tightened the connector test to guard the new content.
• What did NOT change (scope boundary): No runtime, CLI, or transport code. The brv webui command, the webui server, the curate engine, and port handling are untouched — this is documentation/behavioral-contract text plus a test assertion. The 7700 default is referenced, not introduced.

Type of change

• Bug fix
• New feature
• Refactor (no behavior change)
• Documentation
• Test
• Chore (build, dependencies, CI)

Scope (select all touched areas)

• TUI / REPL
• Agent / Tools
• LLM Providers
• Server / Daemon
• Shared (constants, types, transport events)
• CLI Commands (oclif)
• Hub / Connectors
• Cloud Sync
• CI/CD / Infra

(Skill templates live under src/server/templates/skill/ and are deployed/read via the SkillConnector in server/infra/connectors/skill/.)

Linked issues

• Closes #N/A (tracker is Linear, no GitHub issue)
• Related: Linear ENG-2697 — "Rewrite ByteRover skill based on Superpowers skill" (https://linear.app/byterover/issue/ENG-2697). This is an enhancement layered on that rewrite.

Root cause (bug fixes only, otherwise write N/A)

• Root cause: N/A
• Why this was not caught earlier: N/A

Test plan

• Coverage added:
  • Unit test
  • Integration test
  • Manual verification only
• Test file(s): test/unit/infra/connectors/skill/skill-connector.test.ts
• Key scenario(s) covered:
  • Installed SKILL.md includes brv webui.
  • Installed curate.md includes localhost:7700 and the ## Example Session Responses section.

User-visible changes

• After a successful brv curate, the agent now gives the user the dashboard URL http://localhost:7700 (Contexts page) alongside the saved filePath, instead of reporting only the path.
• SKILL.md Quick Reference and command list now document brv webui.
• No brv CLI flags, defaults, or command output changed.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording

Before (old templates against the new assertions):

37 passing
2 failing
AssertionError: expected '---\nname: byterover...' to include 'brv webui'
AssertionError: expected '---\nname: byterover-curate...' to include 'localhost:7700'

After (npx mocha "test/unit/infra/connectors/skill/skill-connector.test.ts"):
39 passing (62ms)

Checklist

• Tests added or updated and passing (ran the touched suite — 39 passing; full npm test not run locally)
• Lint passes (npm run lint) — not run
• Type check passes (npm run typecheck) — not run
• Build succeeds (npm run build) — not run
• Commits follow Conventional Commits format (docs(skill): point users to the webui dashboard after curate)
• Documentation updated (this PR is the documentation)
• No breaking changes
• Branch is up to date with main (origin/main is an ancestor of HEAD)

Risks and mitigations

• Risk: The dashboard URL is hardcoded to http://localhost:7700; if the user changed the webui port (BRV_WEBUI_PORT or a persisted port), the URL the agent surfaces will be wrong.
  • Mitigation: 7700 is the documented first-run default and the most common case; the docs also tell the agent that brv webui opens the same dashboard. Templating the actual port into the message is a reasonable follow-up.
• Risk: The agent may surface the URL even when the daemon/webui isn't reachable.
  • Mitigation: The daemon serves the dashboard by default whenever brv is running, so a curate succeeding implies the daemon is up; worst case is a stale link, no functional impact.

[github-actions]

Claude finished @DatPham-6996's task in 3m 40s —— View job

---

PR Review: docs(skill) — point users to the webui dashboard after curate

Overall score: 3 / 5 — Low-risk docs/test change with a sensible goal, but the central <served-port> abstraction has no resolution path the curate agent can actually use, so the new template is no more correct than the pre-PR hardcoded 7700 and can be strictly worse for an agent that copies templates verbatim.

• Read the changed files
• Analyze diff for correctness, security, code quality
• Verify the test assertions match content (39 assertions look consistent with current template text; couldn't execute the suite — node_modules not installed in this sandbox)
• Post inline comments for specific issues
• Post final summary with score

---

Summary

This PR closes a real UX gap (curate's done state ended at a file path, never pointing the user at the dashboard that already renders the saved topic). Scope is documentation-only — no runtime, CLI, or transport changes — so blast radius is low and reversibility is trivial. Tests tighten the connector contract with both positive (localhost:<served-port>, brv webui --port <port> fallback) and defensive negative (not.to.include('printed Web UI URL')) assertions, which is good practice for prompt/template regressions.

The core concern is that the new <served-port> placeholder is unbacked by any mechanism the agent can use at curate time:

• brv curate --format json exposes data.filePath / data.sessionId only — no webuiUrl / webuiPort (src/oclif/commands/curate*.ts, src/oclif/lib/).
• brv status doesn't surface the webui port either (no webui*Port references in status code).
• The only way to learn the actual port is to run brv webui, which opens a browser — exactly what onboarding.md now tells the agent NOT to do as a routine closeout.

In practice the agent will either always emit http://localhost:7700 (the documented default — same outcome as before this PR) or, worse, leave the literal <served-port> in the user-facing artifact (broken link). The PR body acknowledges this as a follow-up, but the docs introduce the abstraction now with no way to land it. A cleaner shape would be one of:

1. Wire the port into the done envelope. Add data.webuiUrl to the curate done JSON (read it via webui:getPort socket event, same path brv webui already uses). The skill text then says "report data.filePath and data.webuiUrl" — deterministic, no agent inference, no placeholder.
2. Drop the placeholder. Use http://localhost:7700 literally, plus one sentence noting BRV_WEBUI_PORT overrides. Matches reality for almost all users and makes the user-visible artifact in onboarding.md L103/L130 clickable.

Findings

1. Major — <served-port> has no resolution path (curate.md L50, L112, L196; SKILL.md L123, L177; onboarding.md L103/L130/L138). Five duplicated occurrences of guidance that can't be operationalized.
2. Correctness — user-visible artifact is templated text in onboarding.md L103 & L130. Naive agents copy verbatim and the user sees http://localhost:<served-port> instead of a clickable URL. Also: L103 includes "— or run brv webui if that link doesn't open." but L130 doesn't — inconsistent worked examples.
3. DRY / signal-to-noise — same long fallback sentence repeats verbatim 5×. Locking the exact phrasing into a single section and referencing it elsewhere would shrink agent context and reduce drift risk.
4. SKILL.md Quick Reference row L123 mixes column semantics. Every other "Command" cell is an actual brv command; this one is a URL. The Command Map at L177 already covers brv webui correctly — the table row is duplicate-with-a-twist.
5. Praise — defensive negative assertions in the new test (not.to.include('printed Web UI URL'), not.to.include('run \brv webui` after every successful curate')`) lock out two phrasings the project explicitly rejected. Future regressions will fail loudly.
6. Minor — positive localhost:<served-port> assertion in the new universal test only checks onboarding.md. Per-file tests cover today's set; a future skill file added to SKILL_FILE_NAMES won't be scrutinized unless someone adds its own positive test. Non-blocking.

Security / build

• No security concerns — pure docs + test text.
• Couldn't run npm run lint / npm run typecheck / the touched mocha suite (deps not installed in this sandbox). PR body notes lint/typecheck/build weren't run locally either; CI should be the source of truth here.

---

Inline comments (5) are buffered and will post with this review.
· Branch feat/ENG-2697

[github-actions]

Claude finished @DatPham-6996's task in 2m 45s —— View job

---

PR Review — docs(skill): use printed webui URL in skill guidance

Overall score: 4 / 5 — The headline correctness issue from the prior review is fixed: <served-port> is gone, replaced with the literal http://localhost:7700 default. That alone moves this from "abstraction without resolution path" to "deterministic guidance that matches reality for ~all users." Remaining items are consistency/DRY nits, not correctness.

• Fetched latest changes & inspected diff vs origin/main
• Read changed files (SKILL.md, curate.md, onboarding.md, skill-connector.test.ts)
• Confirmed <served-port> was removed in commits 47771b6/cc8a15a/74e0095
• Verified test contract is symmetric (positive localhost:7700 + negative rejections of prior phrasings)
• Posted inline findings (4 conventional comments)

Summary

The followup commits (47771b6, cc8a15a, 74e0095 — all titled "use printed webui URL in skill guidance") effectively pick option 2 from the prior review: drop the unresolvable <served-port> placeholder and use the literal http://localhost:7700 default. The PR is now what it claims to be — documentation-only, no runtime change required.

What's solid:

• Correctness recovered. Naïve agents now copy a real, clickable URL into the user-visible "Saved:" artifact. The pre-PR baseline (hardcoded 7700) is preserved.
• Defensive negative assertions (not.to.include('http://localhost:<served-port>'), not.to.include('printed Web UI URL'), not.to.include('run \brv webui` after every successful curate')`) lock out three phrasings the project has explicitly rejected. Future regressions fail loudly.
• Negative invariant test loops SKILL_FILE_NAMES — so the contract holds for every managed skill file, not just the three with explicit positive coverage. That's the right shape for cross-file drift.
• Quick Reference row in SKILL.md (L123) is cleaned up — the URL-in-Command-column smell from the prior review is gone.

What's still nitty:

1. Two artifact-template blocks in onboarding.md (L102-105 vs L116-132) are not byte-parallel on the closing lines — one has the "or run brv webui" fallback, the other doesn't; one omits the "Also version-controlled…" teaser that L142 says is mandatory. Whichever block the agent copies, the output is half-right. (See inline L102-104.)
2. DRY drift. The "Web UI link + fallback" guidance is now phrased six different ways across curate.md and SKILL.md; the long fallback paragraph appears verbatim in five places. Recommend one canonical paragraph in curate.md → ## View What You Saved, referenced everywhere else. (See inline curate.md L31-33 and SKILL.md L177.)
3. "If a known custom Web UI port is already serving" is still effectively unreachable — the agent has no way to detect this without running brv webui (the very thing onboarding.md L138 says NOT to do routinely). Either drop the caveat or reframe it as "if the user has already told you they're on a different port" so it's actionable. (See inline curate.md L31-33.)
4. Cosmetic: double space at onboarding.md L103 (7700 — or). (Same inline.)

None of these block the fix; they're a follow-up tightening pass.

Security / build

• No security concerns — pure docs + test text.
• I did not run npm run lint / npm run typecheck / the mocha suite in this sandbox; the test assertions are textually consistent with the current templates, so the new tests should pass. CI is the source of truth.
  · Branch feat/ENG-2697

Inline comments (4) are buffered and will post with this review.