Fix code-review findings in the Chrome extension and test harness; release v3.6.0.0 (extension 1.1.0) - #109
Merged
Merged
Conversation
Closes F13. grep's --exclude-dir=superpowers matches that name at any depth, so the retired-CLI sweep silently exempted every committed spec and plan under docs/superpowers/ and superpowers/. - smoke-test.sh: drop --exclude-dir=superpowers from the sweep (the git-ignored .superpowers/ exclusion stays). The sweep comment now explains the any-depth match and lists the extension's model API as a legitimate tagged mention. - Tag the 14 lines under docs/superpowers/ that name the extension's model API (3 files): prose and html-fence lines get an HTML comment tag, lines in javascript fences get a // tag. The matching mutation case (23) lands with the harness rework in S2. Deviation: the last sentence of the comment's "A tag is for..." paragraph is re-wrapped (same words) so the lengthened first sentence does not leave a 100-column line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes F2 and adds the F13 mutation (case 23). fresh() never copied extension/, so since 2b36bea smoke-test failed on every copy ("chrome extension tests pass") and each smoke-guarded case reported GUARDED while guarding nothing. Cases 16 and 17 had also gone stale: their python assert failed ("anchor not unique: 0"), nothing checked it, and both still counted as GUARDED. - Snapshot: build one $WORK/base from the explicit item list, now including extension; fresh() copies base, so the baseline, the diff target and every case copy are the same tree. - No-op guard: expect_fail aborts the run unless diff -rq (ignoring *.bak) against base exits 1, naming the diff exit code. - Baseline: smoke on an unmutated copy must pass or the run aborts, printing each FAIL with its diagnostic lines. Case 0 proves a no-op mutation is refused. - Cases 16/17 anchor on <h2 id="install-title"> instead of the heading text, with the untagged claim on its own line before it. - Case 23: an untagged claim in docs/superpowers/leak.md must fail smoke. - Header says "each fixed defect" instead of "each bug fixed in v3.3.0.0". Deviation: the re-anchored 16/17 heredocs open docs/index.html with encoding='utf-8' for read and write, per the plan's heredoc style rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds actions/setup-node@v4 (node-version '22') to the test job (all three matrix legs), the mutation job and release.yml, whose smoke run (and so test-extension.sh) had no Node pin. The upcoming .mjs tests rely on ESM syntax detection, which is default only from Node 20.19 / 22.7, and once fresh() copies extension/ every smoke-guarded mutation case depends on the pin too. Adds a direct "Chrome extension unit tests" step to the test job, matching every other suite smoke-test calls; check() truncates a failure to 5 lines, so this is the only place CI shows the full output. Prepares F5 (S4/S5). The release.yml comment gets a one-line note that smoke-test.sh runs test-extension.sh, mirroring the mutation job's note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes F5 part 1: the extension tests loaded hand-maintained .cjs copies,
so a bug in the .js Chrome actually runs stayed green (mutations 24a and
24b both passed the old suite).
- git rm the seven .cjs twins.
- New test/extension-harness.mjs: importFresh, loadContentScript
(vm.Script syntax check, then require), async chrome.storage callbacks,
MV3 promise sendMessage, runtime.id/getURL, dispatch() defaulting to a
side-panel sender, and a document-order fake DOM.
- Port the tests to .mjs importing the shipped modules: test-prompts,
test-crawler (top-level await), test-dossier-compiler (source-grep
Test 4 dropped), test-extractor (fake-DOM page(), Test 10 via
dispatch), test-service-worker.js -> test-parser-helper.mjs,
test-sidepanel-logic.js -> test-renderer-helper.mjs; new
test-course-context.mjs tests the file sidepanel.js imports.
- extractor-core.js keeps only detectCourseContext (path unchanged, so
sidepanel.js is untouched); extractor.js drops its unused copy.
- test-extension.sh: Node >= 20.19/22.7 floor, a no-.cjs guard, and runs
every test-*.mjs.
- CLAUDE.md: test-extension.sh line and the Node 22 CI note.
- Mutations 24a (renderer escaping), 24b (isCourseRoot in
extractor-core.js), 24f (.cjs twin returns).
Deviations: the harness uses the plan's sender identity
(EXTENSION_ID 'idstackextensionidfortests', unexported PANEL_SENDER
{id, url}) rather than U2's exported SIDE_PANEL_SENDER; the CLAUDE.md
comment wraps onto two lines to keep the column-aligned style.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the harness Closes F5 part 2: service-worker.js, storage.js and sidepanel.js never ran under the extension suite (no chrome or DOM stub), so a bug planted in any of them stayed green. Mutations 24c-e each passed the S4 suite. - New test/test-storage.mjs: settings round-trip, 20-entry newest-first history, dossier add / replace-by-url / remove / clear. - New test/test-service-worker.mjs: RUN_AUDIT demo and keyed paths, model API error, CRAWL_AND_AUDIT_COURSE, unknown action. Named assertions that each async action replies. The course fixture has syllabus text, so it survives the later empty-course guard. - New test/test-sidepanel.mjs: loads the shipped index.html and sidepanel.js; course-root button, preloaded key, dossier badge, audit round-trip, escaped error card. - Mutations 24c (RUN_AUDIT drops return true), 24d (dossier replace >= 0 -> > 0), 24e (course-audit button visibility swapped). Deviations: the tests are U2 section 4 verbatim except that the storage comment no longer names the internal design unit (U8), and the course fixture carries a one-line comment on why it has syllabus text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes F12. F1 (Canvas while(1); prefix) is refuted: canvas-lms 485acb0a05 removed it in 2019, so there is no Accept header and no prefix strip. - canvas-crawler.js follows Link rel="next" across pages, up to MAX_ASSIGNMENT_PAGES = 10 (500 assignments). A non-OK response, a network error or a non-array body now rejects instead of becoming an empty list, and going past the cap throws a clear message. - prompts.js keeps the 20,000-char assignment budget but fills it with whole assignment blocks only, and the header now reads "(N items; M shown in full below)" (plan override 3). - test-crawler.mjs: a Canvas-shaped fake (real Response/Headers, Link pagination, JSON 4xx) covering two-page reads, 403, a page-2 network failure and the page cap. test-prompts.mjs: an 80-assignment course states "80 items", states the shown count and never cuts a block. - Mutations 25a-25d. Deviations from the step spec: - Mutation 25d uses two anchors: it drops the budget's break and restores the plain .slice(0, 20000). With the whole-block budget the slice alone changes nothing, so the test still passes (checked). - prompts.js also changes the summary build and the header line (override 3), not only L59. S7 does not own either hunk. - The MAX_ASSIGNMENT_PAGES comment no longer says the cap bounds the prompt, because the prompt now has its own budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eferences.md Closes F15. The extension's evidence labels disagreed with evidence/references.md and CLAUDE.md: - TIER_METADATA put randomized trials in T2; it now uses the CLAUDE.md labels and the references.md definitions (DESIGN.md colours unchanged). - EVIDENCE_DOMAINS is deleted with its prompts.js import and re-export: it had no runtime consumer, non-canonical codes, and cited studies that are not in references.md. - Both prompts now carry TIER_SCALE, built from TIER_METADATA. The page prompt asks for critical|warning|info, not 'suggestion'. The course prompt's citations are coded: [Alignment-1] Biggs (1996) [T5], [CogLoad-4] Sweller (1994) [T5], [CogLoad-1] Costley et al. (2023) [T1] and [Assessment-8] Wisniewski et al. (2020) [T1]. That replaces Sweller 2011 and Wood 1976, which are not in references.md, and drops Liou. - Demo findings: Biggs is [Alignment-1] T5, not [Alignment-3] T2. [Cognitive-2], which does not exist, is now [CogLoad-1]. Carpenter (2022), not in references.md, is now [CogLoad-6] Chen et al. (2018) T1. - sidepanel.css drops the off-vocabulary severity-suggestion tokens and rule. New test/test-evidence-labels.mjs parses references.md and CLAUDE.md and checks the three shipped files, the demo results, the built prompts and the side-panel CSS against them. test-prompts.mjs drops the EVIDENCE_DOMAINS and 'Meta-analysis' assertions; test-parser-helper.mjs now expects a T5 demo badge instead of T2. Mutations 26a-26g reintroduce each defect. Deviations from the spec: - The test collects every disagreement and fails once, listing them all, where U9 stopped at the first assert. On the unfixed tree it names [Cognitive-2], Sweller 2011, Wood 1976, Carpenter 2022 and the uncited EVIDENCE_DOMAINS studies in one run. - The CSS check also covers the --severity-*/--sev-* tokens, not only the .finding-card rules, because the plan removes both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d the fetch, key in header Closes F7 (worker side, plus the same defect on the course path), F9 (worker side) and F10. - RUN_AUDIT refuses content under MIN_AUDIT_WORDS = 10 before the model or demo path, and shows payload.emptyReason when the extractor or panel supplies one. - CRAWL_AND_AUDIT_COURSE refuses a course with no syllabus text and no assignments, right after the crawl (demo mode too). - The model call sends the key in an x-goog-api-key header, not the URL, and is bounded by AbortSignal.timeout(25 s); a timeout is reported plainly by checking signal.aborted (Chrome < 124 says AbortError). - Model output must have findings as an array of objects with string or missing tier and severity before it is saved or returned. - test-service-worker.mjs: 13-word payload, one AbortSignal.timeout spy, refusal, transport, timeout, pass-through and malformed-shape scenarios; every refusal asserts empty history. - Mutations 27a-27f. Deviations: - 27e deletes `request.payload?.emptyReason || ` instead of inserting a literal '' there, which would make a tagged template that throws and trips the suite for the wrong reason. - The fetch .catch uses a multi-line form (as in the U5 design) rather than the spec's compressed one; no mutation anchors on it. - The emptyReason scenario uses a realistic reason string, asserted by strict equality, instead of 'X'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tabs via activeTab
Closes F3 and F4 (decision D1 = A).
F3: the service worker answered any sender and spliced the raw origin and
courseId into credentialed Canvas fetches. Its listener now ignores any
sender whose URL is not one of the extension's own pages (a content script
shares sender.id), and CRAWL_AND_AUDIT_COURSE accepts only a bare https
origin and a numeric course id.
F4: the <all_urls> content script is gone ("all your data on all websites"
at install, and its match also widened the worker's CORS allowlist). The
icon click now reaches chrome.action.onClicked (openPanelOnActionClick is
reset to false, since Chrome's toggle path skips the activeTab grant),
opens the panel without an await, and tells an open panel to re-read the
tab. The panel injects content/extractor.js with executeScript and reads
the script's completion value; an unreadable tab carries an emptyReason
that the worker shows. Custom-domain Canvas hosts are requested per site
(optional_host_permissions https://*/*) inside the Audit Entire Course
click, before any await. minimum_chrome_version 116 for sidePanel.open.
extractor.js no longer registers an onMessage listener (which returned
true for every message).
Tests: test-manifest.js, test-extractor.mjs (Test 10: vm injection run
twice in one world), test-service-worker.mjs (sender gate, five invalid
targets, custom-domain crawl, panel behaviour, onClicked) and
test-sidepanel.mjs (injection, synchronous permission request, denial,
TAB_ACCESS_GRANTED, unreadable tab). The harness gains action, sidePanel,
permissions and scripting stubs and drops tabs.sendMessage/onTabMessage.
Mutation cases 28a-28n.
Deviations from the spec: the TAB_ACCESS_GRANTED listener sits inside the
file's usual `typeof chrome` guard (the pinned lines are unchanged, indented
two more spaces; 28n still anchors uniquely). Test 10 reuses the harness
chrome stub and the Test 1 assignment fixture instead of U4's hand-built
stub. The side-panel permission scenarios use a custom-domain course root
(canvas.asu.edu), the case the optional host permission exists for.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… read Docs via export Closes F6, F11 (content side) and F7 (Google Docs). - Modules pages (including a course home in Modules view, #context_modules) now send every .module-item-title, not only the first. - A URL-string guard refuses Canvas student-record views (gradebook and SpeedGrader, grades, People at course and account level, groups, discussions, submissions, Inbox) with content '' and an emptyReason. Canvas pages no longer fall back to document.body; #rubrics joins the content selector. Only numeric /courses/N URLs count as Canvas. - extractPageContent is async. For Google Docs it reads the doc's /export?format=txt with fetch's default credentials mode and a 10 s timeout, accepts only text/plain, sets emptyReason on failure, and skips published /d/e/ docs. - test-extractor.mjs: Tests 3, 4, 12, 13 and an await in Test 10. Mutations 29a-29i. Deviations: the spec's "content not 'Student records page'" check for an empty url is vacuous (the guard always sets content ''), so the test asserts the title is not 'Student records page' and generic extraction still runs. Test 13 also asserts the export fetch sets no credentials option. The guard regex is held in a function-scoped const (studentRecordUrl); 29d anchors on its if-line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e on error; Retry/votes/clipboard; defensive renderer Closes review F8, F14 and the renderer side of F9. - F8: a page audit is labelled with the payload that was sent (const sent), not the tab active when the reply lands; a course audit is labelled origin/courses/id with response.courseData.title, matching the worker's auditHistory entry. refreshActiveTab takes a sequence number and drops a stale refresh after tabs.query, after executeScript and before the fallback, so a slow tab cannot overwrite a newer one. - F14: renderError clears activeAuditItem and hides the result action bar (renderResults shows it again); Retry re-clicks the button that started the failed audit (D12 = A; set synchronously, before the course handler's permissions.request), unless a tab switch has hidden that button (no course root), so Retry never crawls, or asks for access to, a non-course page; a vote removes only the vote buttons, keeping Audit Another Page; both copy buttons go through copyWithFeedback, which reports "Copy failed" and never rejects. - F9 (renderer): non-array findings render as none, null and non-object entries are dropped, and a non-string tier is stringified. Tests: new test/test-sidepanel-state.mjs (eleven panel scenarios on the shipped sidepanel.js; ten fail on the previous tree, the eleventh guards the hidden-button check on Retry; the course Retry scenario counts the crawls sent, so a Retry that sends nothing fails) and a Test 9 block in test-renderer-helper.mjs. Harness: FakeElement.remove(); click() returns a promise and collects listener errors in clickErrors, and an exit hook fails any suite that leaves clickErrors non-empty; tabs may be a function. Mutations 30a-30l (guarded 86 on Python 3.9). Deviations: - test-sidepanel-state.mjs reports every failing scenario and then asserts none failed, instead of stopping at the first one; it still ends with assert.deepStrictEqual(clickErrors, []) and process.exit(0). - New fixtures cite [Alignment-1] at T5 and [CogLoad-1] instead of U7's T1/T2 and [CogLoad-2], so no new tier mismatch with references.md. - Course scenarios flush after the click: S9's handler awaits permissions.request before it sends (U7 predates S9). - Retry skips a hidden Audit Entire Course button. U7 accepted that residual as "an error card", but since S9 the click raises a host permission prompt for whatever https site is active. No counted mutation for the guard, to keep the pinned count at 86; removing it fails the new scenario. - The harness exit hook is not in the spec. Since click() catches listener errors, suites that never read clickErrors (test-sidepanel.mjs) would otherwise pass a rejecting handler that used to crash them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes F11 (docs side) and the course-crawler overclaim; also the README "dashboard", "JSON" and "quizzes" claims and the undisclosed Google Fonts request (D11). - Side panel: the demo line says every audit shows the same sample findings and nothing is sent; "Privacy & Student Data" says what is sent to Google with a key, what Google's free tier allows, and which Canvas pages are refused. No FERPA or no-PII claim. The AI Studio link line is unchanged. - PRIVACY.md: a "What it reads" bullet matching S9/S10, chrome.storage.sync for the key, the last-20 audit history, the free-tier terms (tagged link), the Fonts request, plugin-scoped network sentences, new date. - README: tab-switch and custom-domain notes, crawler bullet limited to the syllabus and assignments, no dashboard/JSON/quizzes, demo and key setup text, the FAQ covers the extension. - test/test-disclosures.mjs ties each claim to the shipped storage.js, parser-helper.js, canvas-crawler.js and extractor.js. It fails 15 checks on the old docs. Mutations 31a-31i; PRIVACY.md joins the snapshot list. Deviations: the test collects every disagreement before asserting (the test-evidence-labels.mjs pattern); the README custom-domain note follows the Full Course Audit bullets, not the colon that introduces them; the tab-switch note also exempts Canvas sites already allowed. PRIVACY.md, README notes, the date and the Fonts line are prose reviewed by reading. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cuts the release for review findings F2-F15 (F1 refuted; no claim made). - Version 3.5.1.0 in VERSION, .claude-plugin/plugin.json, the README header and docs/index.html (softwareVersion, hero string, and a "patched through v3.5.1.0" note on the v3.5.0.0 card). - CHANGELOG: new top entry grouped as site access; student-record refusal, Modules and Docs; course crawl; empty pages/courses, malformed output, timeout and key header; side-panel state; evidence labels; privacy promises (with the "Correction to the v3.5.0.0 notes" bullet); For contributors. The v3.5.0.0 entry is untouched. - Extension manifest 1.1.0 and side-panel badge v1.1. test-manifest.js asserts the badge equals v<major>.<minor> of the manifest version. - bin/package-extension.sh names the zip from manifest.json (node -p) and excludes icons/generate-icons.js; README's packaging line follows. - Mutation 32: badge drift (v1.1 -> v1.0) must fail test-extension.sh. Deviations: - 3.5.1.0, not the spec's 3.5.0.1 (plan override 1). - Failing first: on the unbumped tree the badge and manifest agree, so the new assertion was shown failing by bumping the manifest alone (badge v1.0 vs 1.1.0), then passing once the badge moved to v1.1. - README's packaging line keeps its "ready for submission" clause and is now L230 (S12 shifted it); the refusal of student-record pages is described once in the CHANGELOG and the privacy bullet points to it. - The zip name and the generate-icons.js exclusion stay unguarded by a suite (spec exception); verified once with package-extension.sh and unzip -l. The built zip is in the git-ignored build/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f50b6e75e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+238
to
+239
| if (lastAuditBtn && lastAuditBtn.style.display === 'none') return; | ||
| if (lastAuditBtn) lastAuditBtn.click(); |
There was a problem hiding this comment.
If a course audit for course A fails and the user switches to course B's root or Modules page before clicking Retry, the course button remains visible, so this guard passes and lastAuditBtn.click() invokes the handler using B's current activeCourseContext. The retry therefore requests permission for and audits course B rather than repeating the failed audit of A; capture and replay the failed request's { origin, courseId } instead of only retaining its button.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 2fa2f2e. The course audit now records the course it started on (lastCourseTarget). Retry returns to the ready screen, without crawling anything, when the course now shown is a different one. Covered by the new scenario "Retry after a switch to another course does not audit that course (F14)" in test/test-sidepanel-state.mjs, which fails without the guard, and by mutation 30n.
Owner
Author
There was a problem hiding this comment.
Follow-up in 6a6ca78: this guard replaced the older hidden-button guard, which it made redundant (CI's mutation job showed the overlap). On another page of the same course, Retry now re-runs that course's audit. Mutation 30m covers the guard.
This branch was cut from a local main 23 commits behind origin. The merge brings in main's v3.5.1.0 (responsive landing page, mutation-suite repair), bot PRs #74-#83, and #89 (Consensus evidence engine). Resolution: - Extension code keeps this branch's structure. From main it re-applies #89's Consensus pass, BYOK key field and badge, #82's container-scoped vote query, #83's rel="noopener noreferrer", and #76's Date fix. #81 and this branch both moved the Gemini key into a header. - The .cjs twins stay deleted, and so does #89's consensus-client.cjs. test-consensus-extension.js becomes test-consensus-client.mjs and imports the shipped module. #74/#80's crawler error paths move into test-crawler.mjs. - The Consensus badge referenced --sev-suggestion-bg, which this branch removed with the suggestion severity. It now uses its own --consensus-bg with the same colors. - manifest.json adds api.consensus.app to this branch's host list. - mutation-test keeps this branch's snapshot, baseline and no-op checks. Main's landing-page cases are appended as 33-42. - smoke-test drops the superpowers exclusion again (F13) and keeps main's designs exclusions and consensus CLI check. It no longer runs the consensus client test separately, because test-extension.sh already runs every test-*.mjs. - Version: main already carries v3.5.1.0, and idstack.org announces it, so this release is v3.6.0.0. Its CHANGELOG entry also covers #89, which shipped without one. The extension stays at 1.1.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#89 made both halves of idstack send finding text to api.consensus.app when a Consensus key is set, and added that host to the extension's host_permissions. None of the privacy docs said so: PRIVACY.md and README still listed Canvas and the update check as the plugin's only network traffic, and the side panel described only Google. - PRIVACY.md: a Consensus API entry under Third-party services (where the key is read from, what is sent, the ~/.idstack/cache/consensus/ cache, and why a key in project.json travels with the project), plus an extension bullet (what is sent after each audit, the chrome.storage.local cache, and demo findings still being checked). The API key bullet now covers both keys in chrome.storage.sync. - The side panel's privacy note and the Consensus key help text say that each finding's claim goes to Consensus. README's "Is my data safe?" answer does the same. - test-disclosures.mjs: every host_permissions host must be named in PRIVACY.md's extension section, and a Consensus key field requires the panel's note to mention Consensus. Both checks fail on the previous docs. Mutation 31j proves the host check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
These PRs targeted parser-helper.cjs, test-service-worker.js and test-extractor.js, all of which this branch removed, so they cannot merge. Their cases that add coverage now live in the ESM suites: - test-parser-helper.mjs: JSON arrays, whitespace with no fence, and a SyntaxError for a trailing comma, truncated JSON, or plain text (#99, #101). - test-course-context.mjs: null and non-URL input, a #fragment on a course root, a non-Canvas path, and a missing or non-numeric course id (#100). #108's one real fix, `cut` in test-doctor's mock PATH, already reached this branch from main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Consensus disclosure rewrote PRIVACY.md's API key bullet to cover both keys, so 31d's anchor stopped matching. expect_fail's no-op check caught this on CI and stopped the run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…that failed Codex review on #109: after a course audit fails, a switch to another course's root or Modules page keeps Audit Entire Course visible. The hidden-button guard (mutation 30m) passes, and Retry re-clicked the button, which audits the course now active. Retry now also returns to the ready screen when the current course is not the one that failed. Covered by a new test-sidepanel-state scenario, which fails without the guard, and by mutation 30n. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
This was referenced Sep 24, 2026
smoke-test's retired-CLI sweep flagged the reviewer bot's name in the comment added with 30n. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The course-target guard added in 2fa2f2e also covers the switch to a non-course tab. That left the older hidden-button guard with one effect of its own: on another page of the same course it stopped Retry from re-running that course's audit, which is what Retry is for. CI's mutation job showed the overlap: removing the old guard (30m) no longer failed any test. Retry now has a single guard. It proceeds only when the course now shown is the one that failed. Mutation 30m targets that guard and fails both switch scenarios, so 30n is folded into it. A new scenario pins the same-course behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
savvides
changed the title
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.
Update, 2026-09-24: merged with main, now v3.6.0.0
This branch was cut from a local
mainthat was 23 commits behindorigin/main. That is why GitHub marked it conflicting and why it reused the version number v3.5.1.0. It now includes main, and the release is v3.6.0.0:Seven commits were added on top of the 13 reviewed ones:
consensus-client.cjstwin is deleted, and its test now imports the shipped module.--sev-suggestion-bg, which this PR removed along with thesuggestionseverity. It now has its own--consensus-bgtoken with the same colors.api.consensus.appis added to this PR's host list.api.consensus.app, but PRIVACY.md, README and the side panel still named only Google. They now describe Consensus too.test-disclosures.mjsfails if ahost_permissionshost is missing from PRIVACY.md, and mutation 31j proves that check.Summary
This PR fixes the findings from a code review of
v3.4.0.1..HEAD(the Chrome extension,bin/and test changes), then cuts v3.6.0.0 (first drafted as v3.5.1.0; see the update above), which ships extension 1.1.0. There is one commit per step, in dependency order: the test guards are proven first, then the product fixes land, then the docs and release.Review findings: 15 in total (2 blocking, 11 moderate, 2 minor). 14 are fixed. One (F1) was refuted, and no code change is needed for it.
while(1);. Canvas removed that prefix in canvas-lms 485acb0, so the crawler is unchanged. Manual check 5 below is a tripwire in case an instance still sends it.mutation-test.shnever copiedextension/. Since 2b36bea, smoke-test has therefore failed on every mutated copy, and every smoke-guarded case was reported GUARDED without guarding anything.openPanelOnActionClickdoes not grant activeTab (crbug 1453437). The panel is now opened fromaction.onClicked.Commits
test: sweep committed docs/superpowers for retired-CLI claimstest: make mutation-test prove its guardsci: pin Node 22 wherever the extension suite runstest(extension): import the shipped ESM; retire .cjs twinstest(extension): execute service worker, storage and side panel under the harnessfix(extension): crawl every page of assignments and fail loudlyfix(extension): align evidence tiers, citations and severities with references.mdfix(extension): refuse empty pages/courses, validate model JSON, bound the fetch, key in headerfix(extension): gate SW messages to the panel; drop <all_urls>; read tabs via activeTabfix(extension): read every Modules item, refuse student-record pages, read Docs via exportfix(extension): label results with the audited page; clear stale state on error; Retry/votes/clipboard; defensive rendererdocs: make the extension's privacy and capability claims match the coderelease: v3.5.1.0 (extension 1.1.0)Merge origin/main into fix/review-remediation; release as v3.6.0.0docs: disclose what the Consensus integration sendstest: port useful cases from closed bot PRs #99 #100 #101test: re-anchor mutation 31d on the two-key storage sentencefix(extension): Retry does not audit a different course than the one that failedtest: keep a retired CLI's name out of mutation 30n's commentfix(extension): one Retry guard, keyed on the course that failedDecisions
<all_urls>content script is removed.instructure.comstays a required host, so Canvas tabs there are still read automatically.optional_host_permissions.minimum_chrome_versionis now 116.chrome.storage.sync, and PRIVACY.md now says so.Test plan
These were re-run after the merge. Local runs are on macOS with Python 3.9.6 and Node 24; the rendered-landing suite cannot launch Chrome in this environment, so that one check is verified on CI.
smoke-test365/365 on CI (364/365 locally; the one failure is the Chrome launch described above)integration-test51/51test-setup17/17test-doctor13/13test-status22/22test-manifest-merge23/23test-version-classifier27/27test-plugin-status9/9test-preamble-python6/6test-extensionpassescheck-evidence-cards,check-doc-accuracyandtest-consensus-cli.pypass;idstack-gen-skills --dry-runreports 0 stalemutation-test.shon CI: guarded 108, NOT guarded 0, skipped 0 (main had 36 cases and this PR's pre-merge tree had 97). Main's 10 landing-page cases are included. The no-op guard caught one stale anchor (31d), and the mutation job caught one redundant Retry guard; both are fixed..cjsfiles remain. The retired-CLI grep is clean.bin/package-extension.shbuildsbuild/idstack-chrome-extension-v1.1.0.zip.manifest.jsonis at the root; there are no.cjsfiles and nogenerate-icons.js.Manual Chrome checks before the Web Store upload. Load
extension/unpacked; any failure blocks the upload.c6c5957tree unpacked, swap in this branch and reload. The icon opens the panel and reads the tab./assignmentsrequests;Unexpected token 'w'(the F1 tripwire);/d/e/doc is not fetched.This also confirms that Chrome waits for the injection's Promise result.
CRAWL_AND_AUDIT_COURSEgets no response. Audits from the panel still work.api.consensus.apponly for claims that are not yet cached;Web Store tasks (manual):
Follow-ups (not in this PR)
Extension behavior
finishReasonis unchecked, and raw API or parse errors reach the user.storage.jsloses updates when get-modify-set calls overlap.auditHistoryis written but never read.apiEndpointandautoAuditsettings are dead.courseDatawhen only the title is used.Consensus (#89, found during the merge)
consensus_meter,consensus-client.jssets the meter to 85, and a cached record without a meter gets 88. The badge then shows "✓ 85% Consensus" for a number nobody measured.Evidence text
improvedDraftstill has bare[T1]/[T2]tags.CONTRIBUTING.md:107has an untested copy of the tier table.Tests and tooling
[ -x ]hooks can skip a suite silently.test-rendered-landing.jsgives Chrome 12 s to open its debugging port. On CI runners it ran out several times during this PR (in the test and mutation jobs), and reruns passed. Consider a longer window, or a retry, before it starts blocking main.v1.1; the next version bump must update it. It fails loudly if forgotten.severity: 'suggestion'and T2[Alignment-3].🤖 Generated with Claude Code