feat(approvals): one approval record, shared RiskTier, pending inbox query (#13421 S1)

[mrveiss]

Thinking Path

#13421 folds seven approval surfaces into one record. approvals is already the canonical table (#17043), so S1 extends it with nullable columns rather than adding a record type, and adds the single risk ladder and the inbox query the frontend slice (S3) builds on. Owner decisions of 2026-10-12 are followed as recorded on the issue.

What Changed

• models/approval.py: nullable source, risk_tier, expires_at, payload_hash, resume_ref; ApprovalSource enum (terminal_command, agent_tool, workflow_plan, plugin_capability, autoresearch, skill_governance, company, mcp_tool_schema).
• Migration 20261012_098 (after the single head 20260928_097): adds the five columns plus two indexes; asserts row count unchanged and every new column NULL (the 094 pattern); downgrade refuses once any row carries a value in one of them.
• Migration 20261012_099 (found on the way, see below): adds the missing task_approval_links.created_at.
• autobot_shared/risk_tier.py: RiskTier (low|medium|high|critical, asserted to be a subset of Severity values), risk_tier_from_severity, risk_tier_from_command_risk, parse_risk_tier (accepts the terminal's uppercase and command_execution.RiskLevel by member name). Grepped first: no existing approval tier. Severity is not widened; UNKNOWN raises instead of guessing; DANGEROUS/FORBIDDEN map to critical.
• services/approval_execution.py: register_resume_handler / resume_decided_approval, keyed by source, run after every decision (approve, reject, revision) when resume_ref is set; failures are recorded like bug(approvals): an approved action that fails or has no handler leaves only a log line, while the approval still reads APPROVED #17141, never raised.
• services/approval_gate_service.py: create_approval takes keyword-only record fields validated by the new services/approval_record_fields.py (sourced approval must be graded; resume_ref needs a registered handler; expires_at aware and future; payload_hash defaults to sha256 of canonical context); _transition resumes through approval_execution; list_pending is the inbox query (all sources, unscoped rows only, so no company-tenant leakage).
• api/approval_gates.py: GET /approval-gates/pending plus source/risk_tier filters on the list route; response carries all five fields. Schemas moved to new api/schemas_approvals.py because schemas_workflows.py may not grow (ceilings lowered in both records, 3011 to 2917).
• store_authority: approval concept registered (Postgres is the record).
• Generated types: autobot-frontend/src/types/generated/api.ts regenerated with the CI recipe (separate commit, diff is approval-only: +86/-3).

Security choices: the HTTP create route does NOT accept source, risk_tier, resume_ref, expires_at or payload_hash (tested): a caller-chosen source would let any user pose as the terminal, and a caller-chosen resume_ref would pick what the server resumes. Only server-side emitters set them.

Defect found and fixed here: on a freshly migrated database selectinload(Approval.task_links) (used by list_approvals and list_pending) failed with column task_approval_links.created_at does not exist; migration 099 fixes it. The same drift on seven other tables is filed once as #18392, not patched here.

For S2 (emitters), recorded as asked

• chat_workflow/approval_baseline.py on main (refactor(tools): one tool-effect catalogue, every mutating tool gated by category with an SLM-only relaxation (#18294) #18310): baseline categories are ALL valid categories minus those the SLM setting chat.relaxed_approval_categories relaxes; callers can only add (union_categories), never remove; SLM unreachable or malformed relaxes nothing (fail closed). S2e (agent tool emitter) must take its gate set from baseline_approval_categories() and must not offer a per-session or per-approval way to lift a category.
• The pending list has the same visibility as the existing list route (any authenticated user). Terminal and tool approvals belong to a chat session, so S2a/S2e must carry the owning session in context and the inbox needs a session-ownership filter (THREAT_MODEL section 2) before those sources are shown to non-owners. Not done in S1 because no emitter sets a session yet.
• expires_at is stored and published, not enforced: nothing moves a stale pending row to EXPIRED yet.
• store_authority approval declares no Redis projection; S2a must add the terminal pending projection with its rebuild, per the note on the entry.
• Moved line: services/approval_gate_service.py global-channel publish baseline entry 388 to 429 (same pre-existing finding, security: approval and checkpoint notifications broadcast workflow/session ids on 'global' #17373).

Verification

• Postgres (disposable local cluster, real alembic upgrade): test_approval_record_columns_098.py + test_approval_inbox_query_13421.py: 7 passed (rows untouched, round trip, refusal, inbox query across sources, resume through the real table).
• Unit: risk_tier_test.py 29, approval_record_test.py 19, approval_gates_inbox_13421_test.py 7: 55 passed; existing approval suites (execution, proposer, action validation, orphan chain, human decider, gate service, global-channel baseline) pass.
• Guards: symbol_fork_ratchet_17312 18 passed; python_file_size_ratchet 31 passed; ceiling_record_parity_17872 11 passed; guard_reach_meta 23 passed; store_authority 16 passed; reach_scope_claim_17844 (alone) 10 passed; docstring_restatement 6, enum_hand_copy 7, frontend_api_contract_ratchet 12 passed.
• Push: pytest part of pre-push passed; its vue-tsc step timed out at 159s, so the push used AUTOBOT_PREPUSH_ALLOW_TIMEOUT=1. The only frontend change is the generated api.ts.
• alembic heads: single head 20261012_099.

Model Used

Claude Sonnet 5.5

Single-issue rationale: S1 of the multi-slice #13421 is the schema every later slice (S2a-e, S3) builds on and it must merge first; the slices are sequenced by design, so none can ride with it.

Refs #13421

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You'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 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 90a628fc-f31f-4195-8cc9-4f2938ca47eb

---

📥 Commits

Reviewing files that changed from the base of the PR and between 13be1b4 and 8e0e123.

---

⛔ Files ignored due to path filters (3)

• autobot-frontend/src/types/generated/api.ts is excluded by !**/generated/**
• repo_tests/python_file_size_ratchet_baseline.py is excluded by !repo_tests/python_file_size_ratchet_baseline.py
• scripts/python_file_size_known_large.py is excluded by !scripts/python_file_size_known_large.py

---

📒 Files selected for processing (20)

• autobot-backend/api/approval_gates.py
• autobot-backend/api/approval_gates_inbox_13421_test.py
• autobot-backend/api/schemas_approvals.py
• autobot-backend/api/schemas_workflows.py
• autobot-backend/migrations/versions/20261012_098_approval_record_columns.py
• autobot-backend/migrations/versions/20261012_099_task_approval_links_created_at.py
• autobot-backend/models/approval.py
• autobot-backend/services/approval_execution.py
• autobot-backend/services/approval_gate_service.py
• autobot-backend/services/approval_gate_service_test.py
• autobot-backend/services/approval_record_fields.py
• autobot-backend/services/approval_record_test.py
• autobot-backend/tests/migrations/test_approval_inbox_query_13421.py
• autobot-backend/tests/migrations/test_approval_record_columns_098.py
• autobot-backend/tests/security/test_approval_human_decider_17042.py
• autobot_shared/risk_tier.py
• autobot_shared/risk_tier_test.py
• autobot_shared/store_authority.py
• repo_tests/docstring_restatement_baseline_17165.py
• repo_tests/global_channel_carries_no_tenant_payload_17354_test.py

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Notice: 29 open PRs — past the runaway threshold (25)

This is not a request to defer this PR. The actual limit is the open-PR cap (AUTOBOT_OPEN_PR_CAP, default 40), enforced at pre-push against NEW branches only -- pushing more commits to this PR's own branch is never blocked by it.

This notice only means the count is high enough to be worth a glance for a runaway — something opening PRs in a loop, or a merge pipeline that has stalled so nothing is draining.

Currently open:

• feat(llm): needs_reauth registry skip + typed error, header headroom harvest, pre-dispatch headroom routing (#15022, #15026, #15028) #18403 feat(llm): needs_reauth registry skip + typed error, header headroom harvest, pre-dispatch headroom routing (llm: degradation store cannot distinguish a dead credential from a transient failure — dead tokens retry every TTL forever #15022, llm: harvest provider rate-limit response headers into a cross-worker quota headroom store (the missing quota monitor) #15026, llm: route on observed headroom before dispatch instead of on a 429 after it; reconcile guessed RPM buckets with observed limits #15028)
• chore(train): merge train 2026-10-12 #7 — 4 approved PRs incl. sandbox hardening and audit_logs SoR (#17225) #18401 chore(train): merge train 2026-10-12 Dev new gui #7 — 4 approved PRs incl. sandbox hardening and audit_logs SoR (tech-debt(audit): #6475 closed on three parallel audit systems — there are now six #17225)
• fix(slm-auth): SSO callbacks redirect with a one-time code, not the session JWT (#18394) #18399 fix(slm-auth): SSO callbacks redirect with a one-time code, not the session JWT (security(sso): SSO callbacks redirect with the session JWT in the query string — now live after #18322 #18394)
• refactor(api): the main app's raw fetch tail goes through fetchWithAuth, and ApiClient gains an opt-in circuit breaker for polled dashboards (#12363) #18398 refactor(api): the main app's raw fetch tail goes through fetchWithAuth, and ApiClient gains an opt-in circuit breaker for polled dashboards (canonical(frontend): no single backend contract — 8 independent HTTP clients (2 generic) + 22 raw fetch sites, and generated api.ts is bypassed (4 importers vs 74 hand-typed responses) and stale (2424 vs 2152 paths) #12363)
• fix(guards): mypy resolves autobot_shared so enum aliases stay types; the embedding-literal guard covers namespaced ids and the infra builders (#18345) #18393 fix(guards): mypy resolves autobot_shared so enum aliases stay types; the embedding-literal guard covers namespaced ids and the infra builders (types: an enum alias of the canonical Severity used as an annotation is 'not valid as a type' in the backend mypy run (6 instances) #18345)
• test(tools): differential test of the tool-approval planes (#13416) #18387 test(tools): differential test of the tool-approval planes (test(security): the two tool-approval planes are never compared — add a differential assertion #13416)
• refactor(infra): classify 43 more infra scripts by runtime, repair 3 validators (#18289) #18380 refactor(infra): classify 43 more infra scripts by runtime, repair 3 validators (infra: 98 more infrastructure modules import backend-only packages — classify each as shared, backend tool or dead (#18288 follow-up) #18289)
• chore(train): merge train 2026-10-12 #6b — 6 approved PRs incl. the #18359 injection fix (#18359) #18379 chore(train): merge train 2026-10-12 #6b — 6 approved PRs incl. the security(slm): remote command injection — service_name from the URL path is interpolated unquoted into an ssh 'sudo -n systemctl' shell command (reachable by admin/operator users) #18359 injection fix (security(slm): remote command injection — service_name from the URL path is interpolated unquoted into an ssh 'sudo -n systemctl' shell command (reachable by admin/operator users) #18359)
• feat(shell): Refined UI step 2: top nav and icon cluster, Minimal/Full mode, Appearance settings, one brand mark (#18240) #18373 feat(shell): Refined UI step 2: top nav and icon cluster, Minimal/Full mode, Appearance settings, one brand mark (Refined UI step 2 · shell: top nav, UI mode, profile appearance #18240)
• security(api): RUM split into admin, user and bounded-open routes, and Prometheus metrics behind an SLM-issued scrape token (#16375) #18372 security(api): RUM split into admin, user and bounded-open routes, and Prometheus metrics behind an SLM-issued scrape token (security(api): 44 config-registered routers declare no auth gate, triaged by exposure (found by widening the router-auth guard, #16368) #16375)
• refactor(config): the #18094 de-fork remainder — one key reader, one feature flag, one language detector, and image-gen keys vault-first through an origin-bound resolver (#18094) #18371 refactor(config): the tech-debt: forked env defaults, shadowed constants and same-name functions — the remainder after #17419/#16177/#18093 #18094 de-fork remainder — one key reader, one feature flag, one language detector, and image-gen keys vault-first through an origin-bound resolver (tech-debt: forked env defaults, shadowed constants and same-name functions — the remainder after #17419/#16177/#18093 #18094)
• fix(egress): guard the backend services/ outbound calls under the SSOT policy, user-supplied URLs public-only (#17576) #18370 fix(egress): guard the backend services/ outbound calls under the SSOT policy, user-supplied URLs public-only (security(egress): rule 8's only enforcement is a five-file literal, and four connectors beside it call tracked_request unguarded #17576)
• fix(codebase-analytics): honest scan outcomes, duplication detectors wired, and forks shown from the one CI detector (#18013, #18067, #17982) #18362 fix(codebase-analytics): honest scan outcomes, duplication detectors wired, and forks shown from the one CI detector (bug(codebase-analytics): /duplicates returns a fabricated "timed out after 120s" in 0.2s — the single-flight lock is never released #18013, Codebase analytics reports 18,200 problems and zero duplication, while the fork detector finds 365 clusters in the same tree #18067, 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)
• refactor(regex): one slugify, one skills tokenizer and one word tokenizer, the no-op ANSI copy gone, and the dead command-utils template retired (#18093, #18124) #18360 refactor(regex): one slugify, one skills tokenizer and one word tokenizer, the no-op ANSI copy gone, and the dead command-utils template retired (tech-debt(backend): six regex concepts are forked across 2–8 files each, and one fork is a no-op #18093, tech-debt(code_analysis): a 190-line code-generation template whose output never landed, now stale against the file that did #18124)
• fix(slm): one SSH systemctl executor with quoted, validated units and gated service routes (#18359) #18352 fix(slm): one SSH systemctl executor with quoted, validated units and gated service routes (security(slm): remote command injection — service_name from the URL path is interpolated unquoted into an ssh 'sudo -n systemctl' shell command (reachable by admin/operator users) #18359)
• security(api): gate the 14 remaining ungated mutating routers — sign-in for reads, admin for shared-state writes (#16375) #18351 security(api): gate the 14 remaining ungated mutating routers — sign-in for reads, admin for shared-state writes (security(api): 44 config-registered routers declare no auth gate, triaged by exposure (found by widening the router-auth guard, #16368) #16375)
• refactor(knowledge): the pattern analyzer and Notion connector use the one content digest (#15722) #18350 refactor(knowledge): the pattern analyzer and Notion connector use the one content digest (canonical(knowledge): two functions named content_hash return different values — full digest vs 16-char truncation #15722)
• refactor(audit): audit_logs is the one system of record, Redis its projection — three stores retired, one write path, one retention table (#17225) #18343 refactor(audit): audit_logs is the one system of record, Redis its projection — three stores retired, one write path, one retention table (tech-debt(audit): #6475 closed on three parallel audit systems — there are now six #17225)
• security(auth): DB role permissions work, one strict-union permission gate for both services, MFA-pending tokens refused, SSO mints from the real user (#18259, #18321, #18322) #18338 security(auth): DB role permissions work, one strict-union permission gate for both services, MFA-pending tokens refused, SSO mints from the real user (security(rbac): UserService.get_user_permissions reads role.Permission, which does not exist — the DB-backed permission path raises and the middleware denies #18259, security(slm): get_current_user accepts MFA-pending temp tokens as full sessions (#17049 fixed only the backend) #18321, fix(slm-sso): SSO login passes a stand-in user without is_platform_admin to create_token_response, which raises #18322)
• refactor(chat): chat_sessions is the one durable home of a conversation turn; transcripts are read-only legacy, found where they really are (#18293) #18309 refactor(chat): chat_sessions is the one durable home of a conversation turn; transcripts are read-only legacy, found where they really are (canonical(state): every chat turn is persisted twice — chat_sessions and an undeclared transcript store, already diverged once #18293)
• chore(deps): bump the all-dependencies group across 1 directory with 15 updates #18273 chore(deps): bump the all-dependencies group across 1 directory with 15 updates
• fix(sandbox): execution and pooled containers run fully hardened, and no caller can widen their reach (#17267) #18232 fix(sandbox): execution and pooled containers run fully hardened, and no caller can widen their reach (refactor(sandbox): container hardening is copy-pasted across three Docker clients, with a docstring as the only thing preventing drift #17267)
• chore(deps): bump the all-dependencies group in /autobot-infrastructure/autobot-npu-worker/docker with 3 updates #18177 chore(deps): bump the all-dependencies group in /autobot-infrastructure/autobot-npu-worker/docker with 3 updates
• chore(deps): bump the all-dependencies group in /autobot-infrastructure/shared/docker/ai-stack with 4 updates #18176 chore(deps): bump the all-dependencies group in /autobot-infrastructure/shared/docker/ai-stack with 4 updates
• chore(deps): bump actions/upload-artifact from 7.0.1 to 7.0.2 in the all-dependencies group #18175 chore(deps): bump actions/upload-artifact from 7.0.1 to 7.0.2 in the all-dependencies group
• chore(deps): update asyncpg requirement from >=0.31.0 to >=0.32.0 in /autobot-slm-backend in the all-dependencies group #18172 chore(deps): update asyncpg requirement from >=0.31.0 to >=0.32.0 in /autobot-slm-backend in the all-dependencies group
• chore(deps): bump the all-dependencies group across 1 directory with 16 updates #18157 chore(deps): bump the all-dependencies group across 1 directory with 16 updates
• chore(deps): update aiohttp requirement from >=3.14.3 to >=3.14.4 in /autobot-backend/code_analysis in the all-dependencies group across 1 directory #18156 chore(deps): update aiohttp requirement from >=3.14.3 to >=3.14.4 in /autobot-backend/code_analysis in the all-dependencies group across 1 directory
• chore(deps-dev): bump the oxlint group across 1 directory with 2 updates #18152 chore(deps-dev): bump the oxlint group across 1 directory with 2 updates

If the queue is draining normally, ignore this. Otherwise:

1. Check whether CI is dispatching at all — see the ci-dispatch-watchdog status on these PRs
2. Merge the ones whose CI has finished and review has passed: gh pr merge <number> --squash --delete-branch
3. Look for a loop opening near-identical PRs

Warn-only runaway detector — .github/workflows/pr-queue-gate.yml. It never blocks a merge.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[mrveiss]

Security review (Opus) at 428598b: blocked. Nothing is exploitable through today's sources, but these must be settled first:

Severity	Location	Finding	Fix
High (merge)	migrations/versions/20261012_098_approval_record_columns.py:33	down_revision = "20260928_097", the same parent as #18343's 20261011_098_audit_logs_canonical_columns.py:46. Whichever lands second leaves two alembic heads, and alembic upgrade head fails. #18343 is in merge train 7b, so it lands first.	After train 7b merges, rebase this PR. Renumber 098 → 099 with down_revision set to #18343's revision, and the current 099 → 100.
Medium	services/approval_gate_service.py _transition (~L380-417)	The status check is a plain SELECT followed by a write, with no lock or conditional UPDATE. Two concurrent approves both pass, so resume_decided_approval runs twice. It is latent today (no register_resume_handler caller), but this PR adds the execution trigger.	Filed as #18413, which blocks S2. Fix it here with a conditional UPDATE … RETURNING, or keep it in #18413.
Medium	_transition	expires_at is not enforced at decision time.	Filed as #18412.
Low	api/approval_gates.py:205-229	/approval-gates/pending has no owner or session filter and returns context. That is no worse than main today, but it must be fixed before S2 sources.	Filed as #18413.

Verified fine:

• The create route rejects the new fields, and there is no mass-assignment path.
• RiskTier is a subset of Severity, with mappers and no fork.
• 099 is additive and idempotent.
• The 098 downgrade refuses while any row holds a value in the new columns, so no data is lost.
• api.ts matches the routes.

[mrveiss]

Review fixes pushed (head 8e0e123): atomic decision claim (one conditional UPDATE ... RETURNING; the loser matches no row and does not resume; concurrent approve/approve and approve/reject tests on Postgres), owner-scoped pending list (new nullable owner_user column in migration 098; admin sees all, a user sees their own plus legacy unsourced gates, a sourced row with no owner is admin-only; sourced creates must name an owner), and expires_at enforcement (past-expiry decision refused and the row moved to EXPIRED and resumed as expired; expired rows hidden from the pending list; expire_stale() once-only). The periodic scheduler for expire_stale() is not wired (#18412 stays open, comment there). Still to do: renumber 098/099 after #18343 lands. Refs #18413, #18412.

[mrveiss]

Security delta review (Opus) at 8e0e123: approve on everything except the migration renumber. The renumber stays deferred: 098's down_revision still collides with #18343's 098, and #18343 is in merge train 7b. Renumber once that is on main.

Verified:

• Atomic _claim: one conditional UPDATE … WHERE status IN (…) [AND unexpired] RETURNING (:409-436), and only the winner resumes (:456-484). SQLite doesn't apply: Postgres is required (user_management/database.py:142-147).
• Ownership on list_pending: :325-353.
• security(approvals): expires_at is stored but not enforced — a past-expiry approval can still be approved and resumed #18412 partial: past-expiry decisions are refused, set to EXPIRED and resumed as expired; expired rows are hidden; expire_stale exists. security(approvals): expires_at is stored but not enforced — a past-expiry approval can still be approved and resumed #18412 correctly stays open.
• Migration 098: owner_user is nullable and indexed, and the downgrade refuses while any row is populated.

Conditions and LOWs:

• Keep Refs #13421 and do not close security(approvals): pending list has no ownership filter and the approve transition is not atomic — must close before S2 sources #18413: GET /approval-gates (:179-203) and GET /approval-gates/{id} (:244-257) have no owner filter and bypass the new scoping. That is latent until S2; details on security(approvals): pending list has no ownership filter and the approve transition is not atomic — must close before S2 sources #18413.
• Pre-existing: any authenticated user can approve or reject any gate. Tracked on backend: approving/rejecting a destructive_action approval-gate request requires only require_interactive_human, not admin role #17161, with this sighting added. It must close before S2 registers a resume handler.
• LOWs:
  • use func.now() rather than the app clock in the unexpired predicate (:342,365,429,467);
  • owner_user accepts whitespace (approval_record_fields.py:72, use .strip());
  • the concurrency tests race on separate sessions but have no barrier, so the race is likely, not forced.