fix: disambiguate shared rules files via footer agent name

[RyanNg1403]

Summary

• Problem: brv connectors list reported every agent that maps to a shared rules file (e.g. Amp, Codex, OpenCode all map to AGENTS.md; Claude Code and OpenClaude both map to CLAUDE.md) as having a rules connector installed, as soon as the BRV markers were present in the file. A user who installed once for one agent saw "ghost" entries for the others.
• Why it matters: misleading list output, and the wrong impression that several agents were configured when only one was. Reported by the team after running brv connectors install for a single agent.
• What changed: the rules connector's status() now reads the existing Generated by ByteRover CLI for X footer that the template service already writes inside the BRV section, and requires the extracted agent name to match the agent being queried. Footer-less files (legacy installs predating the footer) keep today's behavior so existing users are not surprised.
• What did NOT change (scope boundary): install/uninstall paths are untouched. The MCP, hook, and skill connectors are unchanged. The new helper extractInstalledAgentFromBrvSection is exported but only consumed by the rules connector for now.

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

Linked issues

• Closes #
• Related #

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

• Root cause: the rules connector's status check was installed = hasMarkers && !hasMcpTools. For agents that share a rule file (Amp / Codex / OpenCode -> AGENTS.md; Claude Code / OpenClaude -> CLAUDE.md), every sharing agent saw the same markers and reported installed: true, even though the install had been performed once for a single agent. The template service has been writing a per-agent footer (Generated by ByteRover CLI for X) inside the BRV section for some time, but no read path consulted it.
• Why this was not caught earlier: tests and manual verification typically used per-agent rule files (e.g., .cursor/rules/agent-context.mdc), where one file maps to one agent. The shared-file collision only surfaces when a project is configured with one of the agents that share AGENTS.md or CLAUDE.md.

Test plan

• Coverage added:
  • Unit test
  • Integration test
  • Manual verification only
• Test file(s):
  • test/unit/infra/connectors/shared/constants.test.ts (new) — 8 cases for extractInstalledAgentFromBrvSection (happy path, multi-word agents, missing footer, missing markers, footer outside section, end-before-start, blank footer) plus a regression guard for hasMcpToolsInBrvSection
  • test/unit/infra/connectors/rules/rules-connector.test.ts (new) — 5 cases covering the shared-file scenarios: Codex-authored AGENTS.md, Amp-authored AGENTS.md, footer-less legacy file (fallback contract), CLAUDE.md isolation, and the existing MCP-tools-in-section guard
• Key scenario(s) covered:
  • Codex-authored AGENTS.md -> Codex rules installed: true; Amp + OpenCode installed: false
  • Amp-authored AGENTS.md -> Amp rules installed: true; Codex + OpenCode installed: false
  • Footer-less AGENTS.md -> all three sharing agents installed: true (legacy fallback preserved)
  • Fresh end-to-end install (brv connectors install Codex --type rules) writes the footer; subsequent brv connectors list shows only Codex
  • CLAUDE.md populated for Claude Code -> Claude Code only; Amp / Codex / OpenCode installed: false

User-visible changes

brv connectors list no longer reports ghost rules entries for agents that share a rule file. After installing a rules connector for one agent (e.g. Codex), the list shows only that agent, not every other agent that happens to use the same file path.

For users on a footer-less rule file from an older CLI version, behavior is unchanged until the next brv connectors install for that agent rewrites the section with a footer.

Evidence

Manual reproduction (interactive brv connectors list --format json):

Test 1 — Codex-authored AGENTS.md (footer says Codex):

{ "connectors": [
  { "agent": "Codex", "connectorType": "rules", ... }
] }

(before this PR: Amp, Codex, OpenCode all listed as rules)

Test 2 — Amp-authored AGENTS.md:

{ "connectors": [
  { "agent": "Amp", "connectorType": "rules", ... }
] }

Test 3 — footer-less AGENTS.md (legacy fallback):

{ "connectors": [
  { "agent": "Amp", ... }, { "agent": "Codex", ... }, { "agent": "OpenCode", ... }
] }

Test 4 — fresh brv connectors install Codex --type rules followed by brv connectors list: shows Codex only.

Test 5 — CLAUDE.md written for Claude Code: shows Claude Code only.

Unit test run: 15 passing across the two new test files. Full suite: 7377 passing, 16 pending.

Checklist

