[Drift Audit] Correct the plugin-points section in the architecture rule doc - #9371
Draft
daxmobile wants to merge 1 commit into
Draft
[Drift Audit] Correct the plugin-points section in the architecture rule doc#9371daxmobile wants to merge 1 commit into
daxmobile wants to merge 1 commit into
Conversation
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>
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
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.
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
@ContributesActivePluginPointdocumentation (remote-flag gating,ActivePluginPoint<T>,isActive(), "all flags default toTRUE") and is headed "Notable active plugin points in the browser". It named exactly two:JsInjectorPluginContentScopeJsMessageHandlersPluginWhy 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)onUnusedJsInjectorPluginPoint.content-scope-scripts/content-scope-scripts-impl/.../ContentScopeConfigPluginPoint.kt:36—@ContributesPluginPoint(scope = AppScope::class, boundType = ContentScopeJsMessageHandlersPlugin::class)onContentScopeJsMessageHandlersPluginPoint.Corroborating: neither
JsInjectorPlugin(browser-api) norContentScopeJsMessageHandlersPlugin(content-scope-scripts-api) extendsActivePlugin, which@ContributesActivePluginPointrequires, and every consumer injectsPluginPoint<T>, notActivePluginPoint<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 anisActive()gate that do not exist for these two, and would reach forActivePluginPoint<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
featureNameas declared in code:featureNameNewTabPagePluginpluginPointNewTabPagePluginapp/.../browser/newtab/NewTabPageProvider.kt:63NewTabPageSectionPluginpluginPointNewTabPageSectionPluginnew-tab-page-impl/.../RealNewTabPageSectionProvider.kt:100NewTabPageShortcutPluginpluginPointNewTabPageShortcutPluginnew-tab-page-impl/.../shortcuts/NewTabShortcutsProvider.kt:99NativeInputPluginpluginPointNativeInputduckchat-impl/.../nativeinput/NativeInputPlugin.kt:61NativeInputChatTabItemPluginpluginPointNativeInputChatTabItemPluginduckchat-impl/.../inputscreen/ui/suggestions/NativeInputChatTabItemPluginPoint.kt:30One 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 missingActivePluginsupertype, and thePluginPoint<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:
architecture.mdc. I re-verified the DI scope list againstdi/.../scopes/(all seven exist), theGlobalActivityStarteroverload table againstnavigation-api(start/startIntent/startForResultall present with the documented shapes), and the plugin naming-convention claim — thepluginPointprefix really is enforced at compile time (ContributesActivePluginPointProcessor.kt:279), as is the non-blankfeatureName(:275). All accurate.#9153; I stayed out of it to avoid conflicting edits..cursor/rules/wide-events.mdc— the interval/Durationdrift is covered by the still-open#9355. Re-reading the rest againstWideEventClient.kt,FlowStatus,CleanupPolicy,SAMPLED_OUT_FLOW_IDandCompletedWideEventsProcessorfound nothing further, with one borderline call I left alone: the doc says the two transports are "controlled by thewideEventsremote feature flag", where in fact each has its own sub-toggle (sendWideEventsViaPixels()/sendWideEventsViaPost()inDelegatingWideEventSender). Those sub-toggles do belong toWideEventFeature, 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 theddg.didual-build property (build.gradle:255,gradle.properties:31). All still true. ThetargetSdk-to-Android-16 change (#9283) touches nothing the docs state — they deliberately point at the build files instead of restating SDK levels.#9366), autoconsent and content-scope-scripts bumps (#9361,#9354,#9327), theaccount_infokeypair work (#9311,#9325,#9326) — all inside-implcode the rule docs don't describe.#9364edits.claude/skills/prepare-sbt-repro/SKILL.md, which is a skill, not a rule doc, and skill-mirror parity belongs toaiConfigCheck.A caveat on coverage: 21 of the ~31 PRs merged to
developin 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/*.mdcfile (the.claude/rules/architecture.mdsymlink is untouched).🤖 AI-docs Drift Auditor
Steps to test this PR
JsInjectorPluginPoint.ktandContentScopeConfigPluginPoint.ktboth use@ContributesPluginPoint, and neitherJsInjectorPluginnorContentScopeJsMessageHandlersPluginextendsActivePlugin.BrowserWebViewClient:127injectsPluginPoint<JsInjectorPlugin>, notActivePluginPoint<JsInjectorPlugin>.featureNamein the "active" table matches its@ContributesActivePluginPointdeclaration at the file/line listed above.UI changes
Note
🔒 Integrity filter blocked 79 items
The following items were blocked because they don't meet the GitHub integrity level.
search_pull_requests: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".list_commits: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".To allow these resources, lower
min-integrityin your GitHub frontmatter: