Refactor: createScanRule factory for the hand-rolled watcher lifecycle (15 rules)#244
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…e (15 rules)
Two factories already cover the clean rule shapes — createSelectorHideRule
(7 rules) and defineInlineTextRedactRule (3 rules). The remaining DOM-mutating
rules each hand-rolled the identical four-part skeleton: a module-level
watcher, a scanAndX(root), an apply wiring the two, and a teardown calling
watcher.stop(). AGENTS.md documented the skeleton, but documenting boilerplate
doesn't stop a new rule from forgetting skipPlaceholderSubtrees or its teardown.
Add createScanRule({ id, label, description, scan, skipPlaceholderSubtrees? })
in extension/src/lib/scan-rule.ts, generic over the literal RuleId so the
catalog's compile-time id check keeps narrowing. Migrate the 15 holdouts whose
lifecycle is exactly scan-on-apply-and-rescan (−244 LOC in the rule files):
scarcity-redact, disguised-ad-flag, html-comment-strip, noscript-strip,
svg-text-strip, unicode-invisibles-strip, link-spoof-annotate,
schema-trust-sanitize, trust-badge-annotate, cart-addon-annotate,
attribute-injection-sanitize, json-ld-sanitize, hidden-fee-annotate,
hidden-affiliate-sanitize, hidden-text-strip.
Rules with more lifecycle than that keep hand-wiring createSubtreeWatcher
directly — a head router (meta-injection-strip), a route/focus subscription
(form-prefill-annotate), a deferred snapshot (countdown-timer-redact), an
onSubtrees that re-scans document.body (cross-origin-frame-redact,
svg-sprite-strip), or a teardown that restores page state
(confirmshame-sanitize). The factory comment and the updated AGENTS.md section
spell out when to reach past it.
Add scan-rule.test.ts pinning the factory's own contract (scan-once-on-apply,
re-scan surfaced subtrees, forward skipPlaceholderSubtrees, stop on teardown).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
twschiller
changed the title
twschiller
force-pushed
the
refactor/create-scan-rule-factory
branch
from
de507d5 to
c7e5966
Compare
![unblocked[bot]](https://avatars.githubusercontent.com/in/166219?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
createScanRulelifecycle factory and migrates the 15 holdout rules that hand-rolled the identical subtree-watcher skeleton onto it.Two factories already cover the clean rule shapes —
createSelectorHideRule(7 rules) anddefineInlineTextRedactRule(3 rules). The remaining DOM-mutating rules each hand-rolled the same four-part skeleton, ~23 LOC each:const watcher = createSubtreeWatcher({...})scanAndX(root)scan functionapply=scan(root); watcher.start(root)teardown=watcher.stop()AGENTS.mddocumented the skeleton, but documenting boilerplate doesn't eliminate the bug surface — a new rule can still forgetskipPlaceholderSubtrees(its own placeholders then re-trigger the watcher) or drop the teardown (the observer leaks).The factory
extension/src/lib/scan-rule.ts:Owns the watcher lifecycle and returns a
Rulewithapply/teardownwired. Generic over the literalRuleId(mirroring the two existing factories) so the catalog's compile-time id-agreement check inrules/index.tskeeps narrowing.Migrated (15 rules, −244 LOC in the rule files)
scarcity-redact, disguised-ad-flag, html-comment-strip, noscript-strip, svg-text-strip, unicode-invisibles-strip, link-spoof-annotate, schema-trust-sanitize, trust-badge-annotate, cart-addon-annotate, attribute-injection-sanitize, json-ld-sanitize, hidden-fee-annotate, hidden-affiliate-sanitize, hidden-text-strip.
Deliberately left bespoke
Rules whose lifecycle is more than scan-on-apply-and-rescan keep hand-wiring
createSubtreeWatcherdirectly: head router (meta-injection-strip), route/focus subscription (form-prefill-annotate), deferred snapshot (countdown-timer-redact),onSubtreesthat re-scansdocument.body(cross-origin-frame-redact,svg-sprite-strip), teardown that restores page state (confirmshame-sanitize). The factory's doc comment and the updatedAGENTS.mdsection spell out when to reach past it.Tests
New
scan-rule.test.ts(7 tests) pins the factory's own contract: scans once onapply, re-scans surfaced subtrees, forwardsskipPlaceholderSubtrees, stops observing onteardown. Existing per-rule suites are unchanged and green.Verification
bun run check(biome + eslint),tsc×2,knip, full jest (2059 tests, 110 suites), and the markdown pre-commit all pass.🤖 Generated with Claude Code