• Tests added or updated and passing (npm test)
• Lint passes (npm run lint)
• Type check passes (npm run typecheck)
• Build succeeds (npm run build)
• Commits follow Conventional Commits format
• Documentation updated (if applicable)
• No breaking changes (or clearly documented above)
• Branch is up to date with main

Risks and mitigations

• Risk: a user with a footer-less legacy AGENTS.md will continue to see all sharing agents listed until they reinstall.
  • Mitigation: deliberate legacy fallback contract. The next brv connectors install <agent> --type rules rewrites the section with a footer and the new disambiguation kicks in for that file. Documented in the inline comment at rules-connector.ts:135-137 and exercised by a dedicated unit test.
• Risk: a future agent that ships with manual-mode MCP and rules at AGENTS.md/CLAUDE.md would re-introduce a similar shared-file collision on the MCP path, where mcp-connector.statusManual does not yet check the footer.
  • Mitigation: out of scope for this PR (no current overlap). The new helper extractInstalledAgentFromBrvSection is generic; wiring it into statusManual is a two-line follow-up if/when that situation arises.

[github-actions]

Claude finished @RyanNg1403's task in 3m 8s —— View job

---

Claude PR Review

Overall score: 4.5 / 5 — focused, well-tested, low-risk fix that aligns with project conventions. Ship-ready after considering the inline notes (none are blocking).

Summary

This PR fixes a genuine, user-visible bug (brv connectors list showing ghost rules entries for every agent that shares AGENTS.md / CLAUDE.md) by reading the per-agent footer (Generated by ByteRover CLI for X) the template service has already been writing. The fix is the minimum viable change: a new pure helper extractInstalledAgentFromBrvSection plus a single line wired into RulesConnector.status(). Install/uninstall paths are deliberately untouched, scope is well-bounded, and the legacy-fallback contract (footer absent ⇒ trust marker) is explicit, commented at the call site, and exercised by a dedicated test.

What's good

• praise: root cause is correctly identified — installed = hasMarkers && !hasMcpTools was symmetric across agents that share a file, and the disambiguator was already present in the file but never read. Fix consults the existing artifact rather than inventing new state. (src/server/infra/connectors/rules/rules-connector.ts:137-142)
• praise: good factoring — sliceBrvSection (src/server/infra/connectors/shared/constants.ts:15-20) deduplicates the START/END boundary logic, and the additional endIdx < startIdx guard hardens both the new helper and the existing hasMcpToolsInBrvSection for free. The regression test on lines 60-66 of constants.test.ts correctly locks in the existing behavior so the refactor is provably safe.
• praise: TDD-style coverage is solid — 8 helper cases (happy path, multi-word agent, missing footer, missing markers, footer-outside-section, swapped markers, blank footer) + 5 connector-level scenarios covering both the bug (Codex/Amp/OpenCode share AGENTS.md) and the isolation case (CLAUDE.md ↔ AGENTS.md independence). The StubTemplateService whose method throws is a nice trip-wire against the status path accidentally regenerating templates.
• praise: comments follow the project's "explain why, not what" rule. Lines 138-140 of rules-connector.ts are exactly the right size — they capture the legacy-fallback rationale at the decision site without restating what the next line does.

Issues / suggestions (all non-blocking)

• suggestion (nit): BRV_RULE_TAG is matched by indexOf with no trailing-space anchor, so a string like Generated by ByteRover CLI forXxx would match. The template never produces that, so it's a defensive-only concern. Either anchor with ${BRV_RULE_TAG} or use a regex. (inline at src/server/infra/connectors/shared/constants.ts:44-49)
• nit: five test descriptions in constants.test.ts say "returns null" while the assertions check undefined and the helper's JSDoc / return type say undefined. CLAUDE.md prefers undefined; descriptions should match. (inline at test/unit/infra/connectors/shared/constants.test.ts)
• question (non-blocking): consider one debug log when footerAgent !== agent so a user reporting "Amp install isn't showing up" can be triaged without re-running. Not required. (inline at src/server/infra/connectors/rules/rules-connector.ts:137-142)
• suggestion (nit): the buildAgentsMd() legacy fixture in rules-connector.test.ts is a synthesized shape, not a real artifact from a pre-footer CLI version. If a real sample exists, mirroring it would turn this into a true regression test. (inline at test/unit/infra/connectors/rules/rules-connector.test.ts:21-23)

Out-of-scope but worth tracking

