Repository navigation
refactor(extension): end the buildState lizard artifact - #1367
Conversation
… buildState Assisted-by: ClaudeCode:claude-opus-5-5
Assisted-by: ClaudeCode:claude-opus-5-5
Changing the add() call touched perform(), which gate 122 then counts as a new McCabe >10 finding. Assisted-by: ClaudeCode:claude-opus-5-5
rubenvdlinde
changed the title
Quality Report — ConductionNL/keepiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 114/114 | |||
| npm | ✅ | ✅ 661/661 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 2797/2797 · 3 skipped | ||||
| Newman | ✅ 177/177 · 20 skipped | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development. |
||||
| Hydra gates | ✅ |
Quality workflow — 2026-11-02 00:33 UTC
Download the full PDF report from the workflow artifacts.
| active, | ||
| notice, | ||
| canAddAccount: accounts.length < MAX_ACCOUNTS, | ||
| canAddAccount: MAX_ACCOUNTS > accounts.length, |
There was a problem hiding this comment.
🟢 Minor — this changes the code to work around the measuring tool
MAX_ACCOUNTS > accounts.length behaves exactly like accounts.length < MAX_ACCOUNTS, but it is the less natural way to write the check, and the only reason for it is that lizard misreads a < b, inside an object literal as a type argument. Nothing here stops the next person from turning it back, and the description itself says Sigrid's parser may not have this problem. If the number matters, the fix belongs in the measurement (lizard's settings, or reading gate 122's result with this known artifact in mind), not in the extension's code.
SudoThijn
left a comment
SudoThijn
left a comment
There was a problem hiding this comment.
Verdict: COMMENT (Thorough · full)
The swapped comparison behaves exactly as before, including the empty list and the five-account case. My one comment is about why it is there.
🟢 Minor (1)
CI: check-and-build fails on the SendRow date test that #1378 fixed, and e2e on the clipboard alarm test; neither is caused by this change.
Gates: Hydra Gates passed on c2a01177.
… the TOTP key work #1352 (feature/browser-extension-7, merged as 87b34c1) added the authenticator key field to logins, took totp out of the new-item types, and added autoCopyTotp and totpSaved, after #1387 and #1389 had recorded these goldens. Development's Browser extension check-and-build has been red since. The totp options world now edits an existing totp item, because a new item can no longer be one. Every other change in the recordings is the new field, the dropped TOTP option, the two new values, or a shifted React id. Assisted-by: ClaudeCode:claude-opus-5-5
Quality Report — ConductionNL/keepiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 114/114 | |||
| npm | ✅ | ✅ 661/661 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ✅ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 2797/2797 · 3 skipped | ||||
| Newman | ✅ 177/177 · 20 skipped | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development. |
||||
| Hydra gates | ✅ |
Quality workflow — 2026-11-03 01:56 UTC
Download the full PDF report from the workflow artifacts.
Address the review on #1367: buildState reads `accounts.length < MAX_ACCOUNTS` again. The lizard misreading of `a < b,` in an object literal belongs in the measurement, not in the extension's code.
What changes
buildState()in the extension's background router writes the account limit check asMAX_ACCOUNTS > accounts.lengthinstead ofaccounts.length < MAX_ACCOUNTS. This is a measurement fix. lizard reada < b,inside the object literal as the start of a type argument, so it countedbuildState,add,reauthenticateandperformas one 107-line unit with McCabe 34. Sigrid's parser may not have this artifact.Once the artifact is gone,
add(serverUrl, username, appPassword)shows up as its own unit with 3 parameters, so params >=3 goes up 0.14 points. That is code the measurement used to hide, not a new parameter. Passing the message whole would mean touchingperform(), which gate 122 would then count as a new McCabe >10 finding (11). Soadd()stays as it is.SIG numbers (keepiq, TypeScript, lizard via gate 122)
How I verified it
src/background/router.test.tspasses (21 tests), including the empty-list case (canAddAccount: true) and the five-account case (false).NODE_OPTIONS=--max-old-space-size=1536 timeout 120) on the old and the new comparison:>=/<=, the flipped operator and a constanttrueall die on both. The no-op control survives on both.tsc --noEmitexits 0. Gate 122 ratchet: 0 findings, 1 inherited. Gate 119: 0.Inherited
add()has 3 parameters.perform()stays above McCabe 10 (11). Gate 122 reports it as inherited.This change was prepared with Claude Code (AI assisted) and reviewed by a human before merge.