Skip to content

[Drift Audit] Correct the plugin-points section in the architecture rule doc - #9371

Draft
daxmobile wants to merge 1 commit into
developfrom
drift-audit/plugin-points-architecture-doc-cbfca4f1ed344367
Draft

[Drift Audit] Correct the plugin-points section in the architecture rule doc#9371
daxmobile wants to merge 1 commit into
developfrom
drift-audit/plugin-points-architecture-doc-cbfca4f1ed344367

Conversation

@daxmobile

Copy link
Copy Markdown
Collaborator

Task/Issue URL: (app.asana.com/redacted)

Description

.cursor/rules/architecture.mdc — the "Notable active plugin points in the browser" section.

What the doc described: the section sits immediately after the @ContributesActivePluginPoint documentation (remote-flag gating, ActivePluginPoint<T>, isActive(), "all flags default to TRUE") and is headed "Notable active plugin points in the browser". It named exactly two:

  • JsInjectorPlugin
  • ContentScopeJsMessageHandlersPlugin

Why that no longer matches the code: neither is an active plugin point. Both are basic plugin points with no feature-flag gating:

  • app/src/main/java/com/duckduckgo/app/plugins/JsInjectorPluginPoint.kt:23@ContributesPluginPoint(scope = AppScope::class, boundType = JsInjectorPlugin::class) on UnusedJsInjectorPluginPoint.
  • content-scope-scripts/content-scope-scripts-impl/.../ContentScopeConfigPluginPoint.kt:36@ContributesPluginPoint(scope = AppScope::class, boundType = ContentScopeJsMessageHandlersPlugin::class) on ContentScopeJsMessageHandlersPluginPoint.

Corroborating: neither JsInjectorPlugin (browser-api) nor ContentScopeJsMessageHandlersPlugin (content-scope-scripts-api) extends ActivePlugin, which @ContributesActivePluginPoint requires, and every consumer injects PluginPoint<T>, not ActivePluginPoint<T>BrowserWebViewClient:127, DuckChatWebViewClient:28, SettingsWebViewClient:28, ContentScopeScriptsJsMessaging.

The practical cost of the error is that the doc is the only place a reader is pointed at concrete examples, and it points at the wrong kind. Someone following it would expect a pluginPoint… remote flag and an isActive() gate that do not exist for these two, and would reach for ActivePluginPoint<T> at the injection site.

How the doc now reads: the heading drops "active", and the section is split into two small tables — the two basic points (with the interface that declares each point) and the active points that actually exist, each with its plugin-point featureName as declared in code:

Plugin featureName Declared in
NewTabPagePlugin pluginPointNewTabPagePlugin app/.../browser/newtab/NewTabPageProvider.kt:63
NewTabPageSectionPlugin pluginPointNewTabPageSectionPlugin new-tab-page-impl/.../RealNewTabPageSectionProvider.kt:100
NewTabPageShortcutPlugin pluginPointNewTabPageShortcutPlugin new-tab-page-impl/.../shortcuts/NewTabShortcutsProvider.kt:99
NativeInputPlugin pluginPointNativeInput duckchat-impl/.../nativeinput/NativeInputPlugin.kt:61
NativeInputChatTabItemPlugin pluginPointNativeInputChatTabItemPlugin duckchat-impl/.../inputscreen/ui/suggestions/NativeInputChatTabItemPluginPoint.kt:30

