From 01b3a696f670e86e1a0d1e45dd3c3326e3f90688 Mon Sep 17 00:00:00 2001 From: LTDigor Date: Wed, 29 Jul 2026 01:42:23 +0300 Subject: [PATCH 1/4] Add session-local popup undo for freshly added rules --- .../task-1-report.md | 59 +++++++ src/popup/popup.css | 11 ++ src/popup/popup.js | 38 +++-- tests/unit/popup-css.test.js | 6 + tests/unit/popup-current-site.test.js | 158 ++++++++++++++++++ 5 files changed, 258 insertions(+), 14 deletions(-) create mode 100644 .superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md diff --git a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md new file mode 100644 index 0000000..f03d40a --- /dev/null +++ b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md @@ -0,0 +1,59 @@ +# Task 1 report: session-local undo for newly added rules + +## Scope implemented +- Added session-local tracking for rules added during the popup session. +- Freshly added rules are rendered with `Undo` and an accent style class. +- Freshly added rule removals now call the existing `removeBlockedSite(...)` path directly. +- Existing/preloaded rules keep old `×` remove button and challenge-flow behavior. +- Fresh rule set membership is added only after storage write success and removed only after successful deletion. + +## Files changed +- `src/popup/popup.js` + - Added `freshRules` session `Set`. + - Rendered fresh rules with `Undo` + `remove-button--undo` class. + - Re-routed fresh-rule removal through `removeBlockedSite(...)` and clear membership only on successful promise resolution. + - Inlined add flow so new rule is added to storage, then marked fresh, then rendered. +- `src/popup/popup.css` + - Added `.remove-button--undo` and `.remove-button--undo:hover` for distinct accent styling. +- `tests/unit/popup-current-site.test.js` + - Added focused tests for: + - immediate Undo removal of fresh rules + - multiple fresh rules each undoable independently + - preloaded/duplicate rules still using challenge removal path + - temporary unblock cleanup on fresh-rule removal + - re-adding a removed fresh rule +- `tests/unit/popup-css.test.js` + - Added assertion for `.remove-button--undo` style presence. + +## RED step (focused tests first) +Command: +```bash +node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js +``` +Observed failures (expected before fix): +- `freshly added rules can be undone immediately without challenge` — expected `Undo`, got `×` +- `multiple fresh rules each get Undo controls and can be undone independently` — expected `Undo`, got `×` +- `a re-added rule is fresh again and stays undoable` — expected empty list after undo, got `['example.com']` + +## GREEN step (focused) +Command: +```bash +node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js +``` +Result: 30 passed, 0 failed. + +## Full suite +Command: +```bash +node --test +``` +Result: +- tests: 98 +- pass: 98 +- fail: 0 + +## Self-review +- Risk: session-local `freshRules` is intentionally in-memory only and resets on popup reload. That matches task scope. +- Risk: existing behavior for reloading the popup while a rule is fresh is intentionally unchanged (it becomes a normal `×` rule on reload). +- No storage schema changes introduced. +- No unrelated files or features were modified. diff --git a/src/popup/popup.css b/src/popup/popup.css index 3d7d8a1..cf30ffd 100644 --- a/src/popup/popup.css +++ b/src/popup/popup.css @@ -468,6 +468,17 @@ h2 { color: var(--danger); } +.remove-button--undo { + background: rgba(37, 99, 235, 0.1); + color: var(--accent); + font-size: 12px; + font-weight: 800; +} + +.remove-button--undo:hover { + background: rgba(37, 99, 235, 0.2); +} + .confirm-dialog { width: min(318px, calc(100vw - 28px)); padding: 0; diff --git a/src/popup/popup.js b/src/popup/popup.js index 75b08c1..71953c5 100644 --- a/src/popup/popup.js +++ b/src/popup/popup.js @@ -65,6 +65,7 @@ let currentUnblockChallenge = null; let currentBlockedRule = null; let currentBlockedUrl = null; let currentTabId = null; +let freshRules = new Set(); function createMemoryStorage() { let state = { blockedSites: [], temporaryUnblocks: {} }; @@ -138,13 +139,22 @@ function render(sites, temporaryUnblocks = currentTemporaryUnblocks) { }; const remove = documentRef.createElement("button"); - remove.className = "remove-button"; + const isFreshRule = freshRules.has(site); + remove.className = isFreshRule ? + "remove-button remove-button--undo" : + "remove-button"; remove.type = "button"; - remove.textContent = "×"; + remove.textContent = isFreshRule ? "Undo" : "×"; remove.setAttribute("aria-label", `Remove ${site}`); remove.title = "Remove blocked site"; remove.onclick = () => { + if (isFreshRule) { + return removeBlockedSite(site).then(() => { + freshRules.delete(site); + }); + } + openRemoveDialog(site); }; @@ -184,13 +194,6 @@ function getCurrentUnblockableSite(sites) { return null; } -function save(sites, callback) { - return storage.set({ blockedSites: sites }).then(() => { - render(sites, currentTemporaryUnblocks); - return callback?.(); - }); -} - function removeBlockedSite(site) { const nextSites = [...currentSites]; const nextTemporaryUnblocks = { ...currentTemporaryUnblocks }; @@ -533,14 +536,21 @@ function saveRule(rule, successMessage, duplicateMessage, callback) { return callback?.(); } - return save([...sites, rule], () => { - input.value = ""; - setHint(successMessage); - return callback?.(); - }); + const nextSites = [...sites, rule]; + + return storage + .set({ blockedSites: nextSites }) + .then(() => { + freshRules.add(rule); + render(nextSites, currentTemporaryUnblocks); + input.value = ""; + setHint(successMessage); + return callback?.(); + }); }); } + async function openBlockPage(tabId, site, blockedUrl = "") { if (!tabId) return; diff --git a/tests/unit/popup-css.test.js b/tests/unit/popup-css.test.js index c98ba78..945dadf 100644 --- a/tests/unit/popup-css.test.js +++ b/tests/unit/popup-css.test.js @@ -33,6 +33,12 @@ test("popup renders a compact custom-image summary", async () => { ); }); +test("fresh rule remove action uses undo styling", async () => { + const css = await readFile(new URL("../../src/popup/popup.css", import.meta.url), "utf8"); + + assert.match(css, /\.remove-button--undo\s*{[^}]*background:\s*rgba\(37,\s*99,\s*235,\s*0\.1\);/s); +}); + function getRule(css, selector) { const match = css.match(new RegExp(`${selector}\\s*{([^}]*)}`)); diff --git a/tests/unit/popup-current-site.test.js b/tests/unit/popup-current-site.test.js index 8e70c2d..8f75534 100644 --- a/tests/unit/popup-current-site.test.js +++ b/tests/unit/popup-current-site.test.js @@ -232,6 +232,164 @@ test("popup falls back to the default preview when saved image data is invalid", assert.equal(elements.imageHint.classList.has("is-error"), true); }); +test("freshly added rules can be undone immediately without challenge", async (t) => { + const { elements, state, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + } + }); + t.after(cleanup); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + const removeButton = elements.list.children[0].children[2]; + assert.equal(removeButton.textContent, "Undo"); + assert.equal(removeButton.className.includes("remove-button--undo"), true); + assert.equal(elements.unblockDialog.open, false); + + removeButton.onclick(); + await Promise.resolve(); + + assert.deepEqual(state.blockedSites, []); + assert.equal(elements.unblockDialog.open, false); + assert.equal(elements.formHint.textContent, "Blocked site removed."); +}); + +test("multiple fresh rules each get Undo controls and can be undone independently", async (t) => { + const { elements, state, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + } + }); + t.after(cleanup); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + elements.siteInput.value = "openai.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + assert.deepEqual(state.blockedSites, ["example.com", "openai.com"]); + + const firstRemoveButton = elements.list.children[0].children[2]; + const secondRemoveButton = elements.list.children[1].children[2]; + + assert.equal(firstRemoveButton.textContent, "Undo"); + assert.equal(secondRemoveButton.textContent, "Undo"); + + firstRemoveButton.onclick(); + await Promise.resolve(); + + assert.deepEqual(state.blockedSites, ["openai.com"]); + assert.equal(elements.list.children[0].children[2].textContent, "Undo"); +}); + +test("preloaded and duplicate rules retain the challenge-dialog removal path", async (t) => { + const { elements, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + }, + initialState: { + blockedSites: ["example.com"] + } + }); + t.after(cleanup); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + const removeButton = elements.list.children[0].children[2]; + assert.equal(removeButton.textContent, "×"); + assert.equal(elements.formHint.textContent, "This site is already in the list."); + + removeButton.onclick(); + await Promise.resolve(); + await Promise.resolve(); + + assert.equal(elements.unblockDialog.open, true); + assert.equal(elements.challengeDialogTitle.textContent, "Remove blocked site?"); +}); + +test("removing a fresh rule clears temporary unblock state without challenge", async (t) => { + const now = 1_700_000_000_000; + const { elements, state, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + }, + now + }); + t.after(cleanup); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + elements.currentUnblockBtn.onclick(); + await Promise.resolve(); + await Promise.resolve(); + answerCurrentChallenge(elements); + await submitUnblockForm(elements); + + assert.deepEqual(state.temporaryUnblocks, { + "example.com": now + 5 * 60 * 1000 + }); + assert.equal(elements.currentBlockStatus.textContent.startsWith("Unblocked until "), true); + + const removeButton = elements.list.children[0].children[2]; + removeButton.onclick(); + await Promise.resolve(); + + assert.deepEqual(state.temporaryUnblocks, {}); + assert.deepEqual(state.blockedSites, []); + assert.equal(elements.unblockDialog.open, false); +}); + +test("a re-added rule is fresh again and stays undoable", async (t) => { + const { elements, state, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + } + }); + t.after(cleanup); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + const removeButton = elements.list.children[0].children[2]; + removeButton.onclick(); + await Promise.resolve(); + + assert.deepEqual(state.blockedSites, []); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + const readdRemoveButton = elements.list.children[0].children[2]; + assert.equal(readdRemoveButton.textContent, "Undo"); + readdRemoveButton.onclick(); + await Promise.resolve(); + assert.deepEqual(state.blockedSites, []); +}); + test("temporarily unblocking a blocked site confirms with block image preview", async (t) => { const now = 1_700_000_000_000; const { elements, chrome, state, cleanup } = await setupPopupTest({ From 05129aebbc352fd4518d59dd0bf68c146fb44020 Mon Sep 17 00:00:00 2001 From: LTDigor Date: Wed, 29 Jul 2026 01:44:54 +0300 Subject: [PATCH 2/4] Add popup reopen regression test for fresh rule undo state --- .../task-1-report.md | 17 ++++++++ tests/unit/popup-current-site.test.js | 41 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md index f03d40a..bed3979 100644 --- a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md +++ b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md @@ -57,3 +57,20 @@ Result: - Risk: existing behavior for reloading the popup while a rule is fresh is intentionally unchanged (it becomes a normal `×` rule on reload). - No storage schema changes introduced. - No unrelated files or features were modified. + +## Fix round 2 report: popup reopen regression + +### Changed files +- `tests/unit/popup-current-site.test.js` + +### Command +```bash +node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js +``` + +### Focused test output +- Focused popup tests: 31 passed, 0 failed. +- Added regression: `fresh rule does not stay fresh after reopening popup` (proved persisted rule renders `×` and opens challenge). + +### Coverage +- Reuses existing popup-focused coverage for list render/remove flows, now explicitly covers cross-session reopen behavior for freshly added rules. diff --git a/tests/unit/popup-current-site.test.js b/tests/unit/popup-current-site.test.js index 8f75534..b63b609 100644 --- a/tests/unit/popup-current-site.test.js +++ b/tests/unit/popup-current-site.test.js @@ -390,6 +390,47 @@ test("a re-added rule is fresh again and stays undoable", async (t) => { assert.deepEqual(state.blockedSites, []); }); +test("fresh rule does not stay fresh after reopening popup", async (t) => { + const firstSession = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + } + }); + const { cleanup: cleanupFirstSession } = firstSession; + t.after(() => { + cleanupFirstSession(); + }); + + firstSession.elements.siteInput.value = "example.com"; + await firstSession.elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + assert.deepEqual(firstSession.state.blockedSites, ["example.com"]); + + const { elements, state, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + }, + initialState: firstSession.state + }); + t.after(cleanup); + + const removeButton = elements.list.children[0].children[2]; + assert.equal(removeButton.textContent, "×"); + assert.equal(removeButton.className.includes("remove-button--undo"), false); + + removeButton.onclick(); + await Promise.resolve(); + await Promise.resolve(); + + assert.equal(elements.unblockDialog.open, true); + assert.equal(elements.challengeDialogTitle.textContent, "Remove blocked site?"); + assert.deepEqual(state.blockedSites, ["example.com"]); +}); + test("temporarily unblocking a blocked site confirms with block image preview", async (t) => { const now = 1_700_000_000_000; const { elements, chrome, state, cleanup } = await setupPopupTest({ From a1529b78ea31c43e5dec83938e7446dbbac95c6b Mon Sep 17 00:00:00 2001 From: LTDigor Date: Wed, 29 Jul 2026 01:57:00 +0300 Subject: [PATCH 3/4] Fix fresh Undo a11y, hover color, and failure retry handling --- .../task-1-report.md | 37 +++++++++ src/popup/popup.css | 1 + src/popup/popup.js | 19 +++-- tests/unit/popup-current-site.test.js | 76 +++++++++++++++---- 4 files changed, 114 insertions(+), 19 deletions(-) diff --git a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md index bed3979..87795e2 100644 --- a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md +++ b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md @@ -74,3 +74,40 @@ node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js ### Coverage - Reuses existing popup-focused coverage for list render/remove flows, now explicitly covers cross-session reopen behavior for freshly added rules. + +## Final-fix review round + +### Changes +- `src/popup/popup.js` + - Conditionalized remove button accessibility semantics for fresh rules (`Undo adding `, title `Undo adding this blocked site`) while retaining existing `Remove ` semantics for normal rules. + - Wrapped fresh rule remove flow with error handling to surface storage failures via `setHint(...)`, while keeping fresh status until deletion succeeds. +- `src/popup/popup.css` + - Set `.remove-button--undo:hover` text color to `var(--accent)`. +- `tests/unit/popup-current-site.test.js` + - Added assertions for fresh Undo and normal remove accessibility semantics. + - Awaited fresh Undo button handler promises directly. + - Added regression for storage-rejection on fresh Undo and successful retry behavior. + +### RED verification (focused) +Command: +```bash +node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js +``` +Result before fix: 31 passed, 1 failed (`a failed fresh Undo keeps the rule and retries successfully`) due missing/handled-error behavior on fresh Undo failures (expected hint and retained rule semantics). + +### GREEN verification (focused) +Command: +```bash +node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js +``` +Result: 32 passed, 0 failed. + +### Full suite verification +Command: +```bash +node --test +``` +Result: 100 passed, 0 failed. + +### Coverage +- Focused coverage now includes popup remove-a11y assertions, storage-failure simulation for fresh Undo, and retry path, in addition to existing popup CSS/accessibility checks. diff --git a/src/popup/popup.css b/src/popup/popup.css index cf30ffd..22a80ce 100644 --- a/src/popup/popup.css +++ b/src/popup/popup.css @@ -477,6 +477,7 @@ h2 { .remove-button--undo:hover { background: rgba(37, 99, 235, 0.2); + color: var(--accent); } .confirm-dialog { diff --git a/src/popup/popup.js b/src/popup/popup.js index 71953c5..b1edf0b 100644 --- a/src/popup/popup.js +++ b/src/popup/popup.js @@ -145,14 +145,23 @@ function render(sites, temporaryUnblocks = currentTemporaryUnblocks) { "remove-button"; remove.type = "button"; remove.textContent = isFreshRule ? "Undo" : "×"; - remove.setAttribute("aria-label", `Remove ${site}`); - remove.title = "Remove blocked site"; + remove.setAttribute( + "aria-label", + isFreshRule ? `Undo adding ${site}` : `Remove ${site}` + ); + remove.title = isFreshRule ? + "Undo adding this blocked site" : + "Remove blocked site"; remove.onclick = () => { if (isFreshRule) { - return removeBlockedSite(site).then(() => { - freshRules.delete(site); - }); + return removeBlockedSite(site) + .then(() => { + freshRules.delete(site); + }) + .catch((error) => { + setHint(getErrorMessage(error, "Could not remove blocked site."), true); + }); } openRemoveDialog(site); diff --git a/tests/unit/popup-current-site.test.js b/tests/unit/popup-current-site.test.js index b63b609..371cd7c 100644 --- a/tests/unit/popup-current-site.test.js +++ b/tests/unit/popup-current-site.test.js @@ -249,10 +249,11 @@ test("freshly added rules can be undone immediately without challenge", async (t const removeButton = elements.list.children[0].children[2]; assert.equal(removeButton.textContent, "Undo"); assert.equal(removeButton.className.includes("remove-button--undo"), true); + assert.equal(removeButton["aria-label"], "Undo adding example.com"); + assert.equal(removeButton.title, "Undo adding this blocked site"); assert.equal(elements.unblockDialog.open, false); - removeButton.onclick(); - await Promise.resolve(); + await removeButton.onclick(); assert.deepEqual(state.blockedSites, []); assert.equal(elements.unblockDialog.open, false); @@ -285,9 +286,10 @@ test("multiple fresh rules each get Undo controls and can be undone independentl assert.equal(firstRemoveButton.textContent, "Undo"); assert.equal(secondRemoveButton.textContent, "Undo"); + assert.equal(firstRemoveButton["aria-label"], "Undo adding example.com"); + assert.equal(secondRemoveButton["aria-label"], "Undo adding openai.com"); - firstRemoveButton.onclick(); - await Promise.resolve(); + await firstRemoveButton.onclick(); assert.deepEqual(state.blockedSites, ["openai.com"]); assert.equal(elements.list.children[0].children[2].textContent, "Undo"); @@ -312,10 +314,11 @@ test("preloaded and duplicate rules retain the challenge-dialog removal path", a const removeButton = elements.list.children[0].children[2]; assert.equal(removeButton.textContent, "×"); + assert.equal(removeButton["aria-label"], "Remove example.com"); + assert.equal(removeButton.title, "Remove blocked site"); assert.equal(elements.formHint.textContent, "This site is already in the list."); - removeButton.onclick(); - await Promise.resolve(); + await removeButton.onclick(); await Promise.resolve(); assert.equal(elements.unblockDialog.open, true); @@ -350,8 +353,7 @@ test("removing a fresh rule clears temporary unblock state without challenge", a assert.equal(elements.currentBlockStatus.textContent.startsWith("Unblocked until "), true); const removeButton = elements.list.children[0].children[2]; - removeButton.onclick(); - await Promise.resolve(); + await removeButton.onclick(); assert.deepEqual(state.temporaryUnblocks, {}); assert.deepEqual(state.blockedSites, []); @@ -373,8 +375,7 @@ test("a re-added rule is fresh again and stays undoable", async (t) => { }); const removeButton = elements.list.children[0].children[2]; - removeButton.onclick(); - await Promise.resolve(); + await removeButton.onclick(); assert.deepEqual(state.blockedSites, []); @@ -385,8 +386,7 @@ test("a re-added rule is fresh again and stays undoable", async (t) => { const readdRemoveButton = elements.list.children[0].children[2]; assert.equal(readdRemoveButton.textContent, "Undo"); - readdRemoveButton.onclick(); - await Promise.resolve(); + await readdRemoveButton.onclick(); assert.deepEqual(state.blockedSites, []); }); @@ -421,9 +421,10 @@ test("fresh rule does not stay fresh after reopening popup", async (t) => { const removeButton = elements.list.children[0].children[2]; assert.equal(removeButton.textContent, "×"); assert.equal(removeButton.className.includes("remove-button--undo"), false); + assert.equal(removeButton["aria-label"], "Remove example.com"); + assert.equal(removeButton.title, "Remove blocked site"); - removeButton.onclick(); - await Promise.resolve(); + await removeButton.onclick(); await Promise.resolve(); assert.equal(elements.unblockDialog.open, true); @@ -431,6 +432,53 @@ test("fresh rule does not stay fresh after reopening popup", async (t) => { assert.deepEqual(state.blockedSites, ["example.com"]); }); +test("a failed fresh Undo keeps the rule and retries successfully", async (t) => { + const { elements, chrome, state, cleanup } = await setupPopupTest({ + activeTab: { + id: 42, + url: "https://example.com/" + } + }); + t.after(cleanup); + + elements.siteInput.value = "example.com"; + await elements.ruleForm.onsubmit({ + preventDefault() {} + }); + + const removeButton = elements.list.children[0].children[2]; + const originalSet = chrome.storage.local.set; + const originalLastError = chrome.runtime.lastError; + let shouldFail = true; + + chrome.storage.local.set = (nextState, callback) => { + if (shouldFail) { + shouldFail = false; + chrome.runtime.lastError = { message: "Storage unavailable" }; + callback?.(); + return; + } + + return originalSet(nextState, callback); + }; + + await removeButton.onclick(); + + assert.equal(elements.formHint.textContent, "Storage unavailable"); + assert.equal(elements.formHint.classList.has("is-error"), true); + assert.equal(removeButton.textContent, "Undo"); + assert.equal(removeButton["aria-label"], "Undo adding example.com"); + assert.deepEqual(state.blockedSites, ["example.com"]); + + chrome.storage.local.set = originalSet; + chrome.runtime.lastError = originalLastError; + await removeButton.onclick(); + + assert.deepEqual(state.blockedSites, []); + assert.equal(elements.formHint.classList.has("is-error"), false); + assert.equal(elements.formHint.textContent, "Blocked site removed."); +}); + test("temporarily unblocking a blocked site confirms with block image preview", async (t) => { const now = 1_700_000_000_000; const { elements, chrome, state, cleanup } = await setupPopupTest({ From dfda61ae751707152c68af08ff43da5227d48f8d Mon Sep 17 00:00:00 2001 From: LTDigor Date: Wed, 29 Jul 2026 02:01:14 +0300 Subject: [PATCH 4/4] Remove development review artifact --- .../task-1-report.md | 113 ------------------ 1 file changed, 113 deletions(-) delete mode 100644 .superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md diff --git a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md b/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md deleted file mode 100644 index 87795e2..0000000 --- a/.superpowers/sdd/site-blocker-fresh-rule-undo-plan/task-1-report.md +++ /dev/null @@ -1,113 +0,0 @@ -# Task 1 report: session-local undo for newly added rules - -## Scope implemented -- Added session-local tracking for rules added during the popup session. -- Freshly added rules are rendered with `Undo` and an accent style class. -- Freshly added rule removals now call the existing `removeBlockedSite(...)` path directly. -- Existing/preloaded rules keep old `×` remove button and challenge-flow behavior. -- Fresh rule set membership is added only after storage write success and removed only after successful deletion. - -## Files changed -- `src/popup/popup.js` - - Added `freshRules` session `Set`. - - Rendered fresh rules with `Undo` + `remove-button--undo` class. - - Re-routed fresh-rule removal through `removeBlockedSite(...)` and clear membership only on successful promise resolution. - - Inlined add flow so new rule is added to storage, then marked fresh, then rendered. -- `src/popup/popup.css` - - Added `.remove-button--undo` and `.remove-button--undo:hover` for distinct accent styling. -- `tests/unit/popup-current-site.test.js` - - Added focused tests for: - - immediate Undo removal of fresh rules - - multiple fresh rules each undoable independently - - preloaded/duplicate rules still using challenge removal path - - temporary unblock cleanup on fresh-rule removal - - re-adding a removed fresh rule -- `tests/unit/popup-css.test.js` - - Added assertion for `.remove-button--undo` style presence. - -## RED step (focused tests first) -Command: -```bash -node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js -``` -Observed failures (expected before fix): -- `freshly added rules can be undone immediately without challenge` — expected `Undo`, got `×` -- `multiple fresh rules each get Undo controls and can be undone independently` — expected `Undo`, got `×` -- `a re-added rule is fresh again and stays undoable` — expected empty list after undo, got `['example.com']` - -## GREEN step (focused) -Command: -```bash -node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js -``` -Result: 30 passed, 0 failed. - -## Full suite -Command: -```bash -node --test -``` -Result: -- tests: 98 -- pass: 98 -- fail: 0 - -## Self-review -- Risk: session-local `freshRules` is intentionally in-memory only and resets on popup reload. That matches task scope. -- Risk: existing behavior for reloading the popup while a rule is fresh is intentionally unchanged (it becomes a normal `×` rule on reload). -- No storage schema changes introduced. -- No unrelated files or features were modified. - -## Fix round 2 report: popup reopen regression - -### Changed files -- `tests/unit/popup-current-site.test.js` - -### Command -```bash -node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js -``` - -### Focused test output -- Focused popup tests: 31 passed, 0 failed. -- Added regression: `fresh rule does not stay fresh after reopening popup` (proved persisted rule renders `×` and opens challenge). - -### Coverage -- Reuses existing popup-focused coverage for list render/remove flows, now explicitly covers cross-session reopen behavior for freshly added rules. - -## Final-fix review round - -### Changes -- `src/popup/popup.js` - - Conditionalized remove button accessibility semantics for fresh rules (`Undo adding `, title `Undo adding this blocked site`) while retaining existing `Remove ` semantics for normal rules. - - Wrapped fresh rule remove flow with error handling to surface storage failures via `setHint(...)`, while keeping fresh status until deletion succeeds. -- `src/popup/popup.css` - - Set `.remove-button--undo:hover` text color to `var(--accent)`. -- `tests/unit/popup-current-site.test.js` - - Added assertions for fresh Undo and normal remove accessibility semantics. - - Awaited fresh Undo button handler promises directly. - - Added regression for storage-rejection on fresh Undo and successful retry behavior. - -### RED verification (focused) -Command: -```bash -node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js -``` -Result before fix: 31 passed, 1 failed (`a failed fresh Undo keeps the rule and retries successfully`) due missing/handled-error behavior on fresh Undo failures (expected hint and retained rule semantics). - -### GREEN verification (focused) -Command: -```bash -node --test tests/unit/popup-current-site.test.js tests/unit/popup-css.test.js -``` -Result: 32 passed, 0 failed. - -### Full suite verification -Command: -```bash -node --test -``` -Result: 100 passed, 0 failed. - -### Coverage -- Focused coverage now includes popup remove-a11y assertions, storage-failure simulation for fresh Undo, and retry path, in addition to existing popup CSS/accessibility checks.