Repository navigation
fix(frontend): secrets UI hides unenforced controls, derives kinds from the API, restores usage logging; analytics stops reading a failed scan as 0 duplicates (#16450, #15008, #17976, #17983) - #17977
Merged
Conversation
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 27 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe changes update secret management, duplicate-scan analytics, and selected SLM locale labels. Secret management removes unenforced access controls, adopts a canonical secret type, and logs reveal and copy actions. Duplicate analytics distinguishes failed, unscanned, and completed scans. ChangesSecrets management
Duplicate-scan analytics
SLM locale labels
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to The change works for the main flows, but there are small UI and logging issues: a failed background duplicate scan can leave old results on screen, some Arabic, Spanish, and Portuguese labels fall back to English, and copy logging can name the wrong secret in a rare timing case. These are worth fixing but do not need to block merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The Full details: Docstring CoverageExplanation Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 12 files. (25 skipped: 25 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mrveiss
added a commit
that referenced
this pull request
Oct 4, 2026
…17957) #17957 gated `python-shard` on `draft == false`. `python-suite` is not a matrix job, so nothing gated it by inheritance, and it re-asserts the shards' verdict: if: ${{ needs.python-shard.result != 'success' }} For a matrix, `result` is the aggregate across all twelve shards, and twelve skipped shards aggregate to `skipped`, which satisfies `!= 'success'`. Every draft PR therefore reported a red `python-suite` for shards that were deliberately not run -- observed on #17977, and true of all eleven open drafts. The job's own comment shows the authors had already foreseen skipped shards and guarded the path-filter cause; the draft gate added a second cause to a condition written for one. The aggregator now carries the shards' condition verbatim. `ready_for_review` is already in this workflow's trigger types, so the real verdict arrives before the PR can merge and no required context is left permanently skipped. Also adds the guard that was missing. It parses the YAML rather than the file text -- this very file spells the gate expression repeatedly in comments, so a text scan would find it everywhere and report clean (#17941). The invariant is deliberately narrow: not "every dependent of a gated job must be gated", but "a condition that reads a gated job's `result` and does not allow `skipped` inherits that job's gate". The first version was the broad rule and produced a false positive on `deployment-check`, which reads `python-suite.result` and accepts `success || skipped` -- correct code, and a guard with false positives gets muted. Six contrast fixtures pin the distinction, including the prose-only case and the tolerant case. Verified by mutation: removing the new condition makes the guard report `ci.yml: python-suite reads python-shard.result`. Refs #17957, #17975
… derive the secret kinds from the API (#16450, #15008) #16450: SecretsManager offered Visibility (private/shared/group/ organization/system) plus Organization ID / Team IDs / Shared-With, but SecretCreateRequest has none of those fields and dropped them silently -- a user who chose 'Shared with alice' got a secret governed by scope alone. Per the owner's decision the controls are hidden until #10088 enforces them: the form fields, their state, validation, payload fields, the visibility badge and its helpers are gone. #15008: SecretType is now components['schemas']['SecretCreateRequest'] ['type'] from the generated API types (types/secretKind.ts), not a 7-of-10 hand list. SecretsManager's categories are a Record<SecretType, ...>, so a kind added to the backend enum fails type-check until it has a category; oauth_refresh_token and connector_oauth_token gain one. Category and kind labels, and the quick-add template descriptions, are translated in all 11 locales; getTypeLabel no longer derives a singular by trimming an 's'. .secrets.baseline: 7 audited false positives (locale labels under keys named api_key/password), added without line numbers per RATCHET_BASELINES.
…ies (#15008) repo_tests/secrets_baseline_reasons_guard_test.py requires every .secrets.baseline entry to carry a specific reason. The seven new entries are the Secrets page's translated category headings for api_key/password; each reason was confirmed by re-hashing the literal label (plain SHA1 reproduces the stored hashed_secret). Kept as a compact table so the entries module stays under MAX_LINES.
mrveiss
force-pushed
the
issue-16450-15008-secrets-ui
branch
from
October 4, 2026 19:27
5916a12 to
42f74f7
Compare
…plicates" (#17983) The backend answers HTTP 200 with {"status":"error"} when the scan did not run (#17982). Three frontend paths read only the duplicates list and turned that into an empty result: the live endpoint (raw.duplicates ?? []), the cached endpoint (null with no no-data handler, so the initial [] stayed) and the background scan (data.duplicates, status unread); getDuplicatesData then announced 'N found'. composables/analytics/duplicatePayload.ts reads a payload by status into done / not_scanned / failed, and all three paths go through it. The panel shows a translated failed or not-yet-scanned message (11 locales) and keeps 'no duplicates' for a scan that ran; getDuplicatesData reports the failure. Audit of the same file: declarations and hardcodes only return success/no_data today, but their readers are now status-aware too, so a future 200 error cannot read as 'none found'. useAnalyticsDataFetchers.ts shrinks 707 -> 694.
mrveiss
changed the title
Owner
Author
Review pass — one blocker on
|
…o "failed" (#17983) `#17983`'s AC1 -- a failed scan must never read as "0 duplicates" -- held for one of the two ways the scan fails and not the other. `applyDuplicates` only ever runs on a 2xx, so it correctly reads a 200 body of `{"status":"error"}` as `'failed'`. But the endpoint also answers **404** for a source it cannot resolve (`api/codebase_analytics/endpoints/duplicates.py`), and `useFetchEndpoint` throws on `!response.ok` (`useFetchEndpoint.ts:249`) -- so that lands in the catch and reaches `onError`, which nothing was wired to. The hook has supported the callback all along (`useFetchEndpoint.ts:84`); both duplicates endpoints passed `onSuccess` and stopped there. With nothing wired, the state kept whatever it already held: first load, then 404 -> 'not_scanned' ("No duplicate scan has run") successful scan, then 404 -> 'done', WITH THE PREVIOUS SOURCE'S ROWS The second is the one worth the fix. `'not_scanned'` at least says no scan has run; a stale `'done'` asserts a result that is not this source's, under a scan that just failed. Neither shape ever rendered a literal `0`, so the headline defect the PR fixed stays fixed -- this is the remaining hole in the same acceptance criterion, not a regression of it. `failDuplicates` clears the list as well as setting the state: `'failed'` with a populated list is a fourth state no panel renders, and the stale rows would otherwise survive behind the failure notice. The notify path at `:445` already gated on `duplicateScanState.value === 'failed'`, so wiring the state is all that was missing for it to report the failure correctly too. Evidence 13 passed (was 10). Mutation-tested, each mutant killed by the intended test: remove onError from both endpoints -> all 3 new tests fail set the state but do NOT clear list -> only the stale-list test fails The stale-list test carries a CONTROL: it asserts the first scan really did populate (`'done'`, 1 row) before the 404, so the clear is a change of state and not a no-op on an already-empty list. eslint 0 on both changed files. The file lands at 704 lines against its recorded ceiling of 707: the first attempt was refused at 711, and the explanatory comment was compressed rather than the ceiling raised -- the full reasoning lives in this message, which is the right place for it. The guard is bidirectional, so landing UNDER the old ceiling is also refused: 707 -> 704 in both mirrors (`scripts/frontend_file_size_known_large.py` and `repo_tests/frontend_file_size_ratchet_baseline.py`), because an unlowered ceiling re-licenses the lines just cut. Found by a review pass on #17977 and confirmed by reading the three call sites rather than taken on report. Refs #17983
…cates" (#17983) Found by running the live backend, not by reading code. `POST /duplicates/analyze` on the AutoBot source times out and answers HTTP 200 with: {"status":"partial","message":"Analysis timed out after 120.0s. Try a higher min_similarity threshold.","duplicates":[],"total_count":0, "storage_type":"timeout"} `readDuplicatePayload` mapped `status === 'partial'` with ANY array to `'done'`, so that rendered as "no duplicates found" — the single answer this module was written to prevent, surviving in the one branch nobody separated. It is also the shape this codebase ACTUALLY produces, so the panel has been reporting a clean bill of health for a scan that never finished. `partial` means "timed out carrying what I had", which makes its emptiness the whole question: success + [] -> done the scan ran and found nothing: the one case allowed to render "none found" partial + [rows] -> done a real, incomplete result; show the rows partial + [] -> failed the scan learned nothing error -> failed unchanged anything else -> not_scanned Evidence 14 passed (was 13). The new row is the verbatim live payload, added to the existing `it.each` table so it sits beside the `success + []` case it must be distinguished from — the two differ only in the status string, which is exactly why one was mistaken for the other. Scope note: this closes the last hole in #17983's AC1 that I know of. Two earlier gaps on this branch are already fixed — the 200 `{"status":"error"}` body, and the HTTP-error path where `applyDuplicates` was wired to `onSuccess` only so a 404 kept a stale `'done'` with the previous source's rows. Two further defects on the same endpoint are NOT fixed here, because they are backend and this file is frontend: * `refresh=true` returned the cached timeout artifact in 547ms instead of re-running. * the timeout advises raising `min_similarity`, which `POST /duplicates/analyze` does not accept — only `GET /duplicates` does. The advice is unactionable from the endpoint that emits it. Refs #17983
…15008) Three grandfathered entries were out of step with the tree after this branch merged current main. Caught locally before pushing, so CI never saw them: SecretsManager.vue measured 2775, ceiling 2947 -> 2775 (-172) CodebaseAnalytics.vue measured 1297, ceiling 1295 -> 1297 (+2) useChatStore.ts measured 1176, ceiling 1174 -> 1176 (+2) MAX_CEILING_TOTAL 184521 -> 184350 (-171) SecretsManager.vue is the real reduction: #16450 removed the Visibility, Organization, Team and Shared-With controls because nothing enforced them, and the guard is bidirectional -- leaving the ceiling at 2947 would re-license the 172 lines just cut. TWO CEILINGS GO UP BY 2, WHICH IS A DECISION AND NOT AN OVERSIGHT The standing rule is "split, don't raise the ceiling", so this is stated rather than buried. The additions are the fix itself, and there is nothing padding them: CodebaseAnalytics.vue `:scan-state="duplicateScanState"` (prop binding) `duplicateScanState,` (destructure) useChatStore.ts `import type { SecretType } …` `type: SecretType` + its one-line note Each is a single entry in an already multi-line prop list / destructure / import block. There is no comment to compress and no blank line to reclaim, so the only way to hold both files at their frozen size would be to delete something that matters or to drop the wiring that makes the bug fix reachable. I raised them deliberately, 2 lines each, and the aggregate still falls by 171 -- the ratchet turns down overall, which is what it is for. The same call was flagged on #17969 for `environment.py` (694 -> 698 for a log-injection fix) rather than passed over quietly; this is the same judgement, recorded the same way. If the preference is to hold the per-file ceilings absolutely, the wiring would have to move to another file and that should be its own change. Evidence 21 passed on frontend_file_size_ratchet_test (was 2 failed / 19 passed). Both mirrors byte-identical on every entry, verified by diff. 25 passed across duplicateScanState, DuplicatesSection.scanState and SecretsManager.access -- the suites this branch exists for, re-run after the merge of current main. Refs #17983
mrveiss
marked this pull request as ready for review
October 7, 2026 21:32
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
Merged
7 tasks done
mrveiss
added a commit
that referenced
this pull request
Oct 8, 2026
…ession is needed (#18065) I broke the base. `f6b64b67d0` (#18065) left 2 open high-severity CodeQL alerts, and `pipeline-scripts/check_codeql_alert_ceiling.sh` holds the ceiling at 0, so `code-quality` — a REQUIRED context — fails on EVERY open PR. It surfaced on #17977, which touches none of this. Both alerts are `js/incomplete-multi-character-sanitization` at `useVoiceConversation.ts:152`, created minutes after that merge. The cause was not a new vulnerability: the line changed from `let clean = text` to `let clean = stripProtocolTagsForSpeech(text)`, which made it the source of the dataflow, so CodeQL re-anchored the finding onto a line carrying no suppression comment. The comments never moved; the finding did. WHY THE FIX IS A RESTRUCTURE AND NOT A COMMENT My first attempt added the suppression to the newly-anchored line. That was the wrong instinct twice over: * The gate reads `code-scanning/alerts?state=open` with NO `ref` (check_codeql_alert_ceiling.sh:38), so it counts MAIN's alerts regardless of which PR is running. No PR can turn the check green by changing code; the check never looks at the PR. Chasing the anchor could not have worked. * The rule is right and the single pass really is incomplete. Removing an inner match can reassemble an outer one — `<scr<script>ipt>` leaves `<script>` — so one pass cannot be trusted, and a comment asserting otherwise is a claim the code does not support. So the two single-pass `<script>` replaces are gone, and script removal now runs INSIDE the existing `while (prev !== clean)` loop. Repeat-until-unchanged is the canonical complete form for this rule, and the file now carries ZERO `codeql[...]` comments: nothing is being waived. Behaviour is unchanged, checked case by case against the old implementation: 'Hello <script>alert(1)</script> world' -> 'Hello world' SAME '<b>hi</b> there' -> 'hi there' SAME '<scr<script>ipt>alert(1)</script>evil' -> '<screvil' SAME '<SCRIPT SRC=x></SCRIPT>ok' -> 'ok' SAME 'a <script>x</script> b <script>y</script> c' -> 'a b c' SAME 'plain text' -> 'plain text' SAME Line-neutral: 986, exactly the recorded ceiling, so no ratchet entry moves. Evidence eslint clean. useVoiceConversation 5 passed, useVoiceBundle included 11 passed. `codeql[` occurrences in the file: 0 (was 3). Open alerts before this change: #1160, #1161. The alerts can only clear once CodeQL re-scans MAIN, which happens after this merges — the gate's missing `ref` makes that unavoidable, not a choice. Flagged separately: 5+ `codeql[...]` comments elsewhere in the frontend may be inert for the same reason, since GitHub code scanning does not honour inline suppression. Refs #18065
5 tasks
`gh pr update-branch` refused ("Cannot update PR branch due to conflicts") after
#18096 landed. One conflict, and it is the aggregate ratchet pin — the same hub
file both changes touch:
HEAD MAX_CEILING_TOTAL = 184350 this branch: SecretsManager.vue 2947 -> 2775
origin/main MAX_CEILING_TOTAL = 184521 #18065: useVoiceConversation.ts 986
Resolved by MEASURING the merged mirrors rather than choosing a side. Both
per-file mirrors merged cleanly and now sum to 184350, which is HEAD's value —
but it is recorded here as the measured sum, not as "HEAD won": the two
reductions are independent (SecretsManager.vue -172 here, useVoiceConversation.ts
back to 986 there) and the total has to equal the entries, not whichever number
looked familiar.
After the merge:
184 entries, sum 184350
useVoiceConversation.ts 986 (from #18065)
useAnalyticsDataFetchers.ts 704 (this branch)
SecretsManager.vue 2775 (this branch)
21 passed on frontend_file_size_ratchet_test; both mirrors byte-identical.
Also carried in by the merge: the CodeQL ceiling gate now counts the REF UNDER
TEST, so this PR's `code-quality` reads its own alert count. Measured on
`refs/pull/17977/merge`: 0 open alerts. Before #18096 it read main's backlog and
failed for a reason that had nothing to do with this branch.
…pp (#15008) `SLM Frontend Check` was red, and it is this PR's doing through a coupling that is invisible from the diff. `scripts/lift_locale_translations.py:68` sets SOURCE_LOCALES = Path("autobot-frontend") / "src" / "i18n" / "locales" so `--app autobot-slm-frontend/src/locales` reads **autobot-frontend's** locales as its SOURCE (`:164`, `:168`, `:177`). This PR adds 33 keys to all 11 autobot-frontend locale files, which changes what the lifting rule produces for the SLM app — so the SLM files stopped matching, in an app this PR never touches. Re-lifted. The check now exits 0. WHAT CHANGED IS AN IMPROVEMENT, NOT CHURN No keys are added or removed: 54 insertions, 54 deletions, the same 4-6 keys in each of 10 locales, replacing untranslated English with the translations this PR added upstream. For example in `pl.json`: authMethodKey "SSH Key" -> "Klucz SSH" certificate "Certificate" -> "Certyfikat" serverHost "Server Host" -> "Host serwera" A review of this PR had noted `'SSH Key'` and `'Server Host'` as remaining hardcoded strings and judged them acceptable; the PR gave them keys (`templates.sshLabel`, `templates.serverLabel`), and the SLM app inherits the translations through the lifting rule. That is the rule working. TWO WRONG READINGS ON THE WAY HERE, BOTH CORRECTED BY MEASUREMENT `ci-why` reported the failing path as `autobot-slm-frontend/src/locales/...`, which is where the check POINTS but not where the cause lives. Taking that at face value, I was about to report that the gated app had regressed and that #17689 — which records the SLM app as PASSING and autobot-frontend as failing — had inverted. Running the check locally disproved it: on a tree off current main the SLM app passes. Then the same command failed on THIS tree and passed on the other, with the SLM locales and the script byte-identical between them. That contradiction is what pointed at `SOURCE_LOCALES`: the input that differed was the source app, which only this branch changes. Evidence before: lift_locale_translations.py --app autobot-slm-frontend/src/locales --check -> "these locales are not what the lifting rule produces: ar, de, es, fa, fr, he, lv, pl, pt, ur" rc=1 after: rc=0 diff: 10 files, 54 insertions, 54 deletions, no keys added or removed Related: #17689 records that the gate is wired for `autobot-slm-frontend` only, so a change to the UNGATED source app breaks the GATED consumer's check. That asymmetry is the reason this surfaced here rather than on the app that changed. Refs #15008
…ssVar (#17560) `python-suite shard 10/12` was red on `hardcoded_colour_ratchet_17560_test.py`, three ways at once: pinned counts grew 10 colours, each +1 new literals appeared #0ea5e9, #14b8a6, #ec4899, #f97316 total rose 65 > 55 The cause was the shape of `CATEGORY_META`, not a missing token. Each entry held the token and the hex as SEPARATE properties: api_key: { …, colorVar: '--color-primary', fallback: '#6366f1' } and resolved them later with `getCssVar(meta.colorVar, meta.fallback)`. WHY THE CODE CHANGED AND NOT THE GUARD My first instinct was that the ratchet was wrong — the pair resolves lazily, so it follows `[data-theme]`, and I assumed the inline form the ratchet accepts (`getCssVar('--token', '#hex')` in `credentialTemplates`) was evaluated once at import and therefore worse. That would have made the guard the thing to fix. It is not true. Both sit inside `computed()`, so both re-resolve on a theme change. The pair bought nothing functionally and cost the check: a static checker can only tie a literal to a token when the two are LEXICALLY together, and splitting them across properties makes the association invisible to any checker, not just this one. So `color` is now a thunk — deferred exactly as before, hex back inside its call: api_key: { …, color: () => getCssVar('--color-primary', '#6366f1') } … color: meta.color() Had I changed the guard instead, I would have weakened a design-system check to accommodate a form with no advantage. Worth recording, because "the guard is wrong" was the more comfortable conclusion and it was the one I reached first. Evidence hardcoded_colour_ratchet_17560_test 42 passed (was 3 failed) frontend_file_size_ratchet_test 21 passed SecretsManager.access.test 8 passed eslint 0, vue-tsc no errors in this file SecretsManager.vue lands at 2775, exactly its recorded ceiling, so no ratchet entry moves. The conversion is line-neutral; the only growth was my explanatory comment, which went from five lines to one and then onto the existing line above rather than raising the pin. The reasoning lives here instead. Refs #17560
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
a9d4e338-3b9f-40d2-8264-efd6f51d8e4e
📒 Files selected for processing (37)
.secrets.baselineautobot-frontend/src/components/__tests__/SecretsManager.access.test.tsautobot-frontend/src/components/analytics/CodebaseAnalytics.vueautobot-frontend/src/components/analytics/DuplicatesSection.vueautobot-frontend/src/components/analytics/__tests__/DuplicatesSection.scanState.test.tsautobot-frontend/src/components/security/SecretsManager.vueautobot-frontend/src/composables/analytics/__tests__/duplicateScanState.test.tsautobot-frontend/src/composables/analytics/duplicatePayload.tsautobot-frontend/src/composables/analytics/useAnalyticsDataFetchers.tsautobot-frontend/src/composables/useSessionActivityLogger.tsautobot-frontend/src/i18n/locales/ar.jsonautobot-frontend/src/i18n/locales/de.jsonautobot-frontend/src/i18n/locales/en.jsonautobot-frontend/src/i18n/locales/es.jsonautobot-frontend/src/i18n/locales/fa.jsonautobot-frontend/src/i18n/locales/fr.jsonautobot-frontend/src/i18n/locales/he.jsonautobot-frontend/src/i18n/locales/lv.jsonautobot-frontend/src/i18n/locales/pl.jsonautobot-frontend/src/i18n/locales/pt.jsonautobot-frontend/src/i18n/locales/ur.jsonautobot-frontend/src/stores/useChatStore.tsautobot-frontend/src/types/secretKind.tsautobot-slm-frontend/src/locales/ar.jsonautobot-slm-frontend/src/locales/de.jsonautobot-slm-frontend/src/locales/es.jsonautobot-slm-frontend/src/locales/fa.jsonautobot-slm-frontend/src/locales/fr.jsonautobot-slm-frontend/src/locales/he.jsonautobot-slm-frontend/src/locales/lv.jsonautobot-slm-frontend/src/locales/pl.jsonautobot-slm-frontend/src/locales/pt.jsonautobot-slm-frontend/src/locales/ur.jsonrepo_tests/frontend_file_size_ratchet_baseline.pyrepo_tests/frontend_file_size_ratchet_test.pyrepo_tests/secrets_baseline_reason_entries.pyscripts/frontend_file_size_known_large.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
…ing English (#15008) A review caught a regression I introduced and had reported as an improvement. It was right, and my error was sampling: I read `pl.json`'s diff, saw three English placeholders become real translations, and generalised that to all ten locales and every changed key. I never checked whether any change went the other way. Measured properly, the previous commit was 44 improvements and **10 REGRESSIONS**: settings.securitySettings.aPIKeys x2 translated -> "API Keys" loginView.password "كلمة المرور" -> "Password" addNodeModal.password, setupWizardView.password, sharedLinksView.colPassword, fleet.nodeCard.authMethodPassword, settings.admin.*.password, settings.securitySettings.password An Arabic UI showing "Password" is a user-visible defect, and it was mine. ROOT CAUSE: THE SAME ENGLISH STRING, TRANSLATED TWO WAYS `lift_locale_translations.py` matches by exact English text and needs a unique translation or a MAJORITY_THRESHOLD (0.8) consensus; otherwise `_lift` returns None and `build_locale` writes the English. The source held near-identical variants of one string, so no candidate reached the threshold: ar "Password" 'كلمة مرور' vs 'كلمة المرور' 1:1 -> English pt "Password" 'Palavra-passe' vs 'Senha' 1:3 -> English (0.75 < 0.8) es "API Keys" 'Claves API' vs 'Claves de API' 1:1 -> English pt "API Keys" 'Chaves API' vs 'Chaves de API' 1:1 -> English A definite article and a stray "de" were enough. Where the source agreed — es/de "Password", 4 of 4 — the lift worked fine all along. So this is a duplication defect, not a translation gap: one concept with two spellings, drifted far enough to defeat a consumer. Canonicalised to one value per English string per locale, across EVERY source key carrying that string (14 keys over 3 files), which also translates the ones that were plain English and makes each candidate set unanimous. Ties went to the value the SLM app already shows users (`كلمة المرور`, `Claves API`, `Chaves API`) so canonicalising cannot itself change a rendered string; `pt` "Password" went to the 3-of-4 majority `Senha`. Evidence before this commit: 44 improvements, 10 regressions after: 52 improvements, 0 regressions lift --check: rc=0 autobot-frontend check:i18n rc=0, check:locales rc=0 (all locales complete) colour + size ratchets 63 passed Carried along, because canonicalising changed the flagged strings: `.secrets.baseline` SHRANK 1402 -> 1397 entries (replacing the literal "Password" with a translation removes a Secret-Keyword hit), and two `_SECRETS_CATEGORY_LABELS tuples in `repo_tests/secrets_baseline_reason_entries.py` went stale because their hash is of the label text. Updated to the canonical labels, with the new hashes READ from the baseline rather than assumed: I computed sha1 for each candidate and confirmed membership, since a reconstructed hash has the right shape and the wrong content. 12 passed across the three baseline guards. Related: #17689 records that `autobot-frontend`'s locales do not satisfy the lifting rule and that no gate checks them, while the gate runs for the SLM app only. That asymmetry is why a source inconsistency surfaced as a failure in the consumer. Four of its instances are fixed here; the issue stays open for the rest. Refs #15008
…ity, and the third stale-rows path (#15008, #17976, #17983) The three remaining review findings. All real; the third was mine twice over. 1. A PROTOTYPE NAME PASSED AS A CANONICAL KIND `type in CATEGORY_META` and `CATEGORY_META[type]` both see INHERITED names, so a server-supplied kind of `toString` or `constructor` reached `t(meta.singularKey)` with `singularKey` undefined instead of taking the unknown-kind fallback. Both sites now use `Object.hasOwn`. 2. THE ACTIVITY LOG NAMED THE WRONG SECRET `copySecretValue` awaited `navigator.clipboard.writeText(...)` and then called `logViewedSecretUsage('copy')`, which reads `viewingSecret.value` itself. Opening another secret while the write was pending left the clipboard holding the FIRST value while the activity recorded the SECOND — a usage log that names the wrong subject is worse than none, because it is the record someone later audits. The subject is now captured before the await and passed in; `logViewedSecretUsage` takes it as a parameter defaulting to the current selection, so the reveal path is unchanged. 3. THE THIRD STALE-ROWS PATH, WHICH I HAD MISSED `failDuplicates` was wired into both endpoint fetchers and NOT into `loadDuplicates`, where the background-task branch only set the state. `DuplicatesSection` checks `duplicates.length > 0` BEFORE `scanState`, so rows from an earlier scan stayed on screen and hid the failure — the same defect, in the one path I did not touch when fixing the other two. AND THE TEST FOR IT WAS WRONG FIRST My first test called `getDuplicatesData()` and passed with the fix reverted. The branch lives in `loadDuplicates()`; in the other function `duplicatesSilent`'s `onError` — which I had already fixed — clears the rows before that code runs, so the test exercised the wrong path and proved nothing. Found by mutation, not by reading: reverting the fix left 15 passing. Retargeted to `loadDuplicates()`, it now fails on the revert and passes on the fix. It also carries a CONTROL: the first scan is asserted to have really populated (state `done`, one row), so the later empty list is a change rather than a no-op. Evidence duplicateScanState 15 passed (was 14) mutation: revert the dupTask branch to state-only -> that test fails, alone colour + file-size ratchets 63 passed eslint 0, vue-tsc no errors in either file SecretsManager.vue 2775 and useAnalyticsDataFetchers.ts 704 — both exactly at their recorded ceilings, so no ratchet entry moves. The explanations were folded onto the lines they annotate to pay for the code; the reasoning is here. Refs #15008, #17976, #17983
This was referenced Oct 8, 2026
mrveiss
added a commit
that referenced
this pull request
Oct 10, 2026
…om the API, restores usage logging; analytics stops reading a failed scan as 0 duplicates (#16450, #15008, #17976, #17983) (#17977) * fix(secrets): hide the access controls the server never enforced, and derive the secret kinds from the API (#16450, #15008) #16450: SecretsManager offered Visibility (private/shared/group/ organization/system) plus Organization ID / Team IDs / Shared-With, but SecretCreateRequest has none of those fields and dropped them silently -- a user who chose 'Shared with alice' got a secret governed by scope alone. Per the owner's decision the controls are hidden until #10088 enforces them: the form fields, their state, validation, payload fields, the visibility badge and its helpers are gone. #15008: SecretType is now components['schemas']['SecretCreateRequest'] ['type'] from the generated API types (types/secretKind.ts), not a 7-of-10 hand list. SecretsManager's categories are a Record<SecretType, ...>, so a kind added to the backend enum fails type-check until it has a category; oauth_refresh_token and connector_oauth_token gain one. Category and kind labels, and the quick-add template descriptions, are translated in all 11 locales; getTypeLabel no longer derives a singular by trimming an 's'. .secrets.baseline: 7 audited false positives (locale labels under keys named api_key/password), added without line numbers per RATCHET_BASELINES. * test(secrets): tracked reasons for the 7 audited locale baseline entries (#15008) repo_tests/secrets_baseline_reasons_guard_test.py requires every .secrets.baseline entry to carry a specific reason. The seven new entries are the Secrets page's translated category headings for api_key/password; each reason was confirmed by re-hashing the literal label (plain SHA1 reproduces the stored hashed_secret). Kept as a compact table so the entries module stays under MAX_LINES. * fix(analytics): a failed duplicate scan is its own state, never "0 duplicates" (#17983) The backend answers HTTP 200 with {"status":"error"} when the scan did not run (#17982). Three frontend paths read only the duplicates list and turned that into an empty result: the live endpoint (raw.duplicates ?? []), the cached endpoint (null with no no-data handler, so the initial [] stayed) and the background scan (data.duplicates, status unread); getDuplicatesData then announced 'N found'. composables/analytics/duplicatePayload.ts reads a payload by status into done / not_scanned / failed, and all three paths go through it. The panel shows a translated failed or not-yet-scanned message (11 locales) and keeps 'no duplicates' for a scan that ran; getDuplicatesData reports the failure. Audit of the same file: declarations and hardcodes only return success/no_data today, but their readers are now status-aware too, so a future 200 error cannot read as 'none found'. useAnalyticsDataFetchers.ts shrinks 707 -> 694. * fix(analytics): a duplicate scan that fails with an HTTP error is also "failed" (#17983) `#17983`'s AC1 -- a failed scan must never read as "0 duplicates" -- held for one of the two ways the scan fails and not the other. `applyDuplicates` only ever runs on a 2xx, so it correctly reads a 200 body of `{"status":"error"}` as `'failed'`. But the endpoint also answers **404** for a source it cannot resolve (`api/codebase_analytics/endpoints/duplicates.py`), and `useFetchEndpoint` throws on `!response.ok` (`useFetchEndpoint.ts:249`) -- so that lands in the catch and reaches `onError`, which nothing was wired to. The hook has supported the callback all along (`useFetchEndpoint.ts:84`); both duplicates endpoints passed `onSuccess` and stopped there. With nothing wired, the state kept whatever it already held: first load, then 404 -> 'not_scanned' ("No duplicate scan has run") successful scan, then 404 -> 'done', WITH THE PREVIOUS SOURCE'S ROWS The second is the one worth the fix. `'not_scanned'` at least says no scan has run; a stale `'done'` asserts a result that is not this source's, under a scan that just failed. Neither shape ever rendered a literal `0`, so the headline defect the PR fixed stays fixed -- this is the remaining hole in the same acceptance criterion, not a regression of it. `failDuplicates` clears the list as well as setting the state: `'failed'` with a populated list is a fourth state no panel renders, and the stale rows would otherwise survive behind the failure notice. The notify path at `:445` already gated on `duplicateScanState.value === 'failed'`, so wiring the state is all that was missing for it to report the failure correctly too. Evidence 13 passed (was 10). Mutation-tested, each mutant killed by the intended test: remove onError from both endpoints -> all 3 new tests fail set the state but do NOT clear list -> only the stale-list test fails The stale-list test carries a CONTROL: it asserts the first scan really did populate (`'done'`, 1 row) before the 404, so the clear is a change of state and not a no-op on an already-empty list. eslint 0 on both changed files. The file lands at 704 lines against its recorded ceiling of 707: the first attempt was refused at 711, and the explanatory comment was compressed rather than the ceiling raised -- the full reasoning lives in this message, which is the right place for it. The guard is bidirectional, so landing UNDER the old ceiling is also refused: 707 -> 704 in both mirrors (`scripts/frontend_file_size_known_large.py` and `repo_tests/frontend_file_size_ratchet_baseline.py`), because an unlowered ceiling re-licenses the lines just cut. Found by a review pass on #17977 and confirmed by reading the three call sites rather than taken on report. Refs #17983 * fix(analytics): a timed-out duplicate scan is a failure, not "0 duplicates" (#17983) Found by running the live backend, not by reading code. `POST /duplicates/analyze` on the AutoBot source times out and answers HTTP 200 with: {"status":"partial","message":"Analysis timed out after 120.0s. Try a higher min_similarity threshold.","duplicates":[],"total_count":0, "storage_type":"timeout"} `readDuplicatePayload` mapped `status === 'partial'` with ANY array to `'done'`, so that rendered as "no duplicates found" — the single answer this module was written to prevent, surviving in the one branch nobody separated. It is also the shape this codebase ACTUALLY produces, so the panel has been reporting a clean bill of health for a scan that never finished. `partial` means "timed out carrying what I had", which makes its emptiness the whole question: success + [] -> done the scan ran and found nothing: the one case allowed to render "none found" partial + [rows] -> done a real, incomplete result; show the rows partial + [] -> failed the scan learned nothing error -> failed unchanged anything else -> not_scanned Evidence 14 passed (was 13). The new row is the verbatim live payload, added to the existing `it.each` table so it sits beside the `success + []` case it must be distinguished from — the two differ only in the status string, which is exactly why one was mistaken for the other. Scope note: this closes the last hole in #17983's AC1 that I know of. Two earlier gaps on this branch are already fixed — the 200 `{"status":"error"}` body, and the HTTP-error path where `applyDuplicates` was wired to `onSuccess` only so a 404 kept a stale `'done'` with the previous source's rows. Two further defects on the same endpoint are NOT fixed here, because they are backend and this file is frontend: * `refresh=true` returned the cached timeout artifact in 547ms instead of re-running. * the timeout advises raising `min_similarity`, which `POST /duplicates/analyze` does not accept — only `GET /duplicates` does. The advice is unactionable from the endpoint that emits it. Refs #17983 * chore(ratchet): frontend ceilings follow the files, net -171 (#17983, #15008) Three grandfathered entries were out of step with the tree after this branch merged current main. Caught locally before pushing, so CI never saw them: SecretsManager.vue measured 2775, ceiling 2947 -> 2775 (-172) CodebaseAnalytics.vue measured 1297, ceiling 1295 -> 1297 (+2) useChatStore.ts measured 1176, ceiling 1174 -> 1176 (+2) MAX_CEILING_TOTAL 184521 -> 184350 (-171) SecretsManager.vue is the real reduction: #16450 removed the Visibility, Organization, Team and Shared-With controls because nothing enforced them, and the guard is bidirectional -- leaving the ceiling at 2947 would re-license the 172 lines just cut. TWO CEILINGS GO UP BY 2, WHICH IS A DECISION AND NOT AN OVERSIGHT The standing rule is "split, don't raise the ceiling", so this is stated rather than buried. The additions are the fix itself, and there is nothing padding them: CodebaseAnalytics.vue `:scan-state="duplicateScanState"` (prop binding) `duplicateScanState,` (destructure) useChatStore.ts `import type { SecretType } …` `type: SecretType` + its one-line note Each is a single entry in an already multi-line prop list / destructure / import block. There is no comment to compress and no blank line to reclaim, so the only way to hold both files at their frozen size would be to delete something that matters or to drop the wiring that makes the bug fix reachable. I raised them deliberately, 2 lines each, and the aggregate still falls by 171 -- the ratchet turns down overall, which is what it is for. The same call was flagged on #17969 for `environment.py` (694 -> 698 for a log-injection fix) rather than passed over quietly; this is the same judgement, recorded the same way. If the preference is to hold the per-file ceilings absolutely, the wiring would have to move to another file and that should be its own change. Evidence 21 passed on frontend_file_size_ratchet_test (was 2 failed / 19 passed). Both mirrors byte-identical on every entry, verified by diff. 25 passed across duplicateScanState, DuplicatesSection.scanState and SecretsManager.access -- the suites this branch exists for, re-run after the merge of current main. Refs #17983 * fix(i18n): re-lift the SLM locales after this PR changed the source app (#15008) `SLM Frontend Check` was red, and it is this PR's doing through a coupling that is invisible from the diff. `scripts/lift_locale_translations.py:68` sets SOURCE_LOCALES = Path("autobot-frontend") / "src" / "i18n" / "locales" so `--app autobot-slm-frontend/src/locales` reads **autobot-frontend's** locales as its SOURCE (`:164`, `:168`, `:177`). This PR adds 33 keys to all 11 autobot-frontend locale files, which changes what the lifting rule produces for the SLM app — so the SLM files stopped matching, in an app this PR never touches. Re-lifted. The check now exits 0. WHAT CHANGED IS AN IMPROVEMENT, NOT CHURN No keys are added or removed: 54 insertions, 54 deletions, the same 4-6 keys in each of 10 locales, replacing untranslated English with the translations this PR added upstream. For example in `pl.json`: authMethodKey "SSH Key" -> "Klucz SSH" certificate "Certificate" -> "Certyfikat" serverHost "Server Host" -> "Host serwera" A review of this PR had noted `'SSH Key'` and `'Server Host'` as remaining hardcoded strings and judged them acceptable; the PR gave them keys (`templates.sshLabel`, `templates.serverLabel`), and the SLM app inherits the translations through the lifting rule. That is the rule working. TWO WRONG READINGS ON THE WAY HERE, BOTH CORRECTED BY MEASUREMENT `ci-why` reported the failing path as `autobot-slm-frontend/src/locales/...`, which is where the check POINTS but not where the cause lives. Taking that at face value, I was about to report that the gated app had regressed and that #17689 — which records the SLM app as PASSING and autobot-frontend as failing — had inverted. Running the check locally disproved it: on a tree off current main the SLM app passes. Then the same command failed on THIS tree and passed on the other, with the SLM locales and the script byte-identical between them. That contradiction is what pointed at `SOURCE_LOCALES`: the input that differed was the source app, which only this branch changes. Evidence before: lift_locale_translations.py --app autobot-slm-frontend/src/locales --check -> "these locales are not what the lifting rule produces: ar, de, es, fa, fr, he, lv, pl, pt, ur" rc=1 after: rc=0 diff: 10 files, 54 insertions, 54 deletions, no keys added or removed Related: #17689 records that the gate is wired for `autobot-slm-frontend` only, so a change to the UNGATED source app breaks the GATED consumer's check. That asymmetry is the reason this surfaced here rather than on the app that changed. Refs #15008 * fix(design-system): each category colour resolves inside its own getCssVar (#17560) `python-suite shard 10/12` was red on `hardcoded_colour_ratchet_17560_test.py`, three ways at once: pinned counts grew 10 colours, each +1 new literals appeared #0ea5e9, #14b8a6, #ec4899, #f97316 total rose 65 > 55 The cause was the shape of `CATEGORY_META`, not a missing token. Each entry held the token and the hex as SEPARATE properties: api_key: { …, colorVar: '--color-primary', fallback: '#6366f1' } and resolved them later with `getCssVar(meta.colorVar, meta.fallback)`. WHY THE CODE CHANGED AND NOT THE GUARD My first instinct was that the ratchet was wrong — the pair resolves lazily, so it follows `[data-theme]`, and I assumed the inline form the ratchet accepts (`getCssVar('--token', '#hex')` in `credentialTemplates`) was evaluated once at import and therefore worse. That would have made the guard the thing to fix. It is not true. Both sit inside `computed()`, so both re-resolve on a theme change. The pair bought nothing functionally and cost the check: a static checker can only tie a literal to a token when the two are LEXICALLY together, and splitting them across properties makes the association invisible to any checker, not just this one. So `color` is now a thunk — deferred exactly as before, hex back inside its call: api_key: { …, color: () => getCssVar('--color-primary', '#6366f1') } … color: meta.color() Had I changed the guard instead, I would have weakened a design-system check to accommodate a form with no advantage. Worth recording, because "the guard is wrong" was the more comfortable conclusion and it was the one I reached first. Evidence hardcoded_colour_ratchet_17560_test 42 passed (was 3 failed) frontend_file_size_ratchet_test 21 passed SecretsManager.access.test 8 passed eslint 0, vue-tsc no errors in this file SecretsManager.vue lands at 2775, exactly its recorded ceiling, so no ratchet entry moves. The conversion is line-neutral; the only growth was my explanatory comment, which went from five lines to one and then onto the existing line above rather than raising the pin. The reasoning lives here instead. Refs #17560 * fix(i18n): one translation per English string, so the lift stops writing English (#15008) A review caught a regression I introduced and had reported as an improvement. It was right, and my error was sampling: I read `pl.json`'s diff, saw three English placeholders become real translations, and generalised that to all ten locales and every changed key. I never checked whether any change went the other way. Measured properly, the previous commit was 44 improvements and **10 REGRESSIONS**: settings.securitySettings.aPIKeys x2 translated -> "API Keys" loginView.password "كلمة المرور" -> "Password" addNodeModal.password, setupWizardView.password, sharedLinksView.colPassword, fleet.nodeCard.authMethodPassword, settings.admin.*.password, settings.securitySettings.password An Arabic UI showing "Password" is a user-visible defect, and it was mine. ROOT CAUSE: THE SAME ENGLISH STRING, TRANSLATED TWO WAYS `lift_locale_translations.py` matches by exact English text and needs a unique translation or a MAJORITY_THRESHOLD (0.8) consensus; otherwise `_lift` returns None and `build_locale` writes the English. The source held near-identical variants of one string, so no candidate reached the threshold: ar "Password" 'كلمة مرور' vs 'كلمة المرور' 1:1 -> English pt "Password" 'Palavra-passe' vs 'Senha' 1:3 -> English (0.75 < 0.8) es "API Keys" 'Claves API' vs 'Claves de API' 1:1 -> English pt "API Keys" 'Chaves API' vs 'Chaves de API' 1:1 -> English A definite article and a stray "de" were enough. Where the source agreed — es/de "Password", 4 of 4 — the lift worked fine all along. So this is a duplication defect, not a translation gap: one concept with two spellings, drifted far enough to defeat a consumer. Canonicalised to one value per English string per locale, across EVERY source key carrying that string (14 keys over 3 files), which also translates the ones that were plain English and makes each candidate set unanimous. Ties went to the value the SLM app already shows users (`كلمة المرور`, `Claves API`, `Chaves API`) so canonicalising cannot itself change a rendered string; `pt` "Password" went to the 3-of-4 majority `Senha`. Evidence before this commit: 44 improvements, 10 regressions after: 52 improvements, 0 regressions lift --check: rc=0 autobot-frontend check:i18n rc=0, check:locales rc=0 (all locales complete) colour + size ratchets 63 passed Carried along, because canonicalising changed the flagged strings: `.secrets.baseline` SHRANK 1402 -> 1397 entries (replacing the literal "Password" with a translation removes a Secret-Keyword hit), and two `_SECRETS_CATEGORY_LABELS tuples in `repo_tests/secrets_baseline_reason_entries.py` went stale because their hash is of the label text. Updated to the canonical labels, with the new hashes READ from the baseline rather than assumed: I computed sha1 for each candidate and confirmed membership, since a reconstructed hash has the right shape and the wrong content. 12 passed across the three baseline guards. Related: #17689 records that `autobot-frontend`'s locales do not satisfy the lifting rule and that no gate checks them, while the gate runs for the SLM app only. That asymmetry is why a source inconsistency surfaced as a failure in the consumer. Four of its instances are fixed here; the issue stays open for the rest. Refs #15008 * fix(secrets,analytics): own-property kinds, the copied secret's identity, and the third stale-rows path (#15008, #17976, #17983) The three remaining review findings. All real; the third was mine twice over. 1. A PROTOTYPE NAME PASSED AS A CANONICAL KIND `type in CATEGORY_META` and `CATEGORY_META[type]` both see INHERITED names, so a server-supplied kind of `toString` or `constructor` reached `t(meta.singularKey)` with `singularKey` undefined instead of taking the unknown-kind fallback. Both sites now use `Object.hasOwn`. 2. THE ACTIVITY LOG NAMED THE WRONG SECRET `copySecretValue` awaited `navigator.clipboard.writeText(...)` and then called `logViewedSecretUsage('copy')`, which reads `viewingSecret.value` itself. Opening another secret while the write was pending left the clipboard holding the FIRST value while the activity recorded the SECOND — a usage log that names the wrong subject is worse than none, because it is the record someone later audits. The subject is now captured before the await and passed in; `logViewedSecretUsage` takes it as a parameter defaulting to the current selection, so the reveal path is unchanged. 3. THE THIRD STALE-ROWS PATH, WHICH I HAD MISSED `failDuplicates` was wired into both endpoint fetchers and NOT into `loadDuplicates`, where the background-task branch only set the state. `DuplicatesSection` checks `duplicates.length > 0` BEFORE `scanState`, so rows from an earlier scan stayed on screen and hid the failure — the same defect, in the one path I did not touch when fixing the other two. AND THE TEST FOR IT WAS WRONG FIRST My first test called `getDuplicatesData()` and passed with the fix reverted. The branch lives in `loadDuplicates()`; in the other function `duplicatesSilent`'s `onError` — which I had already fixed — clears the rows before that code runs, so the test exercised the wrong path and proved nothing. Found by mutation, not by reading: reverting the fix left 15 passing. Retargeted to `loadDuplicates()`, it now fails on the revert and passes on the fix. It also carries a CONTROL: the first scan is asserted to have really populated (state `done`, one row), so the later empty list is a change rather than a no-op. Evidence duplicateScanState 15 passed (was 14) mutation: revert the dupTask branch to state-only -> that test fails, alone colour + file-size ratchets 63 passed eslint 0, vue-tsc no errors in either file SecretsManager.vue 2775 and useAnalyticsDataFetchers.ts 704 — both exactly at their recorded ceilings, so no ratchet entry moves. The explanations were folded onto the lines they annotate to pay for the code; the reasoning is here. Refs #15008, #17976, #17983
mrveiss
added a commit
that referenced
this pull request
Oct 10, 2026
…every B615 suppression is enumerated with its reason (#13034) (#17974) * fix(ai-ml): the dynamic model loads consult the pinned registry, and every B615 suppression is enumerated with its reason (#13034) * fix(frontend): one accent owner, one attribute, one type (#18066) (#18068) * fix(frontend): one accent owner, one attribute, one type (#18066) * fix(frontend): swatches derive from accents.css, and the guard uses the canonical comment stripper (#18066, #17941) * fix(frontend): one runtime accent list, validated stored accents, one base-token owner (#18066) * test(frontend): lower the ceiling-total pin with useTheme.ts (#18066) * fix(skills): the data_db location comes from SSOT config, not a rebuilt path (#18060) (#18069) * fix(skills): the data_db location comes from SSOT config, not a rebuilt path (#18060) * test(guards): bind the data_db sweep to a reach floor (#15928) * test(guards): pin the data_db sweep to a fraction of its reference population (#15928, #17914) * test(guards): record the data_db sweep's declared scope (#17844) * test(guards): decide path-vs-prose on the AST, not the characters (#18060) * test(guards): require an ast.Load read, and register the scope independent of run order (#18060, #17844) * refactor(validation): one ValidationResult, five concepts named for what they are (#18070) (#18072) * refactor(validation): one ValidationResult, five concepts named for what they are (#18070, #18071) * fix(validation): import ValidationResult from the package's public surface (#18070) * test(hooks): lower the repo-wide print pin with the docstring example (#1082) * test(validation): pin the #18071 syntax-error path against the real types (#18070, #18071) * fix(tests): drop the obsolete code_intelligence stub and real-load the canonical types (#18070) * chore(deps): bump the ai-stack all-dependencies group, with the sqlalchemy hold (#18045) Nine dependency floors in autobot-infrastructure/shared/docker/ai-stack/requirements-ai.txt, plus the sqlalchemy<2.1 hold from #18061. Merged on all three owner-merge conditions: 11 of 11 required contexts SUCCESS, 0 unresolved review threads, 0 behind base. The green is about this file rather than incidental -- five repo_tests gates read requirements-ai.txt (ai_stack_manifest_agreement_17242, ansible_requirements_parity, test_deploy_constraint_rewrite_14272, test_optional_extras_are_not_hard_deps_14279, ansible_manifest_resolution) and security.yml scans it. huggingface-hub >=1.32.0 -> >=2.1.1 crosses a major version and was checked rather than waved through: requirements-npu.txt already declares huggingface-hub>=2.1.1 with transformers>=5.18.0 and records the reason inline ("5.17.x declares huggingface-hub<2.0"). The ai-stack manifest held the old pair, so this is a drift correction and transformers>=5.17.0 -> >=5.18.0 moves with it exactly as that comment requires. Two declarations of one package that had drifted -- the pattern #18060 tracks. The red on this PR for the past day was never a failure on this content: the PR head was pinned at a0031e923c while the branch had advanced to 284ead4128, so every check shown was a RUN_RESULT: cancelled artifact from the abandoned commit. update-branch refused with "head sha did not match the current head ref"; merging main in server-side resynced the head and took it to 0 behind. * refactor: retire ten symbol-fork clusters — 364 to 354 (#12771) Ten baseline entries retired: SearchResult, get_config, get_config_section, ServiceStatus, SourceType, get_slm_db, MeshDB, release, Capability, MessageType. Four of ten clusters were name collisions needing distinct names; one (get_slm_db) was real copy-paste duplication and was consolidated to a single definition imported by its six call sites. The detector advises "merge the others into one" for every cluster; acting on that mechanically is how distinct concepts become a union type, so each cluster was triaged field by field first. Three channels were found where a rename that is internal in Python is observable outside it, each the hard way: - OpenAPI schema component names (reverted two renames; the exposed SourceType is the GITHUB/LOCAL enum in codebase_analytics, not the schemas_analytics one that looked obvious -- identified by matching the generated members, not by guessing) - route-handler function names, which become OpenAPI operationIds (api.ts updated, 8 lines) - a plain Enums str() reaching a serialiser: StandardMessage.to_json is json.dumps(asdict(...), default=str), so the class name is on the wire. Renaming it changed every agent header from "MessageType.REQUEST" to "AgentMessageType.REQUEST" with no test failing. Fixed by swapping which definition keeps the name -- the wire-bearing plain Enum keeps it; the (str, Enum) in gateway/types, which serialises as its value, became GatewayMessageType. Nine reference forms broke during the work, two of them structurally invisible to any name-based sweep: module attribute access through an import alias, and a bare symbol string in setattr. The gate that actually catches these is running the co-located tests of every changed file. Guards fixed rather than worked around: an identity assertion that a blanket rename would have collapsed to assert X is X; a line-pinned baseline that reported one unchanged call as both new and stale; a line-pinned anchor in THREAT_MODEL.md; a docstring that became a restatement of its renamed function; and 35 pre-existing blocking-IO violations plus two over-long functions that the touched-file hooks require paid. Also fixed two pre-existing defects found en route: a TypeError on both ChatWorkflowResult fallback paths (research_results is not a field, so the error path crashed when the workflow had already failed), and three implicit-Optional signatures. The RBAC middleware consolidation was attempted here and reverted; it needs its own PR, and everything learned is recorded on #18088. Closes #18083 Refs #12771, #18084, #18085, #18086, #18087, #18088 * fix(deps): hold huggingface-hub below 2.0 — >=2.1.1 is unsatisfiable (#17847) #18045 raised all three huggingface-hub declarations to >=2.1.1. That floor cannot resolve, so pip fails on the whole manifest and nothing installs -- the live ai-stack was still on its pre-#18045 set (hub 1.32.0, transformers 5.17.0, langchain 1.4.2, torch 2.14.0, fastapi 0.141.1), i.e. all nine floors from that bump unmet, with the service healthy only because it never changed. Measured on PyPI: sentence-transformers 6.1.0 requires huggingface-hub <2.0.0,>=1.3.0 and tokenizers 0.23.2 requires huggingface-hub <2.0,>=0.16.4. No released version of either allows 2.x, and requirements-ai.txt holds both. Proven by resolve, with a control so the pass carries information: merged combination -> ResolutionImpossible (tokenizers vs hub>=2.1.1), exit 1 hub held <2.0 -> exit 0 All three declarations return to >=1.32.0,<2.0 -- autobot-tts-worker spells it huggingface_hub, which is why a hyphen-only grep finds two of three. An explicit <2.0 ceiling replaces the bare floor so a grouped bump cannot raise it silently again, which is what happened in #18045 and, per the file comment, in #10416 before it. transformers>=5.18.0 is left as #18045 set it: 5.18 declares huggingface-hub <3.0,>=1.31.0 and tokenizers <0.24.0,>=0.23.1, both satisfied, and the dry-run resolves with it. Separately observed and not addressed here: the npu-worker installed set (hub 0.36.2, transformers 4.38.2, tokenizers 0.15.2) is far behind its manifest, matching the long-standing #10416 breakage. Refs #17847, #18045, #10416 * refactor: retire the AnalysisResult and register symbol-fork clusters — 354 to 352 (#12771) Two clusters, both unrelated concepts sharing a name. AnalysisResult conversation_analysis/types.py keeps the name base_analyzer.py -> MultiLangResult llm_pattern_analysis/data_models -> PatternAnalysisResult register llm_shared/observability/registry.py keeps the name api/codebase_analytics/orphan_clone_detector -> register_clone_detector services/orphan_storage_cleanup_action -> register_cleanup_action autobot-slm-backend/_slm_stub_lifetime -> register_stub_lifetime In both clusters a call site had already invented the disambiguating names, so these renames DELETE four aliases rather than adding vocabulary. register four signatures take 1, 0, 0 and 3 arguments and write to four unrelated registries, so no two were the same concept. Not externally observable: neither name appears in the generated api.ts, in a prefixed-duplicate form, or as a route handler, so no schema component and no operationId moves; none of the four register definitions is an enum, so the str()-on-the-wire channel does not apply. Five reference forms moved with the renames: a module attribute on a module built by spec_from_file_location (autobot-slm-backend/conftest.py), module attributes in two test files, two QUOTED forward annotations on one line in base_analyzer.py:400, and a single-line __all__ that a multi-line pattern misses (flake8 F822 caught it). A fake pluginmanager own def register(self, plugin, name) was deliberately left alone. Two of those attribute sites were in the sweep output and were missed by piping it through head -- the second time truncation has hidden what a sweep existed to find. Uncapped, one sat at position 46 of 120 lines, nearly all unrelated reg.register(...) object calls. Also decomposed nothing and added nothing: ConflictResolver was untouched here. 825 backend and 10 slm tests pass. The only CI red was the Semgrep SARIF upload step -- bandit and semgrep themselves passed, SAST is not a required context, and the step cleared on re-run. Refs #12771 * refactor: retire the ResearchResult symbol-fork cluster — 352 → 351 (#12771) Refs #12771 * fix(tooling): wire the required-contexts gate, close its verdict set, and share the check-runs walk (#16044) Closes #16044 Closes #16120 Refs #15995 Refs #13986 * refactor: retire the OptimizationRecommendation fork — four audiences, 351 → 350 (#12771) Refs #12771 * fix(chat): one spelling for the LLM protocol tags, and a tool call stops leaking whatever bracket it wore (#18065) Refs #18065 * fix(voice,ci): sanitize speech by parsing, and let the CodeQL gate see the ref under test (#18065) Refs #18065 * fix(frontend): secrets UI hides unenforced controls, derives kinds from the API, restores usage logging; analytics stops reading a failed scan as 0 duplicates (#16450, #15008, #17976, #17983) (#17977) * fix(secrets): hide the access controls the server never enforced, and derive the secret kinds from the API (#16450, #15008) #16450: SecretsManager offered Visibility (private/shared/group/ organization/system) plus Organization ID / Team IDs / Shared-With, but SecretCreateRequest has none of those fields and dropped them silently -- a user who chose 'Shared with alice' got a secret governed by scope alone. Per the owner's decision the controls are hidden until #10088 enforces them: the form fields, their state, validation, payload fields, the visibility badge and its helpers are gone. #15008: SecretType is now components['schemas']['SecretCreateRequest'] ['type'] from the generated API types (types/secretKind.ts), not a 7-of-10 hand list. SecretsManager's categories are a Record<SecretType, ...>, so a kind added to the backend enum fails type-check until it has a category; oauth_refresh_token and connector_oauth_token gain one. Category and kind labels, and the quick-add template descriptions, are translated in all 11 locales; getTypeLabel no longer derives a singular by trimming an 's'. .secrets.baseline: 7 audited false positives (locale labels under keys named api_key/password), added without line numbers per RATCHET_BASELINES. * test(secrets): tracked reasons for the 7 audited locale baseline entries (#15008) repo_tests/secrets_baseline_reasons_guard_test.py requires every .secrets.baseline entry to carry a specific reason. The seven new entries are the Secrets page's translated category headings for api_key/password; each reason was confirmed by re-hashing the literal label (plain SHA1 reproduces the stored hashed_secret). Kept as a compact table so the entries module stays under MAX_LINES. * fix(analytics): a failed duplicate scan is its own state, never "0 duplicates" (#17983) The backend answers HTTP 200 with {"status":"error"} when the scan did not run (#17982). Three frontend paths read only the duplicates list and turned that into an empty result: the live endpoint (raw.duplicates ?? []), the cached endpoint (null with no no-data handler, so the initial [] stayed) and the background scan (data.duplicates, status unread); getDuplicatesData then announced 'N found'. composables/analytics/duplicatePayload.ts reads a payload by status into done / not_scanned / failed, and all three paths go through it. The panel shows a translated failed or not-yet-scanned message (11 locales) and keeps 'no duplicates' for a scan that ran; getDuplicatesData reports the failure. Audit of the same file: declarations and hardcodes only return success/no_data today, but their readers are now status-aware too, so a future 200 error cannot read as 'none found'. useAnalyticsDataFetchers.ts shrinks 707 -> 694. * fix(analytics): a duplicate scan that fails with an HTTP error is also "failed" (#17983) `#17983`'s AC1 -- a failed scan must never read as "0 duplicates" -- held for one of the two ways the scan fails and not the other. `applyDuplicates` only ever runs on a 2xx, so it correctly reads a 200 body of `{"status":"error"}` as `'failed'`. But the endpoint also answers **404** for a source it cannot resolve (`api/codebase_analytics/endpoints/duplicates.py`), and `useFetchEndpoint` throws on `!response.ok` (`useFetchEndpoint.ts:249`) -- so that lands in the catch and reaches `onError`, which nothing was wired to. The hook has supported the callback all along (`useFetchEndpoint.ts:84`); both duplicates endpoints passed `onSuccess` and stopped there. With nothing wired, the state kept whatever it already held: first load, then 404 -> 'not_scanned' ("No duplicate scan has run") successful scan, then 404 -> 'done', WITH THE PREVIOUS SOURCE'S ROWS The second is the one worth the fix. `'not_scanned'` at least says no scan has run; a stale `'done'` asserts a result that is not this source's, under a scan that just failed. Neither shape ever rendered a literal `0`, so the headline defect the PR fixed stays fixed -- this is the remaining hole in the same acceptance criterion, not a regression of it. `failDuplicates` clears the list as well as setting the state: `'failed'` with a populated list is a fourth state no panel renders, and the stale rows would otherwise survive behind the failure notice. The notify path at `:445` already gated on `duplicateScanState.value === 'failed'`, so wiring the state is all that was missing for it to report the failure correctly too. Evidence 13 passed (was 10). Mutation-tested, each mutant killed by the intended test: remove onError from both endpoints -> all 3 new tests fail set the state but do NOT clear list -> only the stale-list test fails The stale-list test carries a CONTROL: it asserts the first scan really did populate (`'done'`, 1 row) before the 404, so the clear is a change of state and not a no-op on an already-empty list. eslint 0 on both changed files. The file lands at 704 lines against its recorded ceiling of 707: the first attempt was refused at 711, and the explanatory comment was compressed rather than the ceiling raised -- the full reasoning lives in this message, which is the right place for it. The guard is bidirectional, so landing UNDER the old ceiling is also refused: 707 -> 704 in both mirrors (`scripts/frontend_file_size_known_large.py` and `repo_tests/frontend_file_size_ratchet_baseline.py`), because an unlowered ceiling re-licenses the lines just cut. Found by a review pass on #17977 and confirmed by reading the three call sites rather than taken on report. Refs #17983 * fix(analytics): a timed-out duplicate scan is a failure, not "0 duplicates" (#17983) Found by running the live backend, not by reading code. `POST /duplicates/analyze` on the AutoBot source times out and answers HTTP 200 with: {"status":"partial","message":"Analysis timed out after 120.0s. Try a higher min_similarity threshold.","duplicates":[],"total_count":0, "storage_type":"timeout"} `readDuplicatePayload` mapped `status === 'partial'` with ANY array to `'done'`, so that rendered as "no duplicates found" — the single answer this module was written to prevent, surviving in the one branch nobody separated. It is also the shape this codebase ACTUALLY produces, so the panel has been reporting a clean bill of health for a scan that never finished. `partial` means "timed out carrying what I had", which makes its emptiness the whole question: success + [] -> done the scan ran and found nothing: the one case allowed to render "none found" partial + [rows] -> done a real, incomplete result; show the rows partial + [] -> failed the scan learned nothing error -> failed unchanged anything else -> not_scanned Evidence 14 passed (was 13). The new row is the verbatim live payload, added to the existing `it.each` table so it sits beside the `success + []` case it must be distinguished from — the two differ only in the status string, which is exactly why one was mistaken for the other. Scope note: this closes the last hole in #17983's AC1 that I know of. Two earlier gaps on this branch are already fixed — the 200 `{"status":"error"}` body, and the HTTP-error path where `applyDuplicates` was wired to `onSuccess` only so a 404 kept a stale `'done'` with the previous source's rows. Two further defects on the same endpoint are NOT fixed here, because they are backend and this file is frontend: * `refresh=true` returned the cached timeout artifact in 547ms instead of re-running. * the timeout advises raising `min_similarity`, which `POST /duplicates/analyze` does not accept — only `GET /duplicates` does. The advice is unactionable from the endpoint that emits it. Refs #17983 * chore(ratchet): frontend ceilings follow the files, net -171 (#17983, #15008) Three grandfathered entries were out of step with the tree after this branch merged current main. Caught locally before pushing, so CI never saw them: SecretsManager.vue measured 2775, ceiling 2947 -> 2775 (-172) CodebaseAnalytics.vue measured 1297, ceiling 1295 -> 1297 (+2) useChatStore.ts measured 1176, ceiling 1174 -> 1176 (+2) MAX_CEILING_TOTAL 184521 -> 184350 (-171) SecretsManager.vue is the real reduction: #16450 removed the Visibility, Organization, Team and Shared-With controls because nothing enforced them, and the guard is bidirectional -- leaving the ceiling at 2947 would re-license the 172 lines just cut. TWO CEILINGS GO UP BY 2, WHICH IS A DECISION AND NOT AN OVERSIGHT The standing rule is "split, don't raise the ceiling", so this is stated rather than buried. The additions are the fix itself, and there is nothing padding them: CodebaseAnalytics.vue `:scan-state="duplicateScanState"` (prop binding) `duplicateScanState,` (destructure) useChatStore.ts `import type { SecretType } …` `type: SecretType` + its one-line note Each is a single entry in an already multi-line prop list / destructure / import block. There is no comment to compress and no blank line to reclaim, so the only way to hold both files at their frozen size would be to delete something that matters or to drop the wiring that makes the bug fix reachable. I raised them deliberately, 2 lines each, and the aggregate still falls by 171 -- the ratchet turns down overall, which is what it is for. The same call was flagged on #17969 for `environment.py` (694 -> 698 for a log-injection fix) rather than passed over quietly; this is the same judgement, recorded the same way. If the preference is to hold the per-file ceilings absolutely, the wiring would have to move to another file and that should be its own change. Evidence 21 passed on frontend_file_size_ratchet_test (was 2 failed / 19 passed). Both mirrors byte-identical on every entry, verified by diff. 25 passed across duplicateScanState, DuplicatesSection.scanState and SecretsManager.access -- the suites this branch exists for, re-run after the merge of current main. Refs #17983 * fix(i18n): re-lift the SLM locales after this PR changed the source app (#15008) `SLM Frontend Check` was red, and it is this PR's doing through a coupling that is invisible from the diff. `scripts/lift_locale_translations.py:68` sets SOURCE_LOCALES = Path("autobot-frontend") / "src" / "i18n" / "locales" so `--app autobot-slm-frontend/src/locales` reads **autobot-frontend's** locales as its SOURCE (`:164`, `:168`, `:177`). This PR adds 33 keys to all 11 autobot-frontend locale files, which changes what the lifting rule produces for the SLM app — so the SLM files stopped matching, in an app this PR never touches. Re-lifted. The check now exits 0. WHAT CHANGED IS AN IMPROVEMENT, NOT CHURN No keys are added or removed: 54 insertions, 54 deletions, the same 4-6 keys in each of 10 locales, replacing untranslated English with the translations this PR added upstream. For example in `pl.json`: authMethodKey "SSH Key" -> "Klucz SSH" certificate "Certificate" -> "Certyfikat" serverHost "Server Host" -> "Host serwera" A review of this PR had noted `'SSH Key'` and `'Server Host'` as remaining hardcoded strings and judged them acceptable; the PR gave them keys (`templates.sshLabel`, `templates.serverLabel`), and the SLM app inherits the translations through the lifting rule. That is the rule working. TWO WRONG READINGS ON THE WAY HERE, BOTH CORRECTED BY MEASUREMENT `ci-why` reported the failing path as `autobot-slm-frontend/src/locales/...`, which is where the check POINTS but not where the cause lives. Taking that at face value, I was about to report that the gated app had regressed and that #17689 — which records the SLM app as PASSING and autobot-frontend as failing — had inverted. Running the check locally disproved it: on a tree off current main the SLM app passes. Then the same command failed on THIS tree and passed on the other, with the SLM locales and the script byte-identical between them. That contradiction is what pointed at `SOURCE_LOCALES`: the input that differed was the source app, which only this branch changes. Evidence before: lift_locale_translations.py --app autobot-slm-frontend/src/locales --check -> "these locales are not what the lifting rule produces: ar, de, es, fa, fr, he, lv, pl, pt, ur" rc=1 after: rc=0 diff: 10 files, 54 insertions, 54 deletions, no keys added or removed Related: #17689 records that the gate is wired for `autobot-slm-frontend` only, so a change to the UNGATED source app breaks the GATED consumer's check. That asymmetry is the reason this surfaced here rather than on the app that changed. Refs #15008 * fix(design-system): each category colour resolves inside its own getCssVar (#17560) `python-suite shard 10/12` was red on `hardcoded_colour_ratchet_17560_test.py`, three ways at once: pinned counts grew 10 colours, each +1 new literals appeared #0ea5e9, #14b8a6, #ec4899, #f97316 total rose 65 > 55 The cause was the shape of `CATEGORY_META`, not a missing token. Each entry held the token and the hex as SEPARATE properties: api_key: { …, colorVar: '--color-primary', fallback: '#6366f1' } and resolved them later with `getCssVar(meta.colorVar, meta.fallback)`. WHY THE CODE CHANGED AND NOT THE GUARD My first instinct was that the ratchet was wrong — the pair resolves lazily, so it follows `[data-theme]`, and I assumed the inline form the ratchet accepts (`getCssVar('--token', '#hex')` in `credentialTemplates`) was evaluated once at import and therefore worse. That would have made the guard the thing to fix. It is not true. Both sit inside `computed()`, so both re-resolve on a theme change. The pair bought nothing functionally and cost the check: a static checker can only tie a literal to a token when the two are LEXICALLY together, and splitting them across properties makes the association invisible to any checker, not just this one. So `color` is now a thunk — deferred exactly as before, hex back inside its call: api_key: { …, color: () => getCssVar('--color-primary', '#6366f1') } … color: meta.color() Had I changed the guard instead, I would have weakened a design-system check to accommodate a form with no advantage. Worth recording, because "the guard is wrong" was the more comfortable conclusion and it was the one I reached first. Evidence hardcoded_colour_ratchet_17560_test 42 passed (was 3 failed) frontend_file_size_ratchet_test 21 passed SecretsManager.access.test 8 passed eslint 0, vue-tsc no errors in this file SecretsManager.vue lands at 2775, exactly its recorded ceiling, so no ratchet entry moves. The conversion is line-neutral; the only growth was my explanatory comment, which went from five lines to one and then onto the existing line above rather than raising the pin. The reasoning lives here instead. Refs #17560 * fix(i18n): one translation per English string, so the lift stops writing English (#15008) A review caught a regression I introduced and had reported as an improvement. It was right, and my error was sampling: I read `pl.json`'s diff, saw three English placeholders become real translations, and generalised that to all ten locales and every changed key. I never checked whether any change went the other way. Measured properly, the previous commit was 44 improvements and **10 REGRESSIONS**: settings.securitySettings.aPIKeys x2 translated -> "API Keys" loginView.password "كلمة المرور" -> "Password" addNodeModal.password, setupWizardView.password, sharedLinksView.colPassword, fleet.nodeCard.authMethodPassword, settings.admin.*.password, settings.securitySettings.password An Arabic UI showing "Password" is a user-visible defect, and it was mine. ROOT CAUSE: THE SAME ENGLISH STRING, TRANSLATED TWO WAYS `lift_locale_translations.py` matches by exact English text and needs a unique translation or a MAJORITY_THRESHOLD (0.8) consensus; otherwise `_lift` returns None and `build_locale` writes the English. The source held near-identical variants of one string, so no candidate reached the threshold: ar "Password" 'كلمة مرور' vs 'كلمة المرور' 1:1 -> English pt "Password" 'Palavra-passe' vs 'Senha' 1:3 -> English (0.75 < 0.8) es "API Keys" 'Claves API' vs 'Claves de API' 1:1 -> English pt "API Keys" 'Chaves API' vs 'Chaves de API' 1:1 -> English A definite article and a stray "de" were enough. Where the source agreed — es/de "Password", 4 of 4 — the lift worked fine all along. So this is a duplication defect, not a translation gap: one concept with two spellings, drifted far enough to defeat a consumer. Canonicalised to one value per English string per locale, across EVERY source key carrying that string (14 keys over 3 files), which also translates the ones that were plain English and makes each candidate set unanimous. Ties went to the value the SLM app already shows users (`كلمة المرور`, `Claves API`, `Chaves API`) so canonicalising cannot itself change a rendered string; `pt` "Password" went to the 3-of-4 majority `Senha`. Evidence before this commit: 44 improvements, 10 regressions after: 52 improvements, 0 regressions lift --check: rc=0 autobot-frontend check:i18n rc=0, check:locales rc=0 (all locales complete) colour + size ratchets 63 passed Carried along, because canonicalising changed the flagged strings: `.secrets.baseline` SHRANK 1402 -> 1397 entries (replacing the literal "Password" with a translation removes a Secret-Keyword hit), and two `_SECRETS_CATEGORY_LABELS tuples in `repo_tests/secrets_baseline_reason_entries.py` went stale because their hash is of the label text. Updated to the canonical labels, with the new hashes READ from the baseline rather than assumed: I computed sha1 for each candidate and confirmed membership, since a reconstructed hash has the right shape and the wrong content. 12 passed across the three baseline guards. Related: #17689 records that `autobot-frontend`'s locales do not satisfy the lifting rule and that no gate checks them, while the gate runs for the SLM app only. That asymmetry is why a source inconsistency surfaced as a failure in the consumer. Four of its instances are fixed here; the issue stays open for the rest. Refs #15008 * fix(secrets,analytics): own-property kinds, the copied secret's identity, and the third stale-rows path (#15008, #17976, #17983) The three remaining review findings. All real; the third was mine twice over. 1. A PROTOTYPE NAME PASSED AS A CANONICAL KIND `type in CATEGORY_META` and `CATEGORY_META[type]` both see INHERITED names, so a server-supplied kind of `toString` or `constructor` reached `t(meta.singularKey)` with `singularKey` undefined instead of taking the unknown-kind fallback. Both sites now use `Object.hasOwn`. 2. THE ACTIVITY LOG NAMED THE WRONG SECRET `copySecretValue` awaited `navigator.clipboard.writeText(...)` and then called `logViewedSecretUsage('copy')`, which reads `viewingSecret.value` itself. Opening another secret while the write was pending left the clipboard holding the FIRST value while the activity recorded the SECOND — a usage log that names the wrong subject is worse than none, because it is the record someone later audits. The subject is now captured before the await and passed in; `logViewedSecretUsage` takes it as a parameter defaulting to the current selection, so the reveal path is unchanged. 3. THE THIRD STALE-ROWS PATH, WHICH I HAD MISSED `failDuplicates` was wired into both endpoint fetchers and NOT into `loadDuplicates`, where the background-task branch only set the state. `DuplicatesSection` checks `duplicates.length > 0` BEFORE `scanState`, so rows from an earlier scan stayed on screen and hid the failure — the same defect, in the one path I did not touch when fixing the other two. AND THE TEST FOR IT WAS WRONG FIRST My first test called `getDuplicatesData()` and passed with the fix reverted. The branch lives in `loadDuplicates()`; in the other function `duplicatesSilent`'s `onError` — which I had already fixed — clears the rows before that code runs, so the test exercised the wrong path and proved nothing. Found by mutation, not by reading: reverting the fix left 15 passing. Retargeted to `loadDuplicates()`, it now fails on the revert and passes on the fix. It also carries a CONTROL: the first scan is asserted to have really populated (state `done`, one row), so the later empty list is a change rather than a no-op. Evidence duplicateScanState 15 passed (was 14) mutation: revert the dupTask branch to state-only -> that test fails, alone colour + file-size ratchets 63 passed eslint 0, vue-tsc no errors in either file SecretsManager.vue 2775 and useAnalyticsDataFetchers.ts 704 — both exactly at their recorded ceilings, so no ratchet entry moves. The explanations were folded onto the lines they annotate to pay for the code; the reasoning is here. Refs #15008, #17976, #17983 * fix(forks): retire the AntiPattern name forks, unbreak two directory-scan endpoints, exclude the ansible mirror — 350 → 330 (#12771) (#18097) * refactor(forks): exclude the mandatory ansible mirror and name two protocols — 350 to 332 (#12771) Two changes, both about the count MEANING something. 1. 16 OF THE 350 WERE NEVER FORKS (#16401) Ansible ships `autobot-slm-backend/slm/agent/` to the target host from `.../ansible/roles/slm_agent/files/slm/agent/`, and `ansible/tests/detect_agent_code_drift_test.py` FAILS if the two trees differ. So every symbol there is defined twice BY DESIGN, and the only ways to "consolidate" one are to break the payload or to break that test. `duplication-guard.yml:149` already excludes the path for exactly this reason, in these words: "there is nothing here this guard can ask anyone to deduplicate." This detector did not, so the baseline carried 16 entries that can never legally reach zero: AgentVersion, HealthCollector, RoleDefinition, RoleDetector, SLMAgent, append, build_heartbeat_payload, build_listening_ports_list, build_role_report, get_agent_version, get_listening_ports, initialize, mark_synced, read_unsynced, reset_version_instance, sd_notify A ratchet whose population includes entries nobody may touch reports a floor it cannot reach, and points whoever works it at a change that must not be made. I hit that directly: `AgentVersion` looked like an ideal first target — two definitions, same line number, byte-identical files — and consolidating it would have broken the deployment payload. My own estimate of the mirror's contribution was 17. The detector says 16, and the detector is authoritative; the number here is its count, not my arithmetic. 2. TWO PROTOCOLS NAMED FOR THE CLASSES THEY DESCRIBE `topology_routing.py` declared `AgentConnection(Protocol)` and `AgentTopology(Protocol)` — the same names as the concrete `@dataclass` and class in `topology.py`, in the same package. One name meant two different things: a seven-field record and a one-field structural view of it. The decoupling is right and is KEPT — depending on `to_agent` alone is what makes the router independent of the dataclass. Only the names change, to say what each protocol requires: AgentConnection(Protocol) -> RoutableConnection (needs `to_agent`) AgentTopology(Protocol) -> CollaboratorSource (needs `get_collaborators`) Contained to one file: both were file-local, used on four code lines, and only `TopologyAwareRouter` is imported elsewhere. The remaining prose mentions of `AgentTopology` in the module docstring are correct — the router really does query the concrete class. Evidence detector: 332 clusters over 2821 parsed files, none new and none stale against a baseline of 332, exit 0 17 passed on symbol_fork_ratchet_17312_test (was 15) 14 passed on topology_routing_test + test_topology_routing_wiring flake8 0, black clean The exclusion is mutation-tested and LOAD-BEARING — removing it fails three tests, including `test_the_population_matches_an_independent_derivation`, because the control re-derives the same exemption with its own spelling. A control that skipped it would answer a looser question and call the disagreement a detector bug. A second guard pins the 16 names out of the baseline by name, because the failure mode is someone pasting them back after a scan run before this existed. Refs #12771 * refactor(forks): one AntiPatternType — finish GH#6757's SSOT, 332 to 331 (#12771) GH#6757 declared code_analysis.src.anti_pattern_detector.AntiPatternType the canonical SSOT and merged this package's members into it (29 members, a strict superset of the local 19 with every shared value identical), then pointed the anti_pattern_detector facade at it. It stopped one file short: anti_pattern_detection/types.py kept its diverged 19-member copy, and the package __init__ re-exported *that* one. So the same name resolved to two distinct Enum classes depending on the import path — the facade handed callers the canonical 29, the package handed them the local 19 — and Enum identity is per-class, so GOD_CLASS from one path compared unequal to GOD_CLASS from the other. Every detector in the package tagged its results with the local class while pattern_analysis/analyzer.py consumed them via the package path. types.py now re-exports the canonical enum instead of redefining it. All 17 member references across code_intelligence resolve against the canonical 29; no cycle (code_analysis/src/__init__.py is import-free and the canonical module pulls only stdlib + autobot_shared, which this package already imports). * fix(code-intelligence): name the suite analyzer, and two directory-scan endpoints stop raising (#12771) `AntiPatternDetector` named two different capabilities. GH#6757 made `code_analysis.src.anti_pattern_detector.AntiPatternDetector` the SSOT and repointed the `code_intelligence.anti_pattern_detector` facade at it, but the package's own class kept the same name — so which class a caller got depended on whether they imported the package or the facade. They are not duplicates and must not be merged: only `analyze_file` is common. The canonical owns the per-file and cross-file rules over a whole-codebase index; the package's walks a directory with caching, delegating to the four category detectors it composes. So the package's class is renamed for what it composes — `AntiPatternSuiteAnalyzer` — rather than consolidated away. Two endpoints in api/code_intelligence.py were already broken by that redirect, both asking the facade-resolved canonical detector for `analyze_directory`, which it does not define: - `:714` also passed `exclude_dirs`, which the canonical `__init__` does not accept — it takes `redis_client` only, so that call raised twice over - `:902` `get_codebase_health_score` `run_isolated` dispatches `getattr(cls(**init_kwargs), method_name)(...)`, so each raised before reaching any analysis. Both now use the suite analyzer, which has the method and the constructor argument and returns the `AnalysisReport` whose `.anti_patterns` these call sites already expected. Swept every other facade consumer: `codebase_analytics/analyzers.py:952` and `code_evolution_miner.py:643` call only `analyze_file`, which the GH#6757 shim covers, so those were unaffected. The threshold constants read off the name at `:111-200` exist on both classes, so those were never broken. The facade's `except ImportError` fallback is removed rather than updated: it could no longer be reached, because `anti_pattern_detection.types` now imports the canonical enum, giving the package and `code_analysis.src` one shared precondition. It was also substituting a class with a different API for the canonical name. Guard: `anti_pattern_offload_contract_test.py` checks statically (ast, no imports — `code_intelligence` is conftest-stubbed, so an import-based test would assert against a MagicMock) that each offloaded call's method and constructor kwargs exist on the class named, that the canonical name is not redefined in the package, and that it never passes by matching nothing. Verified against the pre-fix code: the method check and the kwarg check each fail there. The three grandfathered files are left at their exact recorded ceilings (2156, 772, 1091) — the guard is bidirectional, so prose was compacted to fit rather than any ceiling being moved. Baseline 331 -> 330. * test(code-intelligence): resolve the guard's paths from the test file, and pin the call count (#12771) Both from review on #18097. Paths came from the process working directory, so running pytest from `autobot-backend` looked for `autobot-backend/autobot-backend/...` and raised FileNotFoundError. They now derive from `Path(__file__).resolve().parents[2]`, verified to be the repository root by its `.git` and `CLAUDE.md` markers, so the checks run from either location. (The relative-path idiom was copied from `unwired_tracker_test.py`, which has the same fragility.) `assert checked` only caught matching *nothing*. It is now `checked == _EXPECTED_OFFLOAD_CALLS`, so a call site that silently disappears fails too — the count is pinned rather than merely non-zero, and the constant is named so it is updated deliberately. Also recorded why this is a second file rather than more cases in `api/code_intelligence_offload_wiring_test.py`: that module covers a different failure mode (#12866, the offload conversion leaving the local scanner un-run) and works by patching `run_isolated`. A patched `run_isolated` never executes the real `getattr(cls(**init_kwargs), method_name)`, so no case added there can reveal that the class named lacks the method — the dispatch under test is the mock. It is also `pytest.mark.asyncio` at module level, which does not fit these synchronous checks. Control-checked again after the change: both assertions still fail against the pre-fix code, and the call count is 2. * fix(chat,memory): UTC on the wire, {} not null for message metadata, and an unknowable duration stays unknowable (#13856, #13280, #13344) (#17963) * fix(chat,memory): UTC on the wire, {} not null for message metadata, and an unknowable duration stays unknowable (#13856, #13280, #13344) Three data-handling defects in the chat/memory layer, batched because they are one scope and share no file with any open PR. #13856 — session listing emitted naive LOCAL ISO strings `datetime.fromtimestamp(...)` with no `tz=` at four sites in `chat_history/session_listing.py`. Every `parse_utc_iso` consumer then tags the value as UTC, so it is wrong by the host offset and the sign flips with the host's side of UTC. Now `tz=timezone.utc`, which is the convention the rest of the tree already uses and the `+00:00` form canonicalised by `autobot_shared/time_utils.py` (#5169) — Riga time is a display convention for humans, not a wire format. `updatedAtEpoch` (#13948) stays: it is a number, needs no parser, and the two now agree to the microsecond. #13280 — the metadata writer persisted null `_build_message_dict` wrote `"metadata": raw_data` with `raw_data: Any = None`, so every message added without metadata stored an explicit null — the producer behind the reader crashes #13220 fixed. Now `{}`, via `is not None` so that `0`/`""`/`[]` a caller actually passed survive. The #13220 reader guards are kept and annotated: sessions already on disk are never rewritten, so a defence against historical data is not dead code. #13344 — a negative duration aborted the tracked operation `complete_task` handed a negative `duration_seconds` to the validating `update_task_status`, whose `ValueError` escaped through `TaskExecutionTracker.track_task` and failed the caller's real work. The cause of `started_at > now` is NOT established and is not guessed at here: hypothesis 2 of #13344 is ruled out (`astart_task` blocks on its commit) and hypothesis 1 is not demonstrable (every production writer stores aware UTC and the round-trip is lossless, pinned by a new test). What does not depend on the cause is that an unknowable duration must not be invented: new `TaskExecutionRecord.elapsed_seconds` records `None` and logs both instants and the delta. Not clamped to 0 — a zero is a measurement, and #13344 asks explicitly that the skew not be hidden. #13344 stays open for the cause. Each fix has a regression test that was mutation-checked against its own reverted fix, plus against the wrong fix where one exists (truthiness instead of `is not None`; clamping to zero). The #13856 guard parses the AST rather than grepping, and carries a contrast fixture where the required string appears only in prose. * fix(memory,chat): a naive/aware timestamp pair is unknowable, not a crash (#13344) Three review findings on #17963. **1. `elapsed_seconds` raised where it was supposed to degrade (Major).** `datetime - datetime` across the naive/aware boundary raises `TypeError`, and that subtraction sat *above* the negative-duration fallback — so the fallback could never run, and the exception aborted the tracked operation. That is exactly the failure #13344 exists to prevent, reached through a different door: the function's own docstring promised an unknowable duration "must not abort the tracked operation", and it did. The mismatch is now detected before the subtraction and returns `None`, the same answer the negative case gives. It deliberately does **not** assume UTC for the naive side: picking a zone would invent the measurement this function exists to refuse to invent, the same reasoning that rules out clamping to `0`. Normalising at the write boundary is the real repair, filed as #18103. Verified the guard against the raw subtraction across all four combinations — it blocks exactly when `datetime - datetime` would raise and allows both consistent pairs, so a naive/naive delta is still measured. Pinned by `test_a_consistently_naive_pair_still_measures`, so a later broadening to "reject every naive instant" fails rather than silently discarding a known duration. Both unmeasurable branches now share `_log_unmeasurable`, so their wording cannot drift apart. The existing log assertions still hold: the reason string carries the skew magnitude the old message interpolated. **2. The writer test reached into the API layer.** `_RecordingHistory` overrode `load_session`/`save_session`/`_save_history` but not `_announce_message`, so `add_message` deferred an import of `api.session_events` and awaited the real publisher. It swallows its own failures, so it could not fail the test — but a *writer* test should not depend on notification side effects, and the fixture already declared `announced` and never populated it. Now overridden, recording into that field. **3. `_build_message_dict`'s documented contract was false.** The docstring claimed "the stored shape is always a mapping". It is not: only `None` becomes `{}`, and `0`/`""`/`[]`/`False` survive verbatim — which the PR's own `test_falsy_but_supplied_metadata_survives_verbatim` asserts. The real contract is "never null", and a reader needing a mapping must still check. Said that way, because the overclaim is the kind that invites deleting a null-defence. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(backend): three runtime crashes — a missing KB method, two attributes lost in a split, and a deleted module still imported (#13569, #13811, #13558) (#17968) * fix(backend): three runtime crashes — a missing KB method, two attributes lost in a split, and a deleted module still imported (#13569, #13811, #13558) Each path raised the moment it executed, and each is repaired against what the code was established to have meant rather than silenced. #13569 — `get_document_count` was awaited at four sites and defined at none. Added to `DocumentsMixin`, delegating to `get_stats()["total_documents"]`: "document count" already has one definition here, since `StatsMixin._populate_redis_stats` fills documents, vectors and chunks from a single vector-store count and `add_document` writes one embedding per document. The tempting near-miss, `_count_facts`, counts Redis `fact:*` keys — a correct answer to a different question that `get_stats` publishes as `total_facts`. #13811 — #716 split the memory-graph monolith and dropped two `__init__` assignments while their readers came across intact. `knowledge_base` is restored as an injected optional (its pre-split default was `None`) and `search_cache` as the LRUCache it was. A missing attribute is not a falsy one: `create_entity` wrote the entity to Redis and then raised on the guard, so the row persisted while the caller got a 500. #13558 — `fast_app_factory_fix` was deleted in #567; `get_chat_sessions` and `get_settings` now take the live app from `request.app`. Both failed silently — one swallowed by `return_exceptions=True`, the other by its own `try` — so the chat UI initialised empty with nothing in the logs. `batch_load` is deliberately left: making it live is an authentication decision, not a crash fix (see the PR). Tests parse the AST rather than matching text, because this very change leaves the dead module's name in prose in both files; each guard carries a positive control and a prose-only contrast fixture. `batch_jobs.py` lands at exactly its 964-line ratchet ceiling, so neither size record moves. Refs #13569, #13811, #13558, #17966, #17967 * test(memory): assert the property-graph mirror carries name and type (#13811) #13811's third verification item names the mirror specifically, and its null-name symptom is what an unmirrored node produces. Asserts the node PROPERTIES rather than that add_node was called: a mirror running with empty props reproduces the original /api/graph-rag/path output exactly. * chore(types): regenerate api.ts for the chat-init docstring (#13558) `verify-generated-types` was red because this PR's own change moved the OpenAPI schema. `batch_jobs.py:851` adds a docstring explaining that ``request`` lets the helpers reach live app state and adds no field to either schema (#13558) -- FastAPI publishes a route's docstring as the endpoint `description`, so the committed `api.ts` no longer matched. The two hunks are the same three lines twice: the chat-init path is registered for both GET and POST, so the description appears once per operation. Not hand-written. This is `generated-types.patch` from the `autofix-types` run on this head, applied with `git apply` -- the workflow stopped pushing the fix itself (#15362) because that invalidated every check already running, so it publishes the patch as an artifact instead. Hand-patching a generated file is how a plausible-looking diff gets committed that the generator would not produce; this is the generator's own output. Corrected on the way here: this red was first read as stale-base drift, because the PR's file list contains no `api.ts` and nothing matching `chat.*init`. The chat-init endpoint is DEFINED IN `batch_jobs.py`, which is in the list -- so the grep was accurate about filenames and silent about the question that mattered. Updating the branch to 0 behind was still needed and cleared the other two type jobs, but it was never going to clear this one. Verified: the applied text matches `autobot-backend/api/batch_jobs.py:851` on this branch, so the generated file and its source now agree. Refs #13558 * test(api): the assertion's message was a discarded tuple element (#13558) Review on #17968. The line read: manager._get_chats_directory.assert_called(), "never reached app.state..." Python evaluates that as a 2-tuple expression and throws it away. The `assert_called()` call still runs, so the check itself was never broken — but `assert_called()` takes no message argument, so the string was dead and would never print on failure. It read like `assert x, "msg"` without the `assert`. The rationale is now a comment and the assertion stands alone, so the explanation survives where a reader will see it. Swept the repo by AST rather than regex (a regex matched legitimate cases where the string is a genuine argument, as in `assert_awaited_once_with(key, "name")`): 11 more bare `(call(), "string")` statements exist, all assert_*-shaped, across 9 files. None is a falsely-passing test — the assertion executes in every case — so they are a diagnostic defect, filed as a class rather than swept into this PR, whose scope is three runtime crashes. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * security(trio): neutralise the traversal log value, wire the remote-approval path fail-closed, and let the secret model express ownership (#13602, #14068, #13051) (#17969) * security(trio): neutralise the traversal log value, wire the remote-approval path fail-closed, and let the secret model express ownership (#13602, #14068, #13051) `path` straight into a log line. The log is inside `except ValueError`, so it fires only on a real rejection; the defect is the raw value, which can carry CRLF (forging records), ANSI (driving a terminal reading the log) or megabytes of padding. New `sanitize_log_value` in the module that owns log lines. reached from the real approval path via a Gateway-backed sender, and a reply must come from a sender recorded on the delivery: unknown sender, empty allowlist or unidentified sender all resolve nothing. Delivery is a mirror -- every failure path returns False and the in-app gate still decides, denying on timeout. carries them. They are self-asserted, so a guard pins that no access decision in the legacy store reads them until they are resolved from the caller. Each fix carries an AST-parsed regression guard with a prose-only contrast fixture, and four grandfathered ceilings move with the lines they record. * fix(security): bound the approval send, refuse a negative log bound, and make three guards check what they claim (#13602, #14068, #13051) Review on #17969, plus the two real CI failures. **CI red 1 — the ceiling audit.** `code-quality` failed at `check_python_file_size.py --audit-ceilings`, which walks every tracked file; the pre-commit hook only sees staged ones, so the plain guard passed locally while the audit did not. My conflict resolution was the cause: the rebase conflict showed five pin entries, but the branch had also bumped `schemas_system.py` and `env_registry.py` in hunks git auto-merged, and taking main's whole file reverted those too. Both pin files now carry the measured values, in parity. **CI red 2 — THREAT_MODEL.md anchors.** The branch added two lines above the cited definitions in `redaction.py`, so four anchors were stale by exactly +2. Re-pointed to measured positions: `redact_text` :104, `redact_mapping` :116, `redact_cloud_identifiers` :133, `redact_provider_error` :147. **[1] The env var moves to a sibling module.** `env_registry.py`'s own docstring says a new variable goes in an `env_registry_<component>.py` sibling, and `env_registry_logging.py` records the mechanical reason: that file sits at its ceiling with no slack and the ratchet forbids raising one to make room. So the inline registration could not land there at all — the review finding and the red were the same defect. New `env_registry_security.py` holds it; `env_registry.py` goes 1228 -> 1215, and `ENV_VARS.md` is regenerated. **[4] A negative log bound is refused.** `AUTOBOT_LOG_VALUE_MAX_CHARS=-1` made `limit >= 0` false and skipped truncation, letting a rejected request put its whole path in the log — the opposite of the function's purpose, and it falsified the docstring's "can never be padded out by its subject". A guard has no "unlimited" reading, so a negative bound falls back to the default, and to 0 if the default is misconfigured negative too. Verified across None/0/10/-1/-999. **[6] The approval send is bounded.** `WebSocketAdapter.send_message()` awaits `websocket.send_json()` with no application-level timeout. The caller mirrors *before* starting in-app approval polling, so an unbounded send did not merely lose the mirror — it stopped the in-app gate reaching its own timeout, and the fail-closed denial that gate exists to produce never happened. Bounded inside `deliver_approval` rather than at the call site, because `forget()` is already in scope there and the existing failure path does exactly what the finding asks. **[2] Raw bidi/line-separator characters become escapes.** ` ` and `` were literal, which Ruff flags (PLE2502/RUF001); verified the escapes denote the same code points. My first scan missed the ` ` entirely because `str.splitlines()` splits on it — the instrument was defeated by the very character the test exists to neutralise. **[3] The call-site guard requires the sanitiser, not any call.** It accepted any `ast.Call` as proof of sanitising, so `logger.warning("...%s", str(path))` passed with the raw path reaching the log; and it never inspected the format string, so `logger.warning(f"...{path}")` passed by leaving no positional arguments to check. It now requires `sanitize_log_value(...)` and rejects an f-string interpolating an unsanitised piece. Both bypasses added as contrast fixtures. **[8] The wiring test requires an `ast.Await`.** A test named "...is_awaited..." accepted a bare `mirror_approval_request(...)` — the regression that builds a coroutine and sends nothing. It now matches `ast.Await` whose value is the call, with an unawaited fixture as the control. Two findings are filed rather than fixed here: the delegated-helper gap in the ownership guard (no authorisation breach established) and the mirrored-approval correlation collision, which is latent because `resolve_from_reply` has no production caller — verified against `mirror_approval_request` as a wired control. * fix(ci): fetch the CI system packages once per run instead of once per shard (#18102) (#18110) * fix(ci): give the resumable apt download its own budget, and make a provisioning failure say whether it was progressing (#18102) Four shards failed provisioning on 2026-10-08 across two branches, each burning ~16 minutes and reporting red having run no tests. Re-running cleared every one, which is the remedy #15515 records — but a defect whose remedy is "run it again four times a day" is not fixed, and the codebase is ours whether or not the change that tripped it was. **The exit code identifies the cause, so this is not the budget guess #14550 rejected.** That objection was that an undersized budget was one hypothesis among several, lock contention being likelier. The alternatives are now ruled out: apt is configured to drop a connection idle for APT_HTTP_IDLE_S=30 and to retry a failed fetch APT_FETCH_RETRIES=3 times, so a stalled mirror ends an attempt with apt's own non-zero status at roughly 120s. What all four occurrences showed was exit 124 — coreutils timeout killing it at the full 300s — on every attempt. A connection that never idled 30s was receiving data; it was simply too slow to finish inside the bound. So the download phase gets its own bound, and fewer tries: APT_DOWNLOAD_TIMEOUT_S=540 # was APT_TIMEOUT_S=300 APT_DOWNLOAD_ATTEMPTS=2 # was APT_ATTEMPTS=3 Fewer-and-longer is the right shape *because* `--download-only` resumes: attempt 2 continues from what attempt 1 fetched, so three restarts of a 300s budget buy less than two of 540s. The install phase keeps 300s deliberately — a kill there interrupts dpkg, which is what the repair between attempts exists for, and that risk does not improve with a longer wait. **Cost, stated honestly.** A single exhausted download phase goes from ~15 to ~18 minutes, with each attempt gaining 80% more time to finish. The all-five-phases- fail worst case goes from ~79 to ~84 minutes — both already beyond the 60-minute job budget, so that case was never survivable and this does not create it. **#18102 AC 1: a failure now carries its own evidence.** `apt_cached_bytes()` reports `/var/cache/apt/archives` before and after each attempt, so the log says whether bytes arrived. A too-small bound and a stalled mirror end with the same exit code; the delta separates them, and the next occurrence is diagnosable from its own log instead of inferred from configuration as it was here. `APT_TRIES` as a command prefix is scoped to the one call — verified, because POSIX mode would make it persist and quietly drop the installs to two tries. This step runs `bash -e`, not `bash --posix`. Verified: YAML parses, and the extracted 85-line provisioning script passes `bash -n`. Refs #18102, #15515, #14550, #13896 * fix(ci): fetch the CI system packages once per run instead of once per shard (#18102) The timeout change in the previous commit was a band-aid and is kept only for the one fetch that remains. This is the actual defect: **twelve shards each ran the same ~100MB apt fetch**, so one CI run made that download up to twelve times and a different shard drew the slow mirror every time — four reds on 2026-10-08 were shards 4, 1, 2 and 6, each burning ~16 minutes and reporting red having run no tests. Raising a per-attempt bound does nothing about twelve independent exposures to the same unreliable third party. **New `provision-packages` job** fetches once and publishes the archives as the `ci-apt-debs` artifact. `python-shard` gains it as a `needs:` and downloads it before the setup action, which now installs with `--no-download` — so a shard touches the network for system packages **not at all**. Exposure goes from 12 fetches per run to 1, and a provisioning failure becomes a single named job instead of a random 1-of-12 shard red that reads like a test failure. **It runs the setup action unchanged rather than carrying its own copy of the apt retry logic.** A second copy of that logic is exactly the kind of fork this repo is retiring, and it would then need fixing twice. Running the whole action also warms the venv cache the twelve shards read, so the apparently redundant work is not wasted. **The prefetched path is optional by design.** `coverage.yml` and `test-durations.yml` call this action without a provision job, so an absent directory falls through to the fetch exactly as before. An artifact that cannot satisfy the install also falls through, with a warning — a stale artifact must not cost the suite. `continue-on-error` on the download and `if-no-files-found: ignore` on the upload keep a missing artifact from failing a shard; the fallback is the old behaviour, not an outage. Installed exactly as downloaded — ffmpeg without recommends, tesseract-ocr with them — so the cached set matches what apt resolves in the shard. **Cost:** the shards wait for one job (~1-2 min, less when the venv cache is warm) instead of starting immediately. That buys the removal of a ~16-minute flake hitting roughly four times a day. Verified: both YAML files parse; the extracted 117-line provisioning script passes `bash -n`; `provision-packages` is present, `python-shard needs: [changes, provision-packages]`, and the shard's second step is the artifact download. `download-artifact` pinned `@v8` to match the five existing uses. Refs #18102, #15515, #14550 * fix(ci): measure the apt archive cache with privileges, and say "unknown" when the measurement fails (#18102) Review on #18110, and it lands against the instrument rather than the fix. `apt_cached_bytes()` ran `du` unprivileged. apt stages in-progress downloads in `/var/cache/apt/archives/partial/` at mode 0700, so the runner user cannot read them and `du` counted only completed files — which, for a transfer killed mid-flight, is nothing. The whole purpose of this measurement is to tell a slow mirror from a stalled one. Unprivileged, it would have printed `0 -> 0` and named a **stalled** mirror on precisely the failure where bytes were arriving — reporting the opposite of the truth, and falsifying a correct diagnosis. The PR body claims this change is "falsifiable by its own log"; with that bug, `0 -> 0` was the default reading rather than a finding. Now `sudo du`, so `partial/` is counted. It reports **`unknown`**, not `0`, when the measurement itself fails. A `0` from a failed command is not a measurement (`docs/developer/MEASUREMENT_DISCIPLINE.md`), and collapsing the two would reintroduce the same confusion by another route — the reviewer suggested `0`, and this deviates deliberately. The `|| echo 0` being replaced could not have worked in any case: the exit status of `du | cut` is cut's, so a failed `du` still exited 0 with empty output and the fallback never fired. Same family as the pipe-exit-status trap. Verified across all five paths — normal output `41288704`, a genuine zero `0` (preserved as a real measurement), command failure `unknown`, empty output `unknown`, permission-noise-only `unknown`. YAML parses; the 132-line extracted provisioning script passes `bash -n`. Refs #18102 * fix(ci): resolve the cached install against fresh indexes, not the image's stale ones (#18102) Review on #18110, and it is a real hole in the batching change rather than a nit. `--no-download` resolves the version to install from the runner's own package indexes. The prefetch branch sat BEFORE `apt-get update`, so a shard resolved against whatever indexes its image shipped with, while the provisioning job had downloaded against updated ones. A GitHub-hosted image can carry older indexes, and when the shard's stale index selects a version other than the archive on disk, `--no-download` cannot satisfy it — the shard then takes the network fallback. That failure is silent by construction: the fallback is deliberately a warning, not an error, so the twelve downloads this change exists to remove would have quietly come back and the PR would still have looked like it worked. Fixed by relocating the branch to after the `update`, so both sides resolve against freshly updated indexes. The update itself is advisory and small; the ~100MB of packages is what was being repeated twelve times, and that is still removed. Order is now: update -> install from prefetched archives -> (fallback) download -> install. Ver…
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.
Thinking Path
Three secrets-UI issues, all in
components/security/SecretsManager.vueand its kind vocabulary.SecretCreateRequesthas no such fields (bug(secrets): SecretCreateRequest.owner_id/org_id/team_ids/shared_with accepted but silently dropped (api/secrets.py) #13051), so the values were silently dropped and a "shared with alice" secret was governed byscopealone. Owner decision: hide until enforced. The controls come back with Umbrella/PRD: unify AutoBot secrets — one RBAC-governed store, vaults + one key abstraction (replaces 11 stores, 5 keys, 2 UIs) #10088's server-side enforcement, and the decision is recorded on security(secrets): Visibility/Organization/Team/Shared-With controls in SecretsManager.vue are not enforced server-side #16450, bug(secrets): SecretCreateRequest.owner_id/org_id/team_ids/shared_with accepted but silently dropped (api/secrets.py) #13051 and Umbrella/PRD: unify AutoBot secrets — one RBAC-governed store, vaults + one key abstraction (replaces 11 stores, 5 keys, 2 UIs) #10088.SecretVault.vue, which the issue names, was folded intoSecretsManager.vueby wire in SecretAuditLog, consolidate SecretVault into SecretsManager #16485, so the fix lands there.)logSecretUsagehas had no caller since. It is wired back in.What Changed
validateAccessLevelCombination, the four payload fields, the visibility badge with its helpers and CSS, and the edit-time population.owner_idis untouched: it has no control and is out of scope.SecretsManager.vueshrinks by about 190 lines.visibility*i18n keys are kept for Umbrella/PRD: unify AutoBot secrets — one RBAC-governed store, vaults + one key abstraction (replaces 11 stores, 5 keys, 2 UIs) #10088.types/secretKind.ts:SecretType = components['schemas']['SecretCreateRequest']['type'].useSessionActivityLoggerre-exports it.CATEGORY_META: Record<SecretType, …>inSecretsManager.vue, so a kind added to the backend enum fails type-check until it has a category.oauth_refresh_tokenandconnector_oauth_tokengain categories.getTypeLabelno longer derives a singular by trimming an "s".revealand copying it logscopy. Server-supplied kinds are checked againstCATEGORY_META(unknown →other), not cast.SessionSecret.type(stores/useChatStore.ts) is nowSecretType; the 5-kind narrowing inlinkSecretToSessionis gone..secrets.baseline, following RATCHET_BASELINES' scan/audit/strip, as a minimal diff:api_key/password, each checked by line,is_secret: false, noline_number.useSessionActivityLogger.ts.components/__tests__/SecretsManager.access.test.ts, mounting the real component):#secret-scope.revealis logged once (not on hide),copyis logged after the clipboard write, and an unknown kind is logged asother.Closes #16450
Closes #15008
Closes #17976
Closes #17983
{"status":"error"}, bug(codebase-analytics): duplicate detection reports 'nothing scanned' as a completed scan — the one tool the dedup programme needs returns nothing for AutoBot's own codebase #17982) as "0 duplicates".composables/analytics/duplicatePayload.tsreads a payload by status into done / not_scanned / failed. The live endpoint, the cached endpoint and the background scan all go through it.DuplicatesSectionshows translated failed and not-scanned messages (11 locales); "no duplicates" stays only for a scan that ran.getDuplicatesDatareports the failure.useAnalyticsDataFetchers.ts707 → 694,CodebaseAnalytics.vue+2.duplicateScanState.test.ts(reader cases, plus each fetcher fed a real 200 error payload). Mutation-checked: 3 of its tests fail against the old fetchers. AlsoDuplicatesSection.scanState.test.ts.Verification
5916a126bc:vue-tsc (autobot-frontend): 0 errors,vitest (autobot-frontend): all relevant tests pass,pytest: all relevant tests pass.SecretsManager.access.test.ts8/8,useChatStoresuites 29/29, i18n untranslated ratchet 13/13,oxlint/eslintrc=0.code-reviewer: APPROVE. The import-placement nit is fixed. The finding thatlinkSecretToSessionmaps new kinds toapi_keyis fixed under wire(secrets): reveal/copy session logging (#608) lost its caller when SecretVault folded into SecretsManager (#16485) #17976. That function has never had a caller in history; this is recorded on wire(secrets): reveal/copy session logging (#608) lost its caller when SecretVault folded into SecretsManager (#16485) #17976 as needing a design decision about where a "link to session" action belongs.Model Used
Claude Opus 5.5 (
claude-opus-5-5); review subagent on Sonnet.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements