[Remove] site registry formatter alias - #438
Conversation
|
Warning Review limit reachedNext included review available in 39 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
04a53e4 to
de3f2fc
Compare
de3f2fc to
d74dd38
Compare
|
Self-review completed: 0 [fix here], 0 functional [follow-up]. Fresh independent judgement verified shared formatting and site-refusal integration coverage remain. Local lint and all 1,248 tests pass on Node 24.18.0. The stale logging comment is corrected in #440. CodeRabbit has no inline findings; attempts are rate limited. |
Why
site-registryonly forwarded refused paths to the sharedsafe-log.describeRefused()formatter. That alias duplicated the formatter API and its direct formatter tests without providing site-registry-specific behaviour.What changes
Refusal paths in the delete, reveal, and debug-log guards now call
describeRefused()directly. The local alias/export and its formatter-only assertions are removed; behaviour-level refusal coverage, including the newline-forgery case, remains.How to test this
Platforms: any — this is a Node-only refactor with no user-visible surface.
Starting state:
Install the repository dependencies.
Run
node --test tests/unit/site-registry.test.cjs; all 16 tests pass, including the rejected-path newline-forgery behaviour.Run
npm run lintandnpm test; both complete successfully.What must not have happened:
No registry refusal may reach its injected logger with an unescaped newline, and no delete, reveal, or debug-log operation may gain access to an unregistered path.
Risks and limitations
Low risk: the changed call sites retain the same shared formatter and the refusal-behaviour test remains. Independent self-review found 1 [fix here] architecture documentation issue in
src/logging.js: its comment still names the removed alias; this PR intentionally leaves it untouched because #430 is explicitly scoped tosrc/site-registry.jsand its unit test.Related
Fixes #430
Design decisions and alternatives considered
Inlining preserves the single safe formatter in
safe-log.jsand removes an API that added no registry-specific meaning. The formatter module's own tests remain the source of truth for escaping and truncation.Review outcome (required — see AGENTS.md)
1 [fix here] · 0 [follow-up] — no functional findings; the one documentation finding is deferred because the task explicitly restricts the change to two files.
Implementation notes
Validation passed:
node --test tests/unit/site-registry.test.cjs(16 tests),npm run lint, andnpm test(1,255 tests). The pre-existing uncommittedpackage-lock.jsonchange was not staged or included.