The PR description correctly flags that mcp-connector.statusManual (src/server/infra/connectors/mcp/mcp-connector.ts:377-410) has the same shape as the pre-fix rules path — installed: hasMarkers && hasMcpTools with no footer check. There is no agent today that ships manual-mode MCP into a shared AGENTS.md / CLAUDE.md, so the bug is not yet reachable. The new extractInstalledAgentFromBrvSection helper is exported and ready, so wiring it into statusManual is a two-line follow-up if/when that overlap appears. Worth a tracked issue rather than expanding this PR.

Correctness / security / quality

• Correctness: the slicing uses slice(startIdx, endIdx) so the END marker is excluded from the searched region — BRV_RULE_TAG will not be found inside the marker text itself. trim() handles trailing whitespace and CRLF on Windows (\r is a whitespace character). Multi-word agents ("Augment Code", "Claude Code") are preserved verbatim by reading the whole agent line, not splitting on whitespace.
• Security: no new I/O, no new user-controlled execution paths. The agent-name string is only compared by equality and never used for path joining or shell invocation.
• Quality: complies with project conventions — undefined over null, ?? not || (not needed here), no as Type assertions, no any, ES-module .js import extensions, no SQLite, file-based behavior.

Conclusion

I'd merge this. The four inline notes are polish, not gates. The 0.5 deduction is for: (1) the loose BRV_RULE_TAG boundary and (2) the test-description / behavior-description mismatch — neither affects correctness today, but both are cheap to address.
· branch fix/ENG-2221

[github-actions]

Claude finished @RyanNg1403's task in 1m 38s —— View job

---

Claude PR Review (follow-up: synchronize)

Overall score: 4.8 / 5 — both blocking-ish items from the previous review are cleanly resolved. Ship-ready.

Summary

The new commit 0733d72 is a focused, minimal fix to the two concrete asks from the prior review:

1. Tag boundary tightened — extractInstalledAgentFromBrvSection now matches `${BRV_RULE_TAG} ` (with the trailing space) instead of BRV_RULE_TAG alone. This aligns the read path with what template-service.ts:18,22 actually writes (`${BRV_RULE_TAG} ${agent}`), eliminating the false-positive risk on a malformed "...CLI forXxx" footer. The tagWithDelimiter.length variant (instead of BRV_RULE_TAG.length + 1) is the cleaner of the two suggestions in the prior review.
2. Test descriptions aligned with behavior — all five "returns null" cases (lines 25, 30, 35, 45, 50) are now "returns undefined", matching both the assertions and the helper's JSDoc / return type, and consistent with the project's undefined-over-null convention.
3. Regression test added — constants.test.ts:55-58 pins the new boundary so any future loosening of the matcher will surface immediately. Good defensive choice — the test names the exact malformed shape ("...CLI forXxx") so the intent is unambiguous.

What I checked

• Read all four changed files at HEAD.
• Verified the new matcher is symmetric with the template service write path (grep BRV_RULE_TAG): template-service.ts writes `${BRV_RULE_TAG} ${agent}`, the read path requires `${BRV_RULE_TAG} `, no drift.
• Confirmed the tagWithDelimiter.length slice math is correct (indexOf returns the offset of the first char of the match, + length lands on the character after the trailing space).
• Confirmed the new boundary test is inside the existing describe block and reuses wrapWithBrvSection, so it is consistent with the rest of the suite.
• Did not run npm test locally — dep install requires approval and the prior CI was green; the change is a one-character matcher tightening + test renames, so static analysis is sufficient.

Items not addressed (both non-blocking from prior review, deliberately deferred)

• Optional one-line debug log when footerAgent !== agent (diagnosability nit) — fine to skip.
• Mirroring a real pre-footer install for the legacy fixture (test-shape nit) — fine to skip; the contract under test (footer absent ⇒ trust marker) is what matters and is correctly exercised.

Out-of-scope tracker (carried forward)

mcp-connector.statusManual (src/server/infra/connectors/mcp/mcp-connector.ts:377-410) has the same shape as the pre-fix rules path. No agent currently ships manual-mode MCP into a shared file, so the bug is not yet reachable. The new helper is exported and ready to be wired in if/when that overlap appears — a tracked issue is the right home, not this PR.

Conclusion

The 0.2-point deduction (down from 0.5) reflects only the two non-blocking nits intentionally deferred. Correctness, security, and code quality remain unchanged from the prior review — both clean.

---

• Read changed files (4)
• Verify tag-boundary fix in constants.ts
• Verify test description rename in constants.test.ts
• Cross-check matcher with template-service.ts write path
• Inline comments posted (boundary fix, test rename)
• Final review posted
  · branch fix/ENG-2221