Repository navigation
Conversation
… was 14 files from red (#15928) TWO THINGS, and the first is urgent. hooks-path-override was 14 files from breaking CI for everyone. Measured on d0d94a7: four-glob 6384 + 29 extensionless shell files = 6413 population against floor=6027, so slack 386 against an allowance of 400. The next PR adding 15 tracked .sh/.py/.yml/.yaml files turns that guard red on every open branch at once, for a reason none of them caused. Ratcheted to 6413 — slack 0, headroom back to 400. Nothing reported that. The floor was correct when set and decayed silently, which is why Reach now exposes headroom(): a guard is allowed to sit close to its allowance, but not for that to be invisible. THE RULE MOVES ONTO THE PRIMITIVE. verify_floor() and population() are now methods on Reach, and reach_declarations_test discharges the obligation rather than re-implementing the predicate. The rule belongs to declare(); it was living in whichever test remembered to enumerate declarations. It is still NOT checked at declaration time, and that is a ruling rather than an omission. declare() runs at IMPORT: the root is not known yet, each discovery shells out to git ls-files, and a raise there takes the whole module down before any test can report it — _import_every_guard swallows it into IMPORT_FAILURES, so the declaration never reaches REGISTRY and every parametrised case for that guard is silently lost. That is the precise failure this module exists to catch. Two live declarations would break immediately under an eager check, including self-check::always-empty, which by construction has population 0 and floor 1. population() memoises on (name, resolved root). The same walk was paid three times a session; it is now twice, so owning the check costs less than not owning it. TWO GAPS IN THE SWEEP ITSELF, both the defect applied to the module that names it: MIN_DECLARATIONS was 2 against a live 4 — a floor two below its population, inside the file that forbids floors below population. Now 4. test_every_declaration_is_reached_by_this_sweep is new. pkgutil.iter_modules reaches top-level repo_tests modules only, so a declare() in a subpackage registers nothing, its floor is never verified, and its absence is indistinguishable from having nothing to verify. It now greps every declare() call under repo_tests/ and fails on any name the sweep did not import. NOT IN THIS CHANGE: the 33 hand-rolled floors outside _reach (AC3). Each needs its own discover() run — copying constants across would reproduce the defect at 33x scale, which is what the EXPECTED_SKIPS 200 -> 0 episode already demonstrated. Filed separately.
…rd why the claim primitives stay separate (#15957) The owner ruled that services/task_claim.py and autobot_shared/coordination/ work_claims.py stay separate, permanently. This makes that ruling survive the next author who notices two similar primitives and reads them as duplication. `task` was never a valid work_claims kind, but it was only *absent* from VALID_KINDS. Absent produced "unknown scope kind" -- the answer a typo gets -- so a caller reaching for `task:` could not tell a decision from a gap, and was told only that the kind had not been heard of. RESERVED_KINDS now refuses it with the reason and where task identity belongs instead. Both entry points that name a kind go through one helper, `_require_kind`. list_claims had its own copy of the check, so `task` could have been reserved in Scope.parse and merely unknown in list_claims. One helper is what makes the answer the same wherever the question is asked. This cannot turn a passing call into a failing one: `task:` raised ScopeError before and raises ScopeError after. Only the message changes. No caller asserted on the old message -- checked repo-wide -- and the `task:read` scopes in run_jwt are JWT scopes that never reach Scope.parse. The split is recorded in ARCHITECTURE_EXCEPTIONS.md, and task_claim.py now carries the docstring half that work_claims.py:48-50 already had. The two reasons are both stated: task identity is not work scope, and task_claim emits audit that work_claims structurally cannot, since autobot_shared must not import from autobot-backend. Tests assert on the message rather than on the exception, because `task:` raised before too and a bare pytest.raises would pass against the old code and pin nothing. Each has a contrast: an unknown kind must still say "unknown", and no kind may sit in both sets.
…RESERVED_KINDS, and correct two relayed figures (#15957) Addresses the REQUEST CHANGES review on #16215. Every finding was re-measured before acting on it rather than relayed in turn. HIGH -- a false claim in the record meant to be trusted without re-checking. The task_claim docstring and the ARCHITECTURE_EXCEPTIONS entry said task_claim "emits audit on every outcome, including redis_unavailable and redis_error". claim_task does. renew_claim and release_claim audit only where Redis answered; their fail-open branches return with no audit at all. Both now say exactly that, and the gap is filed as #16217 rather than papered over. MEDIUM -- "roughly 40 lines of Lua" was never measured, and came through two sessions before landing here. Measured: task_claim has 10 non-blank lines (release 5, renew 5), work_claims' matching release and renew have 12, and its 35-line acquire script has no counterpart because claim_task uses SET NX EX. The record now carries the measured, dated figures and notes the correction instead of silently replacing the old ones. MEDIUM -- a third entry point bypassed the shared check. branch_stewardship.prune formatted a caller-supplied kind straight into an index key, so kind="task" -- or a misspelling -- read an index nothing writes to and returned []. That reads as "nothing to prune": a clean answer to a question never asked. It now calls _require_kind before any I/O. Its tests use no Redis fixture, so they cannot be skipped where fakeredis is missing. LOW -- RESERVED_KINDS was a plain dict beside a frozenset, so any caller could pop("task") and un-reserve it at runtime. It is now a MappingProxyType. Neither figure correction weakens the ruling: the mergeable Lua is a quarter of the stated size, and work_claims still structurally cannot emit audit.
… cannot keep current (#15957) The delta re-review noted the docstring named Scope.parse and list_claims as the entry points, omitting branch_stewardship.prune, which the same PR routed through the helper. A list of callers in a docstring is prose not derived from the code: it goes stale the moment a caller is added, and it did, within the PR that wrote it. The docstring now states the property instead of the membership.
… route left open (#15745) Four of the nine routers in KNOWN_UNGATED exposed configuration writes or operational control with no authentication of any kind. Each now takes the house admin gate -- `auth_middleware.check_admin_permission`, used by 129 routers, raising 401 unauthenticated / 403 non-admin -- per the authorization model decided and recorded on #15745. api/redis.py router-level POST /config (Redis config write) + 3 api/developer.py router-level POST /config (dev-mode config write) + 3 services/knowledge_sync_service.py router-level daemon start/stop, manual sync + 3 api/wake_word.py per-route 13 of 14; POST /check stays open wake_word cannot take a router-level gate: the repo owner ruled that POST /check stays open, because the local voice client calls it before any user session exists. So the other thirteen are gated per-route and the module docstring states the exception and its reason, which is what #15745 AC1 asks for -- "gated, or intentionally open with the reason stated in the file". That docstring is load-bearing, not decoration. The enumerator's `gated` is ANY-route: `classify()` appends a mechanism the moment it finds one gate name. So a router gated on 13 of 14 routes reads as fully GATED and the open route becomes invisible. The marker makes the verdict GATED+intentional -- the same state `api/system` is in -- which is accurate rather than hole-hiding. KNOWN_UNGATED only shrinks, and now does by exactly these four. Deliberately NOT in this change: - api/frontend_config. It was ruled "intentionally open -- document it" on the premise that the SPA needs it before login. It returns `NetworkConstants.get_host_configs()` (autobot_shared/network_constants.py:226), which lists every VM's internal address and port. A marker saying "open by design" over that would record a disclosure as a decision, so it stays in KNOWN_UNGATED until the decision is revisited with that fact. - api/transcriber (#15758) and the three knowledge routers: different risk. The first needs its DEFAULT_USER fallback replaced with an ownership check; the others need tenant scoping, not just authentication (#15745 AC4). Separate PRs. - #15738: user CRUD is authenticated; its defect is missing role/ownership checks. Authz, not ungated. Verified before commit: - enumerator on this tree: redis/developer/knowledge_sync GATED (router-level), wake_word GATED+intentional; its ungated set equals the new KNOWN_UNGATED exactly, so the stale-entry and new-entry checks both hold - all four files: `check_admin_permission` and `Depends` imported (checked by AST, not by eye), parse, isort --check-only and black --check clean - no import cycle: auth_middleware imports nothing from api/ or services/, and four services/ modules already import it - zero frontend callers for redis, developer, wake_word; knowledge/sync appears only in the generated OpenAPI types, not a call site - knowledge_sync_service.py 567 lines against MAX_LINES 600; no ceiling needed
…eet's addresses (#15745)
…way and pool tests (#16224) Every fixed asyncio.sleep in these three files that stood between starting a background task and asserting what it did now waits on the task itself, via autobot_shared.eventually: - test_autobot_agent_adapter.py: nine sleeps become eventually(task.done); the hand-rolled 60 x 0.5s status poll becomes one eventually call. - test_gateway_manager.py: both queue tests wait until the message is processed (watching the worker), then wait_for the worker with the helper's deadline instead of swallowing a TimeoutError. test_message_queue_processing finally asserts what it processed; it asserted nothing before. - test_container_pool.py: waits for the replenish instead of 50 ms. Both grandfathered files shrink (665 -> 663, 807 -> 801) and their ceilings drop to match in both ratchet files. New guard repo_tests/fixed_sleep_then_call_assertion_test.py names any test whose own body sleeps a fixed interval and then asserts a call happened. It reads the collectable-test population (2316 modules, 30717 tests measured, floors below), skips nested helpers and lambdas, and spares assert_not_*. The wider shape (any assert after a fixed sleep, 32 sites) is #16255.
#15745) Review of 74a5abd found the docstring's reason false. It said the fleet host list is "served by the admin-gated /system/frontend-config instead, which is where the terminal reads it". api/system.py contains no `hosts` and no get_host_configs at all -- that endpoint never carried a host list. What actually happened: the terminal calls getBackendConfig(), which fetches /system/frontend-config and, on any non-OK response or exception, returns `this.config` -- the merged PUBLIC payload from /api/frontend-config (AppConfig.js). So admins always got DEFAULT_HOSTS, and non-admin callers got the backend host list through that fallback. After 74a5abd they get DEFAULT_HOSTS too. Nothing breaks, but it is a behaviour change, and I had claimed nothing consumed the field: I traced useTerminalStore to getBackendConfig and stopped, without reading the fallback in its body. - the false sentence is rewritten; the marker phrase the enumerator matches ("no authentication required by design") is unchanged - the two `# Issue #372 ... host configs` comments orphaned by the removal go Not done here: api_endpoint_migrations_test.py still asserts assertIn("hosts", source), which 74a5abd made false. That file is skipped at collection, so the assertion never runs -- and editing it trips no-local-schemas on four pre-existing test-fixture BaseModels, which do not belong in production schema files. Filed separately rather than fixed with the wrong move. Whether DEFAULT_HOSTS itself ships internal addresses in the anonymous bundle depends on the build's VITE_*_HOST values -- #16258.
…merging it (#16246) main is the default branch, so scheduled workflows and Dependabot read it, and the owner's decision on #16221 is to sync it from Dev_new_gui regularly. sync-main-to-dev.yml is already the release-sync mechanism, so it gains the schedule rather than a second mechanism being added beside it. - .github/workflows/sync-main-to-dev.yml: a weekly schedule beside the manual trigger; the "release" confirmation still gates manual runs. The release-sync-main push and the #13974 divergence guard are unchanged. The PR step now runs pipeline-scripts/release_sync_main.py, and the checkout and the PR step prefer AUTOBOT_PUSH_TOKEN so the PR's checks start. - pipeline-scripts/release_sync_main.py decides create, update-oldest or nothing across open PRs into main from release-sync-main or Dev_new_gui, and never opens a second. The body gives the commit count, the scheduled workflows the merge activates, changes or stops, and the instruction to merge with --merge, never --squash. It calls no merge endpoint. The workflow list comes from both refs' directory listings, because the compare's file list stops at 300 (measured 300 against 1474). - repo_tests/release_sync_main_test.py covers the decision, the body, the schedule detector, no-merge, and the workflow's shape. - .github/filters/python-paths.yml covers the workflow the test reads. - docs/developer/CI_BOT_ATTRIBUTION.md lists the workflow under the push token.
…5002, #15401) Severity (#14993): codeIntelligence's Severity is a checked subset of the generated canonical union; models.ts re-exports the generated RiskLevel instead of an upper-case copy; .risk-badge styles every canonical value. Roles (#14937): the codegen MANIFEST exports Role, and the store, LoginForm, router meta, constants and usePermissions all derive from it. v-permission admits superadmin through isAdminRole(); viewer and guest are gone, the signed-out set is explicit, and ASSIGNABLE_ROLES records the owner's call to keep UI assignment at admin/user/readonly. Display names (#14939): list_members resolves the whole ladder server-side, as _compose_human_nodes already did; the frontend shares displayNameOf(), and AdminUsersView's em-dash is an i18n key. Redactor (#15002): a drift test runs both copies over one case table and names both files and the diverging input; a plugin-only edit now runs it. Type collisions (#15401): the main GUI's HealthStatus / ServiceStatus / ServiceHealth copies are renamed (the SLM keeps the names) and leave the cross-GUI guard's allowlist. The SLM's ServiceHealth and ServiceStatus now match what the agent actually sends. Pre-commit findings in touched files: get_org_chart was already over the function-length gate and companies.py is frozen at its size, so its two enrichment queries moved to llc/services/org_chart_rows.py (as org_chart_placement did); the shrink, 1780 -> 1732, is recorded in both ratchet data modules. LoginForm's fallback role is DEFAULT_ROLE. The two #15002 path mirrors in ci.yml and frontend-test.yml wait on #16260.
A CI workflow's vendor downloads and dashboard links are not deployment config
(owner's ruling). Only the generic URL rule stands down for .github/workflows/
paths; the IP and port rules still run there, and the same URL anywhere else is
still a finding. The repo's own run link uses ${{ github.server_url }} instead
of a github.com literal. HARDCODING_PREVENTION.md no longer claims YAML is
unscanned.
…iew colours (#14939) The changed-files Stylelint job lints every file a PR touches, and the display-name work brought these two files' raw hex colours into scope. The dialog keeps its electric-blue accent through --color-electric-500/600 and its hover through --color-error-bg. AdminUsersView's table header used var(--color-surface-alt, #f9fafb), but --color-surface-alt is defined nowhere, so the near-white fallback painted the header in every theme, dark included; it now takes --bg-tertiary, the shared DataTable header token.
…t make, and guard the shape (#16302, #16303) A CodeQL query filter matches query metadata (id, tags, kind, ...), never a source file. The three "reviewed exclusions" in codeql-config.yml scoped py/full-ssrf and py/path-injection to one file each through `files:` / `paths:`, so none of them suppressed anything under either setup. #16302 (owner decision, option 1): - codeql-config.yml: the three entries become reason-only comments, applied as code-scanning alert dismissals. The upload_security.py reason names validate_upload_path() and its line numbers are corrected. The header no longer claims per-path granularity for query-filters. - CODEQL_SUPPRESSION.md: the TL;DR, the mechanisms table and section 2 no longer say query-filters suppress per path. - New guard codeql_query_filters_match_queries_16302_test.py fails on any query-filters key CodeQL does not match on (files:, paths:, reason:, ...), and on a missing or empty config. Key allowlist from GitHub Docs "Creating CodeQL query suites" and "Metadata for CodeQL queries". - python-paths.yml: the config joins the python-suite filter, so a change confined to it runs the guard. #16303: security.yml labelled the codeql-action upload-sarif pin `# v3`; its SHA is the v4.37.9 tag commit. The label now says v4.37.9.
…three that named the wrong release (#16303) image-sign.yml labelled three SHA pins one major release behind the same commits autoresearch-image.yml pins. Each tag resolves (lightweight ref) straight to the pinned commit: - docker/login-action dbcb8138: v3 -> v4.6.0 - docker/build-push-action 53b7df96: v6 -> v7.3.0 - sigstore/cosign-installer 6f9f1778: v3 -> v4.1.2 workflow_action_version_regression_test.py only required a SHA pin to carry a version-shaped label, so a label naming the wrong release passed. It now also requires every pin of one SHA, across .github/workflows/*.yml and .github/actions/*/action.yml, to carry agreeing labels: the same version, or one a component-wise prefix of the other (v4 beside v4.37.9, not v40). A planted self-test pins agreement, SHA grouping across subpaths, and non-transitive prefixes. The docstring no longer says those three actions are "genuinely pinned to different releases"; each was one commit with two labels.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
|
Retired, replaced by #16339 (train/2026-09-11-d). Train-c went red on three checks. python-suite shard 7/12 was caused by #16240 alone, and shard 11/12 by #16263 alone. Both had already failed the same shard on their own heads before the train, and each has a root-cause comment. Frontend Security Scan failed on an npm registry 400 from the retiring audits/quick endpoint; the lockfile is identical to base (#16337). Train-d carries the other 8 members unchanged, all merged without conflict onto |
…lls this PR moved behind _emit (#16246) Measured, not inferred: this test on 2eb2e1f reported 322 against base's 359. The diff touches neither the hook, its lib, nor the pre-commit config, deletes or renames no file, and every removed print()/console.* line is in pipeline-scripts/ci_dispatch_watchdog.py, whose _emit helper carries the one noqa'd print. Merge train #16321 dropped this PR on exactly this pin.
…merging it (#16246) (#16263) * ci(release): open a weekly sync PR from Dev_new_gui into main, never merging it (#16246) main is the default branch, so scheduled workflows and Dependabot read it, and the owner's decision on #16221 is to sync it from Dev_new_gui regularly. sync-main-to-dev.yml is already the release-sync mechanism, so it gains the schedule rather than a second mechanism being added beside it. - .github/workflows/sync-main-to-dev.yml: a weekly schedule beside the manual trigger; the "release" confirmation still gates manual runs. The release-sync-main push and the #13974 divergence guard are unchanged. The PR step now runs pipeline-scripts/release_sync_main.py, and the checkout and the PR step prefer AUTOBOT_PUSH_TOKEN so the PR's checks start. - pipeline-scripts/release_sync_main.py decides create, update-oldest or nothing across open PRs into main from release-sync-main or Dev_new_gui, and never opens a second. The body gives the commit count, the scheduled workflows the merge activates, changes or stops, and the instruction to merge with --merge, never --squash. It calls no merge endpoint. The workflow list comes from both refs' directory listings, because the compare's file list stops at 300 (measured 300 against 1474). - repo_tests/release_sync_main_test.py covers the decision, the body, the schedule detector, no-merge, and the workflow's shape. - .github/filters/python-paths.yml covers the workflow the test reads. - docs/developer/CI_BOT_ATTRIBUTION.md lists the workflow under the push token. * ci(release): baseline the 18 trailer commits, keep hand-opened sync bodies, document the optional push token (#16246, #16268) - .github/commit-trailer-baseline.txt: the eighteen full SHAs the owner decided to baseline on #16268, appended once with a comment citing it. No existing entry changed. The list re-derived exactly from git log --format=%H -i --grep='^co-authored-by: *t <' origin/main..origin/Dev_new_gui. - pipeline-scripts/release_sync_main.py: every generated body leads with a BODY_MARKER naming the workflow, and the weekly update rewrites a body only when it carries that marker. A hand-opened sync PR keeps the body its author wrote, even one that names the workflow file. - docs/developer/CI_BOT_ATTRIBUTION.md: AUTOBOT_PUSH_TOKEN is an optional per-repository CI secret that the product never reads; how a fork creates its own; why the sync PR's runs park under the fallback and the watchdog does not release them; the owner step at the bootstrap and every week; no promise of a release within minutes. #16272 removes the manual step. * ci(watchdog): release parked runs on the release-sync PR into main, and no other (#16272) Without a push token the release-sync PR's runs are bot-triggered and park, and the watchdog only swept PRs into Dev_new_gui, so the sync PR's required contexts never reported. - pipeline-scripts/release_sync_pull.py: the one definition of the release-sync PR (head release-sync-main, base main, this repository), shared by the sync tool and the watchdog. It imports neither, because the sync tool already imports the watchdog. is_sync_pull moves here from release_sync_main.py, which now takes its head and base from it. - pipeline-scripts/ci_dispatch_watchdog.py: check_dispatch adds that one PR to the heads it collects, so it goes through the existing collect_heads, sweep_parked_runs and _approve_head path under the same rules. No other PR into main is listed as a head. - The same file, because staging it runs the whole-file hooks over debt it already carried: its 37 bare print() calls now go through one _emit, the single no-print exception, as in ci_red_cause.py; and _run_url derives its fallback link from GITHUB_SERVER_URL instead of a hardcoded host, linking nothing when that is unset. Black reformatted the file on staging, so its size ceiling is lowered to the new count in both ratchet files. - repo_tests/ci_dispatch_watchdog_release_sync_test.py: the sync PR's parked run is approved; another PR into main, a fork's same-named branch and a run parked for a person are not; an unreadable listing raises. - docs/developer/CI_BOT_ATTRIBUTION.md: the manual step is for the bootstrap sync, and until this is live on main. * ci(release): when Actions may not open the sync PR, keep one tracking issue instead of a silent green (#16246, #15834) The owner ruled that this repository syncs by hand (#15834 Q2): no push token, and Actions may not open pull requests. The weekly run could never open the sync PR, and it warned and exited 0 every week. - pipeline-scripts/release_sync_main.py: when GitHub refuses the PR, open or update ONE tracking issue, found by exact title plus the automation label, carrying the PR body, the compare link and the command that opens the PR. Close it once a sync PR is open (a hand-opened one counts) or nothing is left to sync. A failure to read or write it exits 2. The commit count and workflow list are read from Dev_new_gui, because release-sync-main is deleted when the sync PR merges. - .github/workflows/sync-main-to-dev.yml: issues: write, because the default token here is read-only; the script now runs even when nothing is ahead, so it can close the issue, and the push stays conditional on commits. - repo_tests/_release_sync_fakes.py: the shared fakes, moved out of release_sync_main_test.py so both test files use one copy. - repo_tests/release_sync_tracking_issue_test.py: created once, updated in place, never duplicated; closed when in sync and when a sync PR is open; a failed write exits non-zero; the workflow grants issues: write. - docs/developer/CI_BOT_ATTRIBUTION.md: this repository syncs by hand; the owner opens the PR from the tracking issue, and a person-opened PR starts its checks normally. * test(hooks): lower the no-print-console pin to 322, the 37 print() calls this PR moved behind _emit (#16246) Measured, not inferred: this test on 2eb2e1f reported 322 against base's 359. The diff touches neither the hook, its lib, nor the pre-commit config, deletes or renames no file, and every removed print()/console.* line is in pipeline-scripts/ci_dispatch_watchdog.py, whose _emit helper carries the one noqa'd print. Merge train #16321 dropped this PR on exactly this pin.
Test vehicle, do not merge. This PR runs CI once over the combined state of the train's PRs. It is closed once they have landed. It replaces #16314.
Thinking Path
Every merge left about 20 open PRs behind
Dev_new_gui, and the auto-update bot re-ran each one's full CI. That cascade, not new work, was most of the 200+ queued runs, and it kept approved PRs from ever settling. GitHub's merge queue isn't available on a user-owned repository, so the owner approved a manual merge train instead. It's recorded on #15937 (comment 5632692837).The first train (#16314) carried #16308, which then went red on its own duplication-guard. Before rebuilding, I scanned every remaining member's own head for completed reds, taking the latest run per check name. Three had reds this PR caused, and each now has a root-cause comment and the
blockedlabel:infra_script_imports_resolve_test— an unresolvable_scan_helpersimport in a hook.authz-and-selection.#16257's red is an apt provisioning failure on the runner ("apt did not succeed after 3 attempts"), not a test failure, so it stays on the train. This run re-tests it.
What Changed
Branch
train/2026-09-11-c, built fromDev_new_guiatd5a0c6dcb0. Each PR's recorded head was merged through the repository merges API. All 10 merged without conflict. Tipf2e703f668, tree248e453a60. The auto-update bot stays paused until the train is finished.8f22c12d0234569bcf7ddbdc5de65fda7922ead41b9cf50e652eb2e1f715d452ce3ee9f229f5a7c08a18f6d191800aaccb35Not on this train:
Verification
This PR's own CI run is the verification: one full run over the combined tree. If every check settles green:
--match-head-commit <recorded head>.Dev_new_gui's tree is compared with this branch's tree248e453a60.A red run gets bisected by halves to find the culprit, which is dropped. Nothing was run locally.
Model Used
Claude Opus 5 (the merge-owner session).
Refs #15937
Refs #16311
Refs #16304
Refs #16296
Refs #16265
Refs #16215
Refs #16263
Refs #16271
Refs #16257
Refs #16240
Refs #16300
🤖 Generated with Claude Code