One thing for a human to weigh. I could not attribute this to a specific merged PR — no merged PR in the repo mentions JsInjectorPluginPoint, and the checkout available to this run is shallow, so I could not blame the file. It is possible the section was inaccurate when written rather than having drifted. I raised it anyway because the mismatch is not a judgement call — the annotation, the missing ActivePlugin supertype, and the PluginPoint<T> injection sites all agree — and because native input, where two of the real active plugin points live, is the area recent work has concentrated in (#9324, #9338), which is what prompted re-reading this doc. If the preference is that Drift Audit only fixes drift traceable to a code change in its window, that is a fair reason to close this.

I also checked, and did not treat as drift:

  • The rest of architecture.mdc. I re-verified the DI scope list against di/.../scopes/ (all seven exist), the GlobalActivityStarter overload table against navigation-api (start / startIntent / startForResult all present with the documented shapes), and the plugin naming-convention claim — the pluginPoint prefix really is enforced at compile time (ContributesActivePluginPointProcessor.kt:279), as is the non-blank featureName (:275). All accurate.
  • The "URL vs. Search Classification" section — already covered by the still-open #9153; I stayed out of it to avoid conflicting edits.
  • .cursor/rules/wide-events.mdc — the interval/Duration drift is covered by the still-open #9355. Re-reading the rest against WideEventClient.kt, FlowStatus, CleanupPolicy, SAMPLED_OUT_FLOW_ID and CompletedWideEventsProcessor found nothing further, with one borderline call I left alone: the doc says the two transports are "controlled by the wideEvents remote feature flag", where in fact each has its own sub-toggle (sendWideEventsViaPixels() / sendWideEventsViaPost() in DelegatingWideEventSender). Those sub-toggles do belong to WideEventFeature, so the sentence is imprecise rather than false, and that file already has an open PR.
  • .cursor/rules/android-design-system.mdc — two open Drift Audit PRs against it (#9300, #9315); left alone.
  • .cursor/rules/pixels.mdc / pixel-definitions.mdc — process/privacy guidance, untouched by anything in this window.
  • .cursor/rules/maestro-ui-tests.mdc — no .maestro/** tag or config mechanics changed.
  • AGENTS.md — re-checked the two live toolchain claims: JVM target 17 (build.gradle:220, JvmTarget.JVM_17) and JDK 21 (.github/.java-version = 21), plus the ddg.di dual-build property (build.gradle:255, gradle.properties:31). All still true. The targetSdk-to-Android-16 change (#9283) touches nothing the docs state — they deliberately point at the build files instead of restating SDK levels.
  • Merged in this window and irrelevant to the rule docs: the onboarding button margin (#9366), autoconsent and content-scope-scripts bumps (#9361, #9354, #9327), the account_info keypair work (#9311, #9325, #9326) — all inside -impl code the rule docs don't describe. #9364 edits .claude/skills/prepare-sbt-repro/SKILL.md, which is a skill, not a rule doc, and skill-mirror parity belongs to aiConfigCheck.

A caveat on coverage: 21 of the ~31 PRs merged to develop in this window were hidden from me by the integrity filter, so this pass leaned on reading the docs against the current code rather than on the merge list alone.

Only prose changed — no code, and only the canonical .cursor/rules/*.mdc file (the .claude/rules/architecture.md symlink is untouched).

🤖 AI-docs Drift Auditor

Steps to test this PR

  • Confirm the two "basic" rows: JsInjectorPluginPoint.kt and ContentScopeConfigPluginPoint.kt both use @ContributesPluginPoint, and neither JsInjectorPlugin nor ContentScopeJsMessageHandlersPlugin extends ActivePlugin.
  • Spot-check a consumer — e.g. BrowserWebViewClient:127 injects PluginPoint<JsInjectorPlugin>, not ActivePluginPoint<JsInjectorPlugin>.
  • Confirm each featureName in the "active" table matches its @ContributesActivePluginPoint declaration at the file/line listed above.
  • Sanity-check that the active list is complete for the browser, and say so if a plugin point worth naming is missing.

UI changes

Before After
No UI changes No UI changes

Note

🔒 Integrity filter blocked 79 items

The following items were blocked because they don't meet the GitHub integrity level.

  • #8891 search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 06c894f list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 25ee1c9 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 09c5303 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • cbe8848 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 89f01ab list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 9d00f93 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 17f3d59 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 7a9170b list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • a9c2e87 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 5a41409 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 809ce55 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • c14ae3d list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 3eafa84 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • b5ffb52 list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • 57cea6c list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
  • ... and 63 more items

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by AI-docs Semantic Drift Audit ·

JsInjectorPlugin and ContentScopeJsMessageHandlersPlugin were listed as
"notable active plugin points" but both are declared with
@ContributesPluginPoint and consumed as PluginPoint<T>. List them as
basic plugin points and name the real active ones.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant