fix: full governance enforcement online (#1957)#2005
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🧰 Additional context used📓 Path-based instructions (10)web/src/**/*.{js,jsx,ts,tsx,mts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/src/mocks/handlers/**/*.ts📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/src/**/*.{ts,tsx,mts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⚙️ CodeRabbit configuration file
Files:
src/synthorg/observability/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
tests/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⚙️ CodeRabbit configuration file
Files:
src/synthorg/api/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/{workers,api}/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2026-05-05T09:04:46.195ZApplied to files:
🔇 Additional comments (9)
WalkthroughThis PR wires a configured AutonomyChangeStrategy into app startup and AppState; implements approval park/resume (non-destructive parked-context probe, background dispatch_resume, engine resume mixin that injects decision messages); adds progressive-trust enforcement and an MCP self-consumer bridge; extends autonomy APIs/models to require reasons and support strategy grants; persists an approval-item source discriminator (DB migrations and repo updates); and updates frontend types/mocks plus comprehensive unit and e2e tests and docs. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request completes the integration of the GHOST governance subsystems into the live agent runtime. By wiring the ApprovalGate, TrustService, MCP self-consumer, and AutonomyChangeStrategy at boot, the system now enforces security, autonomy, and trust policies directly on running agents. These changes ensure that agent actions are properly gated, permissions are progressively earned, and administrative tools are protected by fail-closed guardrails. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the Park-Resume governance architecture and the progressive-trust security system. Key additions include a shared ApprovalGate for managing parked execution contexts, an AgentEngineResumeMixin to continue runs after approval, and a trust-scoped MCP bridge for agents. The autonomy controller was updated to enforce seniority constraints and require justifications for level changes. Feedback identifies opportunities to optimize component instantiation in the autonomy controller, improve audit logging by capturing the requester's identity, and ensure strict type hinting by replacing "Any" with specific types in the resume logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/synthorg/engine/agent_engine_factories.py`:
- Around line 205-208: The check-then-initialize is subject to a TOCTOU race:
when computing agent_key = str(identity.id) the call to
self._trust_service.get_trust_state(agent_key) followed by
self._trust_service.initialize_agent(agent_key) must be atomic; wrap this
read-modify-write in the same lock used by the trust service (or convert
get_trust_state/initialize_agent to async and call them under the service's
async lock), e.g. acquire the trust service's synchronous lock before calling
get_trust_state and only call initialize_agent while holding that lock so
concurrent requests cannot both initialize the same agent; reference methods:
get_trust_state, initialize_agent and the async-lock-guarded methods
evaluate_agent, apply_trust_change, check_decay to replicate the same protection
here.
In `@src/synthorg/engine/approval_gate.py`:
- Around line 528-537: The current code treats delete() == False as benign and
continues, which can allow duplicate resumes; update the branch handling
delete() returning False (the block using APPROVAL_GATE_RESUME_DELETE_FAILED,
approval_id, parked.id, note) to abort the resume by raising an exception (or a
specific ResumeFailure/RuntimeError) instead of simply logging: log the failure
with the same context and then raise to fail closed so the deserialized context
is not returned to callers.
In `@src/synthorg/workers/execution_service.py`:
- Around line 503-510: The code currently logs APPROVAL_GATE_RESUME_FAILED and
returns when self._engine._approval_gate is None, which swallows the failure;
instead, after logging include the approval_id and reason, raise an exception
(e.g., RuntimeError or the repository's specific error type if one exists) to
surface the missing approval gate so the background registry records a real
failure; modify the block around gate = self._engine._approval_gate (the check
that uses APPROVAL_GATE_RESUME_FAILED and approval_id) to raise with a clear
message (including approval_id and "engine_has_no_approval_gate") rather than
returning.
In `@tests/unit/api/controllers/test_autonomy.py`:
- Around line 86-90: The test currently only checks promotion_pending on the
response stub; update the assertions to verify the approval was actually
persisted by asserting data["approval_enqueued"] is True and that
data["approval_id"] exists and is a non-empty string (e.g. assert
isinstance(data["approval_id"], str) and data["approval_id"]). Keep the existing
assertions for resp, data and promotion_pending, and use the same variables
(resp, data, agent_id, promotion_pending, approval_enqueued, approval_id) so the
test proves the controller hit the real approval pipeline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dfb13a6-4bb6-494b-afc1-1cbdb911e875
📒 Files selected for processing (37)
docs/design/security.mdscripts/_ghost_wiring_manifest.txtsrc/synthorg/api/app.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/api/state.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/trust.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/security/config.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/workers/execution_service.pysrc/synthorg/workers/runtime_builder.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/api/controllers/test_autonomy.pytests/unit/api/test_guards.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_agent_engine_resume.pytests/unit/engine/test_agent_engine_trust.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/security/trust/test_enforcement.pytests/unit/workers/test_execution_service.pytests/unit/workers/test_runtime_builder.pyweb/src/api/types/dtos.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.tsweb/src/mocks/handlers/agents.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Dashboard Test
- GitHub Check: Test E2E
- GitHub Check: Test Integration
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test Unit
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{py,ts,tsx,js,jsx,md,yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
No region/currency/locale privileged; use metric units; British English in all text and documentation
Files:
src/synthorg/observability/events/approval_gate.pydocs/design/security.mdweb/src/api/types/enum-values.gen.tstests/unit/engine/test_agent_engine_resume.pyweb/src/mocks/handlers/agents.tssrc/synthorg/observability/events/trust.pyweb/src/api/types/openapi.gen.tsweb/src/api/types/dtos.gen.tssrc/synthorg/workers/runtime_builder.pysrc/synthorg/api/app_builders.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/security/config.pytests/unit/api/test_guards.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/security/trust/enforcement.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_trust.pysrc/synthorg/engine/approval_gate.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/workers/test_execution_service.pysrc/synthorg/engine/agent_engine_factories.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/engine/test_approval_gate.pysrc/synthorg/api/state.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/api/controllers/_approval_review_gate.pytests/unit/api/test_startup_wiring.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/lifecycle_builder.py
src/synthorg/!(persistence)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Only
src/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Files:
src/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/trust.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/api/app_builders.pysrc/synthorg/security/config.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/api/state.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/lifecycle_builder.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Configuration Precedence: DB > env > code default viaSettingsService/ConfigResolver(Cat-1) or env > code default (Cat-2,read_only_post_init); Cat-3 bootstrap secrets are pure env; YAML is company-template ingestion format only; noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value
No hardcoded numeric values outside of 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal); numerics live insettings/definitions/
Use async/await withasyncio.TaskGroupfor fan-out/fan-in; helpers catchException(re-raiseMemoryError/RecursionError)
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing
Nofrom __future__ import annotations(Python 3.14 has PEP 649); PEP 758 except:except A, B:requires parens when binding
Type hints on all public functions; mypy strict; Google-style docstrings; line length 88; functions <50 lines; files <800 lines
Error classes:<Domain><Condition>Errorformat inheriting fromDomainError; never inheritException/RuntimeErrordirectly
Pydantic v2: frozen +extra="forbid"on every frozen model;@computed_fieldauto-exempt; per-line# lint-allow: frozen-extra-forbidfor exceptions; use@computed_fieldfor derived fields;NotBlankStrfor identifiers
Args models required at every system boundary; useparse_typed()for every external dict ingestion
Immutability: usemodel_copy(update=...)orcopy.deepcopy(); deepcopy at system boundaries
Clock seam:clock: Clock | None = None; injectFakeClockin tests; services own_lifecycle_lock; timed-out stops mark unrestartable
Untrusted content (SEC-1): usewrap_untrusted()fromengine.prompt_safety; useHTMLParseGuardfor HTML
Logging: usefrom synthorg.observability import get_logger; variable alwayslogger; neverimport logging/print()in app...
Files:
src/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/trust.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/api/app_builders.pysrc/synthorg/security/config.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/api/state.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/lifecycle_builder.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
ruff check --fixandruff formatfor linting and formatting;mypy --num-workers=4 src/ tests/for strict type-checking
Files:
src/synthorg/observability/events/approval_gate.pytests/unit/engine/test_agent_engine_resume.pysrc/synthorg/observability/events/trust.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/api/app_builders.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/security/config.pytests/unit/api/test_guards.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/security/trust/enforcement.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_trust.pysrc/synthorg/engine/approval_gate.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/workers/test_execution_service.pysrc/synthorg/engine/agent_engine_factories.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/engine/test_approval_gate.pysrc/synthorg/api/state.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/api/controllers/_approval_review_gate.pytests/unit/api/test_startup_wiring.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/lifecycle_builder.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/trust.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/api/app_builders.pysrc/synthorg/security/config.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/api/state.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/lifecycle_builder.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.md: Numerics in README and public docs must be sourced fromdata/runtime_stats.yamlvia<!--RS:NAME-->markers
Markdown tables for tabular data
Files:
docs/design/security.md
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/dtos.gen.ts
web/src/api/types/**/*.gen.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Generated DTO types (MANDATORY): NEVER hand-edit
web/src/api/types/*.gen.ts. Regenerate withuv run python scripts/generate_dto_types_ts.py. Import DTOs via the barrel (import type { AgentConfig } from '@/api/types').
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/dtos.gen.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/dtos.gen.ts
web/src/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design system components with design tokens only in web projects
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/dtos.gen.ts
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Web projects use React 19; see
web/CLAUDE.mdfor dashboard design system; CLI is Go binary (usego -C cli, nevercd cli); seecli/CLAUDE.md
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/dtos.gen.ts
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}; async tests useauto; global timeout 30s; minimum coverage 80%
Windows unit tests useWindowsSelectorEventLoopPolicy(Python 3.14 IOCP teardown race); subprocess tests override back
Test doubles: useFakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags; bareMagicMockat typed boundaries is forbidden
Hypothesis testing: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)); never skip/xfail flaky tests; fix fundamentally
Files:
tests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pytests/unit/api/test_guards.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_trust.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_approval_gate.pytests/unit/api/test_startup_wiring.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_mcp_self_consumer.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pytests/unit/api/test_guards.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_trust.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_approval_gate.pytests/unit/api/test_startup_wiring.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_mcp_self_consumer.py
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/mocks/handlers/**/*.ts: MSW handlers (MANDATORY):web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler for every exported endpoint. UseonUnhandledRequest: 'error'in test setup; tests override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*').
Use typed envelope helpers (successFor,paginatedFor,voidSuccess) to keep MSW handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/agents.ts
src/synthorg/workers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Worker execution service: use
synthorg.workers.runtime_builder.build_worker_execution_servicebehind provider-present switch; installs viaAppState.worker_execution_service; empty-company rejects task creation withAgentRuntimeNotConfiguredError(4014)
Files:
src/synthorg/workers/runtime_builder.pysrc/synthorg/workers/execution_service.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/api/**/*.py: API startup lifecycle: construction phase wires synchronous services; on_startup phase wires services needing persistence backend;agent_registrybeforeauto_wire_meetings;tunnel_providerwired unconditionally
On-startup ordering:SettingsServiceauto-wire precedesWorkflowExecutionObserverregistration;OntologyServicewires afterpersistence.connect()via_wire_ontology_service
Files:
src/synthorg/api/app_builders.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/state.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/lifecycle_builder.py
🧠 Learnings (5)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/observability/events/approval_gate.pytests/unit/engine/test_agent_engine_resume.pysrc/synthorg/observability/events/trust.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/api/app_builders.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/security/config.pytests/unit/api/test_guards.pysrc/synthorg/api/app.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/security/trust/enforcement.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_trust.pysrc/synthorg/engine/approval_gate.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/workers/test_execution_service.pysrc/synthorg/engine/agent_engine_factories.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/engine/test_approval_gate.pysrc/synthorg/api/state.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/api/controllers/_approval_review_gate.pytests/unit/api/test_startup_wiring.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/lifecycle_builder.py
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/design/security.md
🔇 Additional comments (32)
src/synthorg/observability/events/approval_gate.py (2)
19-20: LGTM!
27-30: LGTM!docs/design/security.md (1)
8-10: LGTM!Also applies to: 98-107
web/src/api/types/enum-values.gen.ts (1)
81-87: LGTM!tests/unit/engine/test_agent_engine_resume.py (3)
32-55: LGTM!
56-81: LGTM!
83-98: LGTM!web/src/mocks/handlers/agents.ts (1)
77-92: LGTM!src/synthorg/observability/events/trust.py (1)
18-19: LGTM!web/src/api/types/openapi.gen.ts (1)
5783-5783: LGTM!Also applies to: 5803-5804, 5838-5867
web/src/api/types/dtos.gen.ts (1)
142-142: LGTM!src/synthorg/workers/runtime_builder.py (1)
19-19: LGTM!Also applies to: 123-130
src/synthorg/api/app_builders.py (1)
45-46: LGTM!Also applies to: 209-234
tests/unit/workers/test_runtime_builder.py (1)
68-75: LGTM!Also applies to: 83-127, 148-155, 182-189
src/synthorg/security/config.py (1)
302-342: LGTM!Also applies to: 444-447
tests/unit/api/test_guards.py (1)
163-193: LGTM!src/synthorg/api/app.py (1)
24-24: LGTM!Also applies to: 532-534, 564-564
src/synthorg/api/controllers/autonomy.py (1)
3-33: LGTM!Also applies to: 45-68, 135-205, 208-221
src/synthorg/engine/agent_engine_resume.py (1)
59-257: LGTM!src/synthorg/security/trust/enforcement.py (1)
20-64: LGTM!tests/unit/engine/test_approval_gate_wiring.py (1)
61-99: LGTM!tests/unit/security/trust/test_enforcement.py (1)
14-76: LGTM!tests/unit/engine/test_agent_engine_trust.py (1)
28-102: LGTM!src/synthorg/engine/approval_gate.py (1)
352-372: LGTM!Also applies to: 512-526, 550-579
tests/unit/engine/test_loop_helpers_approval.py (1)
54-110: LGTM!tests/unit/workers/test_execution_service.py (1)
27-28: LGTM!Also applies to: 299-405
src/synthorg/api/controllers/_approval_review_gate.py (1)
47-115: LGTM!Also applies to: 294-302
tests/unit/api/test_startup_wiring.py (1)
17-20: LGTM!Also applies to: 202-264
tests/unit/api/controllers/test_approvals_helpers.py (1)
132-174: LGTM!Also applies to: 198-255, 341-344
tests/unit/engine/test_mcp_self_consumer.py (1)
1-195: LGTM!src/synthorg/api/lifecycle_builder.py (1)
198-248: LGTM!Also applies to: 684-701, 964-977
scripts/_ghost_wiring_manifest.txt (1)
27-30: LGTM!
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/synthorg/engine/agent_engine_resume.py`:
- Around line 101-106: Replace the misleading error event constant
APPROVAL_GATE_RESUME_STARTED with a new failure constant and use it in the
logger.error call: add APPROVAL_GATE_RESUME_FAILED =
"approval_gate.resume_failed" to observability/events/approval_gate.py, then
update the logger.error invocation in agent_engine_resume.py (the block that
currently logs APPROVAL_GATE_RESUME_STARTED and raises ExecutionStateError when
the parked context is invalid) to log APPROVAL_GATE_RESUME_FAILED and include
the same approval_id and note payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: fc683d21-11a2-4f64-9c6a-ee1c5262530a
📒 Files selected for processing (10)
data/runtime_stats.yamlsrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/trust/service.pysrc/synthorg/workers/execution_service.pytests/e2e/test_runtime_online_seam.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_approval_gate.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: Test Integration
- GitHub Check: Dashboard Test
- GitHub Check: Test Unit
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test E2E
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{py,ts,tsx,js,jsx,md,yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
No region/currency/locale privileged; use metric units; British English in all text and documentation
Files:
data/runtime_stats.yamltests/e2e/test_runtime_online_seam.pysrc/synthorg/security/trust/service.pysrc/synthorg/engine/agent_engine_factories.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/workers/execution_service.pytests/unit/engine/test_approval_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}; async tests useauto; global timeout 30s; minimum coverage 80%
Windows unit tests useWindowsSelectorEventLoopPolicy(Python 3.14 IOCP teardown race); subprocess tests override back
Test doubles: useFakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags; bareMagicMockat typed boundaries is forbidden
Hypothesis testing: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)); never skip/xfail flaky tests; fix fundamentally
Files:
tests/e2e/test_runtime_online_seam.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_approval_gate.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/e2e/test_runtime_online_seam.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_approval_gate.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
ruff check --fixandruff formatfor linting and formatting;mypy --num-workers=4 src/ tests/for strict type-checking
Files:
tests/e2e/test_runtime_online_seam.pysrc/synthorg/security/trust/service.pysrc/synthorg/engine/agent_engine_factories.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/workers/execution_service.pytests/unit/engine/test_approval_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.py
src/synthorg/!(persistence)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Only
src/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Files:
src/synthorg/security/trust/service.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Configuration Precedence: DB > env > code default viaSettingsService/ConfigResolver(Cat-1) or env > code default (Cat-2,read_only_post_init); Cat-3 bootstrap secrets are pure env; YAML is company-template ingestion format only; noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value
No hardcoded numeric values outside of 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal); numerics live insettings/definitions/
Use async/await withasyncio.TaskGroupfor fan-out/fan-in; helpers catchException(re-raiseMemoryError/RecursionError)
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing
Nofrom __future__ import annotations(Python 3.14 has PEP 649); PEP 758 except:except A, B:requires parens when binding
Type hints on all public functions; mypy strict; Google-style docstrings; line length 88; functions <50 lines; files <800 lines
Error classes:<Domain><Condition>Errorformat inheriting fromDomainError; never inheritException/RuntimeErrordirectly
Pydantic v2: frozen +extra="forbid"on every frozen model;@computed_fieldauto-exempt; per-line# lint-allow: frozen-extra-forbidfor exceptions; use@computed_fieldfor derived fields;NotBlankStrfor identifiers
Args models required at every system boundary; useparse_typed()for every external dict ingestion
Immutability: usemodel_copy(update=...)orcopy.deepcopy(); deepcopy at system boundaries
Clock seam:clock: Clock | None = None; injectFakeClockin tests; services own_lifecycle_lock; timed-out stops mark unrestartable
Untrusted content (SEC-1): usewrap_untrusted()fromengine.prompt_safety; useHTMLParseGuardfor HTML
Logging: usefrom synthorg.observability import get_logger; variable alwayslogger; neverimport logging/print()in app...
Files:
src/synthorg/security/trust/service.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/security/trust/service.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.py
src/synthorg/workers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Worker execution service: use
synthorg.workers.runtime_builder.build_worker_execution_servicebehind provider-present switch; installs viaAppState.worker_execution_service; empty-company rejects task creation withAgentRuntimeNotConfiguredError(4014)
Files:
src/synthorg/workers/execution_service.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/api/**/*.py: API startup lifecycle: construction phase wires synchronous services; on_startup phase wires services needing persistence backend;agent_registrybeforeauto_wire_meetings;tunnel_providerwired unconditionally
On-startup ordering:SettingsServiceauto-wire precedesWorkflowExecutionObserverregistration;OntologyServicewires afterpersistence.connect()via_wire_ontology_service
Files:
src/synthorg/api/controllers/autonomy.py
🧠 Learnings (1)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
tests/e2e/test_runtime_online_seam.pysrc/synthorg/security/trust/service.pysrc/synthorg/engine/agent_engine_factories.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/workers/execution_service.pytests/unit/engine/test_approval_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/engine/agent_engine_resume.py
🔇 Additional comments (36)
data/runtime_stats.yaml (1)
2-12: LGTM!tests/e2e/test_runtime_online_seam.py (1)
131-139: LGTM!src/synthorg/security/trust/service.py (2)
8-8: LGTM!Also applies to: 76-82
84-102: LGTM!src/synthorg/engine/agent_engine_factories.py (4)
19-25: LGTM!Also applies to: 46-49
78-87: LGTM!
191-228: LGTM!
293-304: LGTM!tests/unit/api/controllers/test_autonomy.py (5)
1-7: LGTM!Also applies to: 9-19
31-44: LGTM!
71-103: LGTM!
105-137: LGTM!
139-147: LGTM!src/synthorg/engine/approval_gate.py (3)
29-29: LGTM!Also applies to: 353-373
513-549: LGTM!
562-566: LGTM!Also applies to: 582-590
src/synthorg/workers/execution_service.py (6)
34-34: LGTM!Also applies to: 46-51, 73-79
111-131: LGTM!
232-259: LGTM!
275-299: LGTM!
449-561: LGTM!
601-629: LGTM!tests/unit/engine/test_approval_gate.py (3)
10-10: LGTM!Also applies to: 182-216
304-358: LGTM!
410-440: LGTM!src/synthorg/api/controllers/autonomy.py (6)
3-7: LGTM!
14-25: LGTM!
29-33: LGTM!Also applies to: 46-53, 55-69
161-185: LGTM!Also applies to: 187-218
220-226: LGTM!
204-207: ⚡ Quick winRemove this comment — the concern is unfounded.
resolve_decided_by()returnsactor.label or actor.actor_id, whereactor_idis typed asNotBlankStr(guaranteed non-empty). Since no explicit override is passed at line 206, the function always falls back to a non-empty value. Even iflabelis an empty string, Python'soroperator ensuresactor_idis returned, soNotBlankStr()validation will never fail.> Likely an incorrect or invalid review comment.src/synthorg/engine/agent_engine_resume.py (5)
1-34: LGTM!
37-61: LGTM!
63-144: LGTM!
146-182: LGTM!
184-260: LGTM!
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2005 +/- ##
==========================================
+ Coverage 85.00% 85.02% +0.01%
==========================================
Files 1884 1887 +3
Lines 111359 111644 +285
Branches 9497 9522 +25
==========================================
+ Hits 94666 94929 +263
- Misses 14373 14391 +18
- Partials 2320 2324 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Aureliolo
force-pushed
the
fix/governance-online
branch
from
209e172 to
ba5589b
Compare
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/workers/test_execution_service.py (1)
333-405: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd a regression test for the missing-approval-gate path.
This suite covers no-provider and lifecycle-baseline resume failures, but not the
AgentEngineExecutionService._resume_parked()branch that now raises whenengine._approval_gateisNone. Pinning that path would protect the new fail-loud behaviour from silently regressing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/workers/test_execution_service.py` around lines 333 - 405, Add a regression test that exercises AgentEngineExecutionService._resume_parked when the engine has no approval gate: construct an AgentEngineExecutionService (or use the existing service factory) with an engine whose engine._approval_gate is None, call service.dispatch_resume(...) and assert it raises AgentRuntimeNotConfiguredError (matching the expected message), mirroring the style of test_no_provider_dispatch_resume_rejects and test_lifecycle_baseline_dispatch_resume_rejects so the missing-approval-gate branch is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/synthorg/api/controllers/_approval_review_gate.py`:
- Around line 73-91: The routing currently probes app_state.approval_gate and
calls gate.has_parked_context(approval_id) to decide resume vs review, which is
non-deterministic; instead persist the approval source/type at creation and use
that persistent field for routing. Update the approval creation logic to store a
definitive attribute (e.g. approval.source or approval.origin) and replace the
runtime probe in the function referencing app_state.approval_gate and
has_parked_context with a deterministic check against that persisted field;
leave MemoryError/RecursionError propagation and keep the
APPROVAL_GATE_RESUME_FAILED logging for probe failures only as fallback, but
ensure the primary branch uses the stored approval.source to route to resume or
review.
In `@src/synthorg/api/controllers/autonomy.py`:
- Around line 187-209: The code calls strategy.request_promotion(...) and stores
strategy_granted but never passes that verdict into the apply path; modify the
call to app_state.agent_registry.update_autonomy (or the AutonomyUpdate payload)
to include the strategy decision so the registry/enqueue can act on it: either
add a field like strategy_granted/auto_approved to AutonomyUpdate (and thread
that through update_autonomy and the approval_store handling) or change
update_autonomy's API to accept the strategy_granted boolean so runtime changes
to app_state.autonomy_change_strategy actually affect approval/enqueue behavior
(refer to strategy.request_promotion, strategy_granted, AutonomyUpdate, and
app_state.agent_registry.update_autonomy).
In `@src/synthorg/api/lifecycle_builder.py`:
- Around line 684-700: The current except block swallows failures from
_wire_approval_gate leaving app_state.approval_gate unset which allows a private
gate to be built from _approval_store and breaks resume semantics in
provider-present runtimes; modify the exception handler so that after catching
Exception as exc you check whether the runtime is provider-present (e.g. inspect
app_state.provider_present or the equivalent flag on persistence/app_state) and
if provider-present re-raise the exception to abort startup, otherwise keep the
existing logger.warning call (still propagating MemoryError and RecursionError
as currently written); ensure you reference _wire_approval_gate,
app_state.approval_gate and _approval_store in the change and preserve
API_SERVICE_AUTO_WIRE_FAILED logging for the non-provider-present path.
- Around line 231-241: The boot-time ApprovalGate is being created without the
configured approval interrupt timeout; update the constructor call to pass the
same configured timeout used by the engine’s _make_approval_gate() (i.e., supply
interrupt_timeout_seconds=<the configured value from app_state or its config,
the same attribute used by _make_approval_gate()>), so ApprovalGate(...)
includes interrupt_timeout_seconds and thus honors non-default
approval-interrupt settings.
In `@src/synthorg/security/trust/service.py`:
- Around line 76-103: All access to the shared maps (_trust_states,
_change_history) must use a single lock discipline: ensure
get_or_initialize_agent, get_trust_state, get_change_history, evaluate_agent,
apply_trust_change and check_decay all synchronise on the same _state_lock.
Replace the current pattern where get_or_initialize_agent uses _init_lock alone
by making it acquire the async _state_lock as well (e.g. convert
get_or_initialize_agent to an async method or provide a synchronous wrapper that
obtains the asyncio.Lock via the event loop) so the check-and-create is
performed under _state_lock, and update get_trust_state, get_change_history and
the post-evaluation access in check_decay to acquire/release _state_lock around
reads to the maps; remove independent use of _init_lock for map protection so
there is a single lock protecting _trust_states and _change_history.
In `@tests/unit/api/controllers/test_approvals_helpers.py`:
- Around line 138-145: Replace bare MagicMock substitutes for the worker
execution seam with the typed test double helper so the mock preserves the
expected interface: instead of creating mock_worker = MagicMock() and assigning
it to app_state.worker_execution_service, use
mock_of[WorkerExecutionService](...) (or the actual worker service type used by
AppState) to construct the stub and pass it into
app_state.worker_execution_service; do the same for any other occurrences noted
(the mocks around lines 170-177 and 232-244) so all worker_execution_service
doubles are created via mock_of[T](**overrides) rather than raw MagicMock and
keep AsyncMock for async methods like dispatch_resume/complete_review as
overrides to mock_of.
- Around line 225-254: Add an assertion that the worker dispatch path actually
executed by verifying mock_worker.dispatch_resume was awaited (e.g.
mock_worker.dispatch_resume.assert_awaited() or
mock_worker.dispatch_resume.assert_awaited_once()/assert_awaited_once_with(...)
with the expected args) after calling _signal_resume_intent so the test fails if
dispatch_resume was not awaited; keep the existing
mock_review.complete_review.assert_not_awaited() check.
In `@tests/unit/engine/test_loop_helpers_approval.py`:
- Around line 88-95: Replace the untyped MagicMock at the ApprovalGate boundary
with the repository's typed helper: create the gate using
mock_of[ApprovalGate](...) (or mock_of(ApprovalGate) generic form used in the
project) instead of MagicMock(spec=ApprovalGate), then apply the same overrides
(set gate.should_park.return_value = escalation and gate.park_context =
AsyncMock(side_effect=_capture_park)). Ensure the variable name gate and the
AsyncMock _capture_park usage remain unchanged so the test keeps the same
behavior while enforcing the typed boundary.
---
Outside diff comments:
In `@tests/unit/workers/test_execution_service.py`:
- Around line 333-405: Add a regression test that exercises
AgentEngineExecutionService._resume_parked when the engine has no approval gate:
construct an AgentEngineExecutionService (or use the existing service factory)
with an engine whose engine._approval_gate is None, call
service.dispatch_resume(...) and assert it raises AgentRuntimeNotConfiguredError
(matching the expected message), mirroring the style of
test_no_provider_dispatch_resume_rejects and
test_lifecycle_baseline_dispatch_resume_rejects so the missing-approval-gate
branch is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11abc147-a59e-4853-bab0-52ee2608a222
📒 Files selected for processing (40)
data/runtime_stats.yamldocs/design/security.mdscripts/_ghost_wiring_manifest.txtsrc/synthorg/api/app.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/api/state.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/trust.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/security/config.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/security/trust/service.pysrc/synthorg/workers/execution_service.pysrc/synthorg/workers/runtime_builder.pytests/e2e/test_runtime_online_seam.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/api/controllers/test_autonomy.pytests/unit/api/test_guards.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_agent_engine_resume.pytests/unit/engine/test_agent_engine_trust.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/security/trust/test_enforcement.pytests/unit/workers/test_execution_service.pytests/unit/workers/test_runtime_builder.pyweb/src/api/types/dtos.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.tsweb/src/mocks/handlers/agents.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: Test Integration
- GitHub Check: Test E2E
- GitHub Check: Test Unit
- GitHub Check: Dashboard Test
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Lighthouse Site
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (15)
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/openapi.gen.ts
web/src/api/types/**/*.gen.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Generated DTO types (MANDATORY): NEVER hand-edit
web/src/api/types/*.gen.ts. Regenerate withuv run python scripts/generate_dto_types_ts.py. Import DTOs via the barrel (import type { AgentConfig } from '@/api/types').
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/openapi.gen.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/openapi.gen.ts
web/src/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design system components with design tokens only in web projects
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/openapi.gen.ts
**/*.{py,ts,tsx,js,jsx,md,yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
No region/currency/locale privileged; use metric units; British English in all text and documentation
Files:
web/src/api/types/enum-values.gen.tssrc/synthorg/security/autonomy/models.pytests/e2e/test_runtime_online_seam.pyweb/src/mocks/handlers/agents.tsdata/runtime_stats.yamlsrc/synthorg/observability/events/trust.pysrc/synthorg/api/app.pytests/unit/engine/test_approval_gate_wiring.pysrc/synthorg/security/trust/service.pydocs/design/security.mdtests/unit/api/test_startup_wiring.pyweb/src/api/types/dtos.gen.tstests/unit/engine/test_loop_helpers_approval.pytests/unit/security/trust/test_enforcement.pysrc/synthorg/api/app_builders.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pytests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/config.pytests/unit/engine/test_agent_engine_trust.pyweb/src/api/types/openapi.gen.tssrc/synthorg/api/state.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/api/controllers/test_approvals_helpers.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/agent_engine_resume.pytests/unit/api/test_guards.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/api/controllers/_approval_review_gate.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/workers/execution_service.pytests/unit/workers/test_execution_service.pysrc/synthorg/engine/agent_engine.pytests/unit/api/controllers/test_autonomy.py
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Web projects use React 19; see
web/CLAUDE.mdfor dashboard design system; CLI is Go binary (usego -C cli, nevercd cli); seecli/CLAUDE.md
Files:
web/src/api/types/enum-values.gen.tsweb/src/mocks/handlers/agents.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/openapi.gen.ts
src/synthorg/!(persistence)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Only
src/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Files:
src/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pysrc/synthorg/api/app.pysrc/synthorg/security/trust/service.pysrc/synthorg/api/app_builders.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/config.pysrc/synthorg/api/state.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Configuration Precedence: DB > env > code default viaSettingsService/ConfigResolver(Cat-1) or env > code default (Cat-2,read_only_post_init); Cat-3 bootstrap secrets are pure env; YAML is company-template ingestion format only; noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value
No hardcoded numeric values outside of 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal); numerics live insettings/definitions/
Use async/await withasyncio.TaskGroupfor fan-out/fan-in; helpers catchException(re-raiseMemoryError/RecursionError)
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing
Nofrom __future__ import annotations(Python 3.14 has PEP 649); PEP 758 except:except A, B:requires parens when binding
Type hints on all public functions; mypy strict; Google-style docstrings; line length 88; functions <50 lines; files <800 lines
Error classes:<Domain><Condition>Errorformat inheriting fromDomainError; never inheritException/RuntimeErrordirectly
Pydantic v2: frozen +extra="forbid"on every frozen model;@computed_fieldauto-exempt; per-line# lint-allow: frozen-extra-forbidfor exceptions; use@computed_fieldfor derived fields;NotBlankStrfor identifiers
Args models required at every system boundary; useparse_typed()for every external dict ingestion
Immutability: usemodel_copy(update=...)orcopy.deepcopy(); deepcopy at system boundaries
Clock seam:clock: Clock | None = None; injectFakeClockin tests; services own_lifecycle_lock; timed-out stops mark unrestartable
Untrusted content (SEC-1): usewrap_untrusted()fromengine.prompt_safety; useHTMLParseGuardfor HTML
Logging: usefrom synthorg.observability import get_logger; variable alwayslogger; neverimport logging/print()in app...
Files:
src/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pysrc/synthorg/api/app.pysrc/synthorg/security/trust/service.pysrc/synthorg/api/app_builders.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/config.pysrc/synthorg/api/state.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
ruff check --fixandruff formatfor linting and formatting;mypy --num-workers=4 src/ tests/for strict type-checking
Files:
src/synthorg/security/autonomy/models.pytests/e2e/test_runtime_online_seam.pysrc/synthorg/observability/events/trust.pysrc/synthorg/api/app.pytests/unit/engine/test_approval_gate_wiring.pysrc/synthorg/security/trust/service.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/security/trust/test_enforcement.pysrc/synthorg/api/app_builders.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pytests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/config.pytests/unit/engine/test_agent_engine_trust.pysrc/synthorg/api/state.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/api/controllers/test_approvals_helpers.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/agent_engine_resume.pytests/unit/api/test_guards.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/api/controllers/_approval_review_gate.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/workers/execution_service.pytests/unit/workers/test_execution_service.pysrc/synthorg/engine/agent_engine.pytests/unit/api/controllers/test_autonomy.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pysrc/synthorg/api/app.pysrc/synthorg/security/trust/service.pysrc/synthorg/api/app_builders.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/config.pysrc/synthorg/api/state.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/agent_engine.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}; async tests useauto; global timeout 30s; minimum coverage 80%
Windows unit tests useWindowsSelectorEventLoopPolicy(Python 3.14 IOCP teardown race); subprocess tests override back
Test doubles: useFakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags; bareMagicMockat typed boundaries is forbidden
Hypothesis testing: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)); never skip/xfail flaky tests; fix fundamentally
Files:
tests/e2e/test_runtime_online_seam.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pytests/unit/engine/test_agent_engine_trust.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/api/test_guards.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_autonomy.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/e2e/test_runtime_online_seam.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/security/trust/test_enforcement.pytests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pytests/unit/engine/test_agent_engine_trust.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/api/test_guards.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_autonomy.py
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/mocks/handlers/**/*.ts: MSW handlers (MANDATORY):web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler for every exported endpoint. UseonUnhandledRequest: 'error'in test setup; tests override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*').
Use typed envelope helpers (successFor,paginatedFor,voidSuccess) to keep MSW handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/agents.ts
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/api/**/*.py: API startup lifecycle: construction phase wires synchronous services; on_startup phase wires services needing persistence backend;agent_registrybeforeauto_wire_meetings;tunnel_providerwired unconditionally
On-startup ordering:SettingsServiceauto-wire precedesWorkflowExecutionObserverregistration;OntologyServicewires afterpersistence.connect()via_wire_ontology_service
Files:
src/synthorg/api/app.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/state.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.md: Numerics in README and public docs must be sourced fromdata/runtime_stats.yamlvia<!--RS:NAME-->markers
Markdown tables for tabular data
Files:
docs/design/security.md
src/synthorg/workers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Worker execution service: use
synthorg.workers.runtime_builder.build_worker_execution_servicebehind provider-present switch; installs viaAppState.worker_execution_service; empty-company rejects task creation withAgentRuntimeNotConfiguredError(4014)
Files:
src/synthorg/workers/runtime_builder.pysrc/synthorg/workers/execution_service.py
🧠 Learnings (5)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/security/autonomy/models.pytests/e2e/test_runtime_online_seam.pysrc/synthorg/observability/events/trust.pysrc/synthorg/api/app.pytests/unit/engine/test_approval_gate_wiring.pysrc/synthorg/security/trust/service.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/security/trust/test_enforcement.pysrc/synthorg/api/app_builders.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pytests/unit/engine/test_agent_engine_resume.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/security/config.pytests/unit/engine/test_agent_engine_trust.pysrc/synthorg/api/state.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/api/controllers/test_approvals_helpers.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/agent_engine_resume.pytests/unit/api/test_guards.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/api/controllers/_approval_review_gate.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/workers/execution_service.pytests/unit/workers/test_execution_service.pysrc/synthorg/engine/agent_engine.pytests/unit/api/controllers/test_autonomy.py
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/design/security.md
🔇 Additional comments (29)
web/src/api/types/enum-values.gen.ts (1)
81-87: LGTM!src/synthorg/security/autonomy/models.py (1)
10-12: LGTM!Also applies to: 165-172
tests/e2e/test_runtime_online_seam.py (1)
131-139: LGTM!web/src/mocks/handlers/agents.ts (1)
77-92: LGTM!scripts/_ghost_wiring_manifest.txt (1)
27-30: LGTM!data/runtime_stats.yaml (1)
2-3: LGTM!Also applies to: 6-6, 10-10
src/synthorg/observability/events/trust.py (1)
18-19: LGTM!src/synthorg/api/app.py (1)
24-24: LGTM!Also applies to: 532-534, 564-564
tests/unit/engine/test_approval_gate_wiring.py (1)
61-99: LGTM!docs/design/security.md (1)
8-10: LGTM!Also applies to: 98-107
tests/unit/api/test_startup_wiring.py (1)
17-20: LGTM!Also applies to: 202-264
web/src/api/types/dtos.gen.ts (1)
144-144: LGTM!tests/unit/security/trust/test_enforcement.py (1)
1-76: LGTM!src/synthorg/api/app_builders.py (1)
45-46: LGTM!Also applies to: 209-235
src/synthorg/observability/events/approval_gate.py (1)
19-20: LGTM!src/synthorg/security/trust/enforcement.py (1)
1-64: LGTM!tests/unit/engine/test_agent_engine_resume.py (1)
1-99: LGTM!tests/unit/workers/test_runtime_builder.py (1)
68-69: LGTM!Also applies to: 75-75, 83-127, 148-149, 155-155, 182-183, 189-189
src/synthorg/engine/approval_gate.py (1)
29-29: LGTM!Also applies to: 353-374, 513-527, 530-549, 562-566, 582-590
src/synthorg/security/config.py (1)
302-341: LGTM!Also applies to: 444-447
tests/unit/engine/test_agent_engine_trust.py (1)
1-102: LGTM!web/src/api/types/openapi.gen.ts (1)
5954-5954: File is properly generated per the coding guideline; no issues found.The file header contains proper auto-generation markers, and all code (including the new
AutonomyConfig.change_strategyfield,AutonomyLevelRequest.reasonfield, and newAutonomyStrategyConfigandAutonomyStrategyTypetypes) is consistently formatted and matches OpenAPI TypeScript generation patterns. No signs of hand-editing detected.src/synthorg/api/state.py (1)
119-121: LGTM!Also applies to: 194-195, 351-352, 391-392, 1056-1066, 1214-1225
src/synthorg/engine/mcp_self_consumer.py (1)
1-139: LGTM!src/synthorg/workers/runtime_builder.py (1)
19-19: LGTM!Also applies to: 123-130
src/synthorg/engine/agent_engine_resume.py (1)
1-261: LGTM!tests/unit/api/test_guards.py (1)
163-193: LGTM!src/synthorg/api/lifecycle_builder.py (1)
964-977: LGTM!src/synthorg/engine/agent_engine_factories.py (1)
19-25: LGTM!Also applies to: 30-30, 46-49, 79-87, 191-228, 293-305
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/synthorg/security/autonomy/models.py`:
- Around line 168-170: Update the newly added description string that begins
"Runtime autonomy-change strategy selection (promotion / downgrade /
recovery)..." to use British English by changing the word "organization" to
"organisation" wherever it appears in that concatenated string literal; locate
the string in src/synthorg/security/autonomy/models.py (the description for the
autonomy-change strategy field) and edit the literal fragments so the combined
resulting description reads with "organisation" instead of "organization".
In `@tests/unit/api/controllers/test_approvals_helpers.py`:
- Around line 251-260: The test needs to assert that the fallback probe path was
actually exercised by verifying that has_parked_context was awaited when
_signal_resume_intent falls back; update the test to await or assert that
mock_review.has_parked_context was awaited (e.g., assert_awaited/await_called)
after calling _signal_resume_intent, and retain the existing
mock_review.complete_review.assert_not_awaited() to ensure the primary path was
not taken. Target the _signal_resume_intent call and the
mock_review.has_parked_context method in your assertion so the fallback path is
explicitly covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f5dbc7e-46c7-4eb5-963d-8ae7432602fa
📒 Files selected for processing (25)
docs/design/security.mdsrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/core/approval.pysrc/synthorg/core/enums.pysrc/synthorg/hr/registry.pysrc/synthorg/observability/events/security.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/persistence/postgres/revisions/20260518000001_approval_source.sqlsrc/synthorg/persistence/postgres/schema.sqlsrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/persistence/sqlite/revisions/20260518000001_approval_source.sqlsrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/security/autonomy/models.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/hr/test_registry_autonomy.pytests/unit/meta/test_approval_repo.pytests/unit/workers/test_execution_service.pyweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: Lighthouse Site
- GitHub Check: Test Integration
- GitHub Check: Dashboard Test
- GitHub Check: Test E2E
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test Unit
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{py,ts,tsx,js,jsx,md,yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
No region/currency/locale privileged; use metric units; British English in all text and documentation
Files:
tests/unit/meta/test_approval_repo.pysrc/synthorg/core/enums.pysrc/synthorg/observability/events/security.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/core/approval.pydocs/design/security.mdsrc/synthorg/hr/registry.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pyweb/src/api/types/enum-values.gen.tstests/unit/engine/test_loop_helpers_approval.pytests/unit/hr/test_registry_autonomy.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pytests/unit/workers/test_execution_service.pyweb/src/api/types/openapi.gen.tstests/unit/api/controllers/test_approvals_helpers.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}; async tests useauto; global timeout 30s; minimum coverage 80%
Windows unit tests useWindowsSelectorEventLoopPolicy(Python 3.14 IOCP teardown race); subprocess tests override back
Test doubles: useFakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags; bareMagicMockat typed boundaries is forbidden
Hypothesis testing: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)); never skip/xfail flaky tests; fix fundamentally
Files:
tests/unit/meta/test_approval_repo.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/hr/test_registry_autonomy.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_approvals_helpers.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/meta/test_approval_repo.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/hr/test_registry_autonomy.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_approvals_helpers.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
ruff check --fixandruff formatfor linting and formatting;mypy --num-workers=4 src/ tests/for strict type-checking
Files:
tests/unit/meta/test_approval_repo.pysrc/synthorg/core/enums.pysrc/synthorg/observability/events/security.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/core/approval.pysrc/synthorg/hr/registry.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/hr/test_registry_autonomy.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_approvals_helpers.py
src/synthorg/!(persistence)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Only
src/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/security.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/core/approval.pysrc/synthorg/hr/registry.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Configuration Precedence: DB > env > code default viaSettingsService/ConfigResolver(Cat-1) or env > code default (Cat-2,read_only_post_init); Cat-3 bootstrap secrets are pure env; YAML is company-template ingestion format only; noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value
No hardcoded numeric values outside of 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal); numerics live insettings/definitions/
Use async/await withasyncio.TaskGroupfor fan-out/fan-in; helpers catchException(re-raiseMemoryError/RecursionError)
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing
Nofrom __future__ import annotations(Python 3.14 has PEP 649); PEP 758 except:except A, B:requires parens when binding
Type hints on all public functions; mypy strict; Google-style docstrings; line length 88; functions <50 lines; files <800 lines
Error classes:<Domain><Condition>Errorformat inheriting fromDomainError; never inheritException/RuntimeErrordirectly
Pydantic v2: frozen +extra="forbid"on every frozen model;@computed_fieldauto-exempt; per-line# lint-allow: frozen-extra-forbidfor exceptions; use@computed_fieldfor derived fields;NotBlankStrfor identifiers
Args models required at every system boundary; useparse_typed()for every external dict ingestion
Immutability: usemodel_copy(update=...)orcopy.deepcopy(); deepcopy at system boundaries
Clock seam:clock: Clock | None = None; injectFakeClockin tests; services own_lifecycle_lock; timed-out stops mark unrestartable
Untrusted content (SEC-1): usewrap_untrusted()fromengine.prompt_safety; useHTMLParseGuardfor HTML
Logging: usefrom synthorg.observability import get_logger; variable alwayslogger; neverimport logging/print()in app...
Files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/security.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/core/approval.pysrc/synthorg/hr/registry.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/security.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/core/approval.pysrc/synthorg/hr/registry.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.py
src/synthorg/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/persistence/**/*.py: New repository protocols must inherit from generic categories inpersistence/_generics.py(SingletonRepository,IdKeyedRepository,FilteredQueryRepository,AppendOnlyRepository,StatefulRepository,MVCCRepository); keep protocols@runtime_checkable
Bespoke repository methods only permitted under ADR-0001 D7 (real performance optimization or domain invariant callers must not bypass)
Repository CRUD methods:save(entity),get(id),delete(id) -> bool,list_items(...),query(...)returning tuples
Datetime in persistence: useparse_iso_utc/format_iso_utcfrompersistence._shared; reject naive datetimes; usenormalize_utcfor already-typed
Files:
src/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/persistence/sqlite/approval_repo.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.md: Numerics in README and public docs must be sourced fromdata/runtime_stats.yamlvia<!--RS:NAME-->markers
Markdown tables for tabular data
Files:
docs/design/security.md
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/src/api/types/**/*.gen.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Generated DTO types (MANDATORY): NEVER hand-edit
web/src/api/types/*.gen.ts. Regenerate withuv run python scripts/generate_dto_types_ts.py. Import DTOs via the barrel (import type { AgentConfig } from '@/api/types').
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/src/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design system components with design tokens only in web projects
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Web projects use React 19; see
web/CLAUDE.mdfor dashboard design system; CLI is Go binary (usego -C cli, nevercd cli); seecli/CLAUDE.md
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/api/**/*.py: API startup lifecycle: construction phase wires synchronous services; on_startup phase wires services needing persistence backend;agent_registrybeforeauto_wire_meetings;tunnel_providerwired unconditionally
On-startup ordering:SettingsServiceauto-wire precedesWorkflowExecutionObserverregistration;OntologyServicewires afterpersistence.connect()via_wire_ontology_service
Files:
src/synthorg/api/lifecycle_builder.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.py
🧠 Learnings (5)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
tests/unit/meta/test_approval_repo.pysrc/synthorg/core/enums.pysrc/synthorg/observability/events/security.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/core/approval.pysrc/synthorg/hr/registry.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/service.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/hr/test_registry_autonomy.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pytests/unit/workers/test_execution_service.pytests/unit/api/controllers/test_approvals_helpers.py
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/design/security.md
🔇 Additional comments (32)
src/synthorg/persistence/postgres/revisions/20260518000001_approval_source.sql (1)
10-11: LGTM!src/synthorg/persistence/sqlite/revisions/20260518000001_approval_source.sql (1)
10-11: LGTM!src/synthorg/persistence/sqlite/schema.sql (1)
1074-1076: LGTM!tests/unit/meta/test_approval_repo.py (1)
25-25: LGTM!src/synthorg/core/enums.py (1)
582-601: LGTM!src/synthorg/observability/events/security.py (1)
196-196: LGTM!src/synthorg/persistence/postgres/schema.sql (1)
1152-1154: LGTM!src/synthorg/persistence/postgres/approval_repo.py (1)
22-23: LGTM!Also applies to: 47-48, 53-60, 123-124, 188-189, 260-261
src/synthorg/core/approval.py (1)
20-21: LGTM!Also applies to: 46-51, 63-63
docs/design/security.md (1)
8-11: LGTM!Also applies to: 98-113
src/synthorg/hr/registry.py (1)
39-40: LGTM!Also applies to: 645-749
src/synthorg/security/service.py (1)
589-602: LGTM!src/synthorg/security/trust/service.py (1)
76-104: LGTM!web/src/api/types/enum-values.gen.ts (1)
40-45: LGTM!Also applies to: 87-94
tests/unit/engine/test_loop_helpers_approval.py (1)
18-19: LGTM!Also applies to: 55-112
tests/unit/hr/test_registry_autonomy.py (1)
148-187: LGTM!src/synthorg/tools/approval_tool.py (1)
160-176: LGTM!src/synthorg/api/lifecycle_builder.py (3)
198-263: LGTM!
700-726: LGTM!
990-1003: LGTM!src/synthorg/persistence/sqlite/approval_repo.py (1)
15-15: LGTM!Also applies to: 38-60, 121-121, 219-219, 285-285, 397-403, 434-440, 486-494, 555-563
src/synthorg/api/controllers/_approval_review_gate.py (2)
47-135: LGTM!
314-322: LGTM!src/synthorg/api/controllers/autonomy.py (2)
29-69: LGTM!
187-231: LGTM!tests/unit/workers/test_execution_service.py (1)
302-436: LGTM!web/src/api/types/openapi.gen.ts (5)
5764-5764: LGTM!
5776-5795: LGTM!
5975-5975: LGTM!
5995-5996: LGTM!
6030-6059: LGTM!tests/unit/api/controllers/test_approvals_helpers.py (1)
3-3: LGTM!Also applies to: 8-8, 28-28, 35-37, 41-45, 55-55, 61-69, 144-144, 152-196, 198-230, 261-267, 268-282, 292-296, 306-306, 334-334, 365-365, 386-386, 388-388, 395-395, 422-422
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/api/controllers/test_approvals_helpers.py (1)
195-222:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLock down deterministic source routing in the review-gate-source path.
This test still passes if
_signal_resume_intent()incorrectly probeshas_parked_context()first and then falls through. Add an explicit bypass assertion to protect the source-driven routing contract.Suggested assertion tightening
await _signal_resume_intent( app_state, "approval-1", approved=True, decided_by="admin", task_id="task-1", ) + mock_gate.has_parked_context.assert_not_awaited() mock_worker.dispatch_resume.assert_not_awaited() mock_review.complete_review.assert_awaited_once_with( task_id="task-1", requested_by="admin", approved=True,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/api/controllers/test_approvals_helpers.py` around lines 195 - 222, The test must ensure source-driven routing for review-gate approvals doesn't probe parked-context; in test_flow1_review_gate_source_falls_through add an explicit assertion that mock_gate.has_parked_context was not awaited (e.g., call mock_gate.has_parked_context.assert_not_awaited()) so _signal_resume_intent's REVIEW_GATE path cannot pass by first calling has_parked_context; keep the existing assertions for mock_worker.dispatch_resume and mock_review.complete_review as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/unit/api/controllers/test_approvals_helpers.py`:
- Around line 195-222: The test must ensure source-driven routing for
review-gate approvals doesn't probe parked-context; in
test_flow1_review_gate_source_falls_through add an explicit assertion that
mock_gate.has_parked_context was not awaited (e.g., call
mock_gate.has_parked_context.assert_not_awaited()) so _signal_resume_intent's
REVIEW_GATE path cannot pass by first calling has_parked_context; keep the
existing assertions for mock_worker.dispatch_resume and
mock_review.complete_review as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8f6bba2a-6e68-4a08-abf3-585c117a94aa
📒 Files selected for processing (9)
src/synthorg/security/autonomy/models.pytests/unit/api/controllers/test_approvals_helpers.pyweb/src/__tests__/helpers/factories.tsweb/src/api/types/enums.tsweb/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/stores/approvals.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Build Backend
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: Test Unit
- GitHub Check: Dashboard Test
- GitHub Check: Test Integration
- GitHub Check: Test E2E
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Lighthouse Site
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (15)
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enums.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/stores/approvals.tsweb/src/__tests__/helpers/factories.ts
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/mocks/handlers/**/*.ts: MSW handlers (MANDATORY):web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler for every exported endpoint. UseonUnhandledRequest: 'error'in test setup; tests override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*').
Use typed envelope helpers (successFor,paginatedFor,voidSuccess) to keep MSW handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/approvals.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enums.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/stores/approvals.tsweb/src/__tests__/helpers/factories.ts
web/src/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design system components with design tokens only in web projects
Files:
web/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enums.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/stores/approvals.tsweb/src/__tests__/helpers/factories.ts
**/*.{py,ts,tsx,js,jsx,md,yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
No region/currency/locale privileged; use metric units; British English in all text and documentation
Files:
web/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enums.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/stores/approvals.tsweb/src/__tests__/helpers/factories.tssrc/synthorg/security/autonomy/models.pytests/unit/api/controllers/test_approvals_helpers.py
web/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Web projects use React 19; see
web/CLAUDE.mdfor dashboard design system; CLI is Go binary (usego -C cli, nevercd cli); seecli/CLAUDE.md
Files:
web/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enums.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/stores/approvals.tsweb/src/__tests__/helpers/factories.ts
web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{jsx,tsx}: Use@eslint-react/no-leaked-conditional-renderingto catch the{count && <Foo />}bug where0renders verbatim. ForReactNode | undefinedprops use{value != null && value !== false && <jsx>}; for compound truthiness useBoolean(...).
Use@eslint-react/globalsto restrictwindow/document/localStorage/ etc. inside render. Hoist offenders into auseCallbackevent handler, auseEffect, or auseSyncExternalStore-backed hook.
Files:
web/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/pages/approvals/ApprovalCard.stories.tsx
web/src/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Storybook 10 is ESM-only; essentials are built into core, but
@storybook/addon-docsis now separate; imports moved tostorybook/testandstorybook/actions
Files:
web/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/pages/approvals/ApprovalCard.stories.tsx
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/stores/**/*.ts: List reads (fetch*) must seterror: string | nullon the store instead of toasting
Test teardown (MANDATORY): any new store that schedules timers or attaches event listeners must expose an equivalent cleanup hook and register it in the globalafterEach. The globalafterEachinweb/src/test-setup.tsxalready callsuseToastStore.getState().dismissAll(),cancelPendingPersist(), anduseThemeStore.getState().teardown().
Files:
web/src/stores/approvals.ts
web/src/{api/endpoints,stores}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Cursor pagination (MANDATORY): list endpoints must use opaque cursor-based paging via
PaginationMeta. Stores must keepnextCursor+hasMorein state (not offset arithmetic) and early-return when!hasMore || !nextCursor. Display counts must come fromdata.length.
Files:
web/src/stores/approvals.ts
src/synthorg/!(persistence)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Only
src/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Files:
src/synthorg/security/autonomy/models.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Configuration Precedence: DB > env > code default viaSettingsService/ConfigResolver(Cat-1) or env > code default (Cat-2,read_only_post_init); Cat-3 bootstrap secrets are pure env; YAML is company-template ingestion format only; noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value
No hardcoded numeric values outside of 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal); numerics live insettings/definitions/
Use async/await withasyncio.TaskGroupfor fan-out/fan-in; helpers catchException(re-raiseMemoryError/RecursionError)
Comments explain WHY only; no reviewer citations, issue back-refs, or migration framing
Nofrom __future__ import annotations(Python 3.14 has PEP 649); PEP 758 except:except A, B:requires parens when binding
Type hints on all public functions; mypy strict; Google-style docstrings; line length 88; functions <50 lines; files <800 lines
Error classes:<Domain><Condition>Errorformat inheriting fromDomainError; never inheritException/RuntimeErrordirectly
Pydantic v2: frozen +extra="forbid"on every frozen model;@computed_fieldauto-exempt; per-line# lint-allow: frozen-extra-forbidfor exceptions; use@computed_fieldfor derived fields;NotBlankStrfor identifiers
Args models required at every system boundary; useparse_typed()for every external dict ingestion
Immutability: usemodel_copy(update=...)orcopy.deepcopy(); deepcopy at system boundaries
Clock seam:clock: Clock | None = None; injectFakeClockin tests; services own_lifecycle_lock; timed-out stops mark unrestartable
Untrusted content (SEC-1): usewrap_untrusted()fromengine.prompt_safety; useHTMLParseGuardfor HTML
Logging: usefrom synthorg.observability import get_logger; variable alwayslogger; neverimport logging/print()in app...
Files:
src/synthorg/security/autonomy/models.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
ruff check --fixandruff formatfor linting and formatting;mypy --num-workers=4 src/ tests/for strict type-checking
Files:
src/synthorg/security/autonomy/models.pytests/unit/api/controllers/test_approvals_helpers.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/security/autonomy/models.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}; async tests useauto; global timeout 30s; minimum coverage 80%
Windows unit tests useWindowsSelectorEventLoopPolicy(Python 3.14 IOCP teardown race); subprocess tests override back
Test doubles: useFakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags; bareMagicMockat typed boundaries is forbidden
Hypothesis testing: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)); never skip/xfail flaky tests; fix fundamentally
Files:
tests/unit/api/controllers/test_approvals_helpers.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/api/controllers/test_approvals_helpers.py
🧠 Learnings (1)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/security/autonomy/models.pytests/unit/api/controllers/test_approvals_helpers.py
🔇 Additional comments (8)
web/src/mocks/handlers/approvals.ts (1)
23-23: LGTM!web/src/pages/approvals/ApprovalDetailDrawer.stories.tsx (1)
13-13: LGTM!web/src/pages/approvals/ApprovalTimeline.stories.tsx (1)
12-12: LGTM!web/src/api/types/enums.ts (1)
21-21: LGTM!web/src/pages/approvals/ApprovalCard.stories.tsx (1)
13-13: LGTM!web/src/stores/approvals.ts (1)
18-18: LGTM!Also applies to: 273-276
web/src/__tests__/helpers/factories.ts (1)
237-237: LGTM!src/synthorg/security/autonomy/models.py (1)
10-12: LGTM!Also applies to: 157-172, 263-272
Rebase resolved data/runtime_stats.yaml to main's copy; regenerate so generator_revision + values track current HEAD and the freshness gate stays green.
lifecycle_builder: thread configured approval interrupt timeout into the boot ApprovalGate; abort startup (not warn) when the shared gate fails to wire in provider-present mode (split park/resume gates would silently break resume). trust/service: drop the redundant _init_lock -- the sync get-or-init seam runs in the event-loop thread with no await, so a second lock both fights the single-_state_lock discipline and is unnecessary; documented. tests: mock_of[ApprovalGate]/[WorkerExecutionService] at typed boundaries; add missing-approval-gate resume regression test.
…derabbit) CodeRabbit #2: the AutonomyChangeStrategy verdict was computed but never threaded into the apply path, so non-HUMAN_ONLY strategies were inert. AutonomyUpdate gains granted_by_strategy; a granting strategy now produces an auto-decided (APPROVED, decided_by=strategy:<name>) approval item and the registry applies the level change immediately. HUMAN_ONLY (default) unchanged -- still pends. docs/design/security.md amended (user-approved deviation from queue-is-sole-apply-driver). Adds registry granted-path test.
…abbit) CodeRabbit #1: resume-vs-review routing probed a live parked-context backend (non-deterministic on outage). ApprovalItem gains a persisted ApprovalSource discriminator (default REVIEW_GATE; SecOps escalation + request_human_approval set PARKED_CONTEXT). Routing keys off the persisted source; the has_parked_context probe stays only as a logged fallback when the just-decided row cannot be re-read. Schema column + sqlite/postgres revisions (converging the two existing heads) + both repos + regenerated web DTO types. test_approvals_helpers reworked for deterministic source routing.
mock_of's static return is Any by design; the _store helper must not declare -> ApprovalStore (no-any-return). Return Any to match the mock_of contract; callers assign into the typed app_state.approval_store slot.
Pre-push affected-pytest caught two gaps in the #1 change: (1) sqlite save_many param_rows tuple missing item.source.value (16 placeholders vs 15 bindings -- the save() path was fixed but save_many's deeper-indented tuple was not); (2) the test-local _CREATE_TABLE approvals fixture in tests/unit/meta/test_approval_repo.py lacked the new source column. Both fixed; all 245 approval-related tests pass.
…ire_approval_gate Finding #3's unconditional config_resolver.get_engine_bridge_config() 503'd gate wiring on minimal states with no resolver (test_startup_wiring). Mirror the established has_config_resolver-guarded fallback (cf. WorkflowExecutionObserver wiring): use the EngineBridgeConfig seed default for approval_interrupt_timeout_seconds when the resolver is not wired, so gate wiring never fails on that account.
CI (Dashboard Type Check + Lighthouse/melange/CI-Pass cascade): round-3 added a required ApprovalItem.source to the generated Approval DTO; six hand-written web files build Approval literals without it. Add source to web factories/mocks/stores/3 stories; stores/approvals.ts sanitizes it via sanitizeWsEnum(APPROVAL_SOURCE_VALUES, 'review_gate') mirroring risk_level/status; re-export APPROVAL_SOURCE_VALUES from enums barrel. CodeRabbit: British English 'organization'->'organisation' in autonomy/models.py field description; assert mock_gate.has_parked_context awaited in the fallback-probe test so that path is explicitly locked down.
CodeRabbit (outside-diff): test_flow1_review_gate_source_falls_through passed even if _signal_resume_intent probed has_parked_context before falling through. Assert mock_gate.has_parked_context.assert_not_awaited() so the deterministic REVIEW_GATE-source bypass is explicitly locked.
Aureliolo
force-pushed
the
fix/governance-online
branch
from
0c5fb2b to
310cbb5
Compare
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/synthorg/security/trust/service.py (1)
329-335:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftUnlocked read-modify-write in
check_decayraces withget_or_initialize_agent.The new
get_or_initialize_agentclaims event-loop atomicity protects its check-then-create pattern, butcheck_decayperforms an unlocked read (line 329) followed by a write (line 335) to_trust_states. Ifget_or_initialize_agentruns between those two lines—possible becausecheck_decaysuspends at theawaiton line 325—the decay timestamp update will overwrite the freshly initialized state, losing the creation metadata.🔒 Protect the check_decay RMW under _state_lock
result = await self.evaluate_agent(agent_id, snapshot) # Update decay check timestamp *after* evaluation key = str(agent_id) - state = self._trust_states.get(key) - if state is not None: - now = datetime.now(UTC) - updated = state.model_copy( - update={"last_decay_check_at": now}, - ) - self._trust_states[key] = updated + async with self._state_lock: + state = self._trust_states.get(key) + if state is not None: + now = datetime.now(UTC) + updated = state.model_copy( + update={"last_decay_check_at": now}, + ) + self._trust_states[key] = updated return result🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/synthorg/security/trust/service.py` around lines 329 - 335, check_decay currently does an unlocked read of self._trust_states (state = self._trust_states.get(key)) and then writes back an updated model after an await, so get_or_initialize_agent can race and overwrite the new state; fix by re-checking and performing the read-modify-write under self._state_lock after the await: after the await on line 325 acquire self._state_lock, re-read state = self._trust_states.get(key), and if not None create the updated model_copy with last_decay_check_at and assign it back to self._trust_states[key]; reference check_decay, get_or_initialize_agent, _trust_states, _state_lock and last_decay_check_at when locating and applying the change.web/src/api/types/enums.ts (1)
17-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRe-export the
ApprovalSourcetype alongside its tuple.Line 21 adds
APPROVAL_SOURCE_VALUES, but the matchingtype ApprovalSourceis not surfaced from this stable import module, unlike the other enum exports in this file.Suggested fix
export { ACTIVITY_EVENT_TYPE_VALUES, AGENT_STATUS_VALUES, APPROVAL_RISK_LEVEL_VALUES, APPROVAL_SOURCE_VALUES, APPROVAL_STATUS_VALUES, @@ type ActivityEventType, type AgentStatus, type ApprovalRiskLevel, + type ApprovalSource, type ApprovalStatus,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/api/types/enums.ts` around lines 17 - 73, The file re-exports APPROVAL_SOURCE_VALUES but omitted the corresponding type; add "type ApprovalSource" to the export list so the ApprovalSource type from enum-values.gen is surfaced alongside APPROVAL_SOURCE_VALUES, matching the pattern used for other enums like ApprovalRiskLevel/ type ApprovalRiskLevel and ApprovalStatus/ type ApprovalStatus.src/synthorg/hr/registry.py (1)
667-699:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't persist a terminal auto-approved item before the autonomy mutation succeeds.
approval_store.add(item)happens before the second lock acquisition that appliesautonomy_level. If the agent is unregistered or the registry is cleared in that await gap, this method raisesAgentNotFoundErrorafter anAPPROVEDapproval has already been persisted, leaving audit state that says the promotion succeeded when the registry never changed. Keep the approval non-terminal until after the mutation succeeds, or add compensating rollback/update logic for this failure path.Also applies to: 719-724
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/mocks/handlers/agents.ts`:
- Around line 77-90: The handler reads request.json() into body and calls
body.reason.trim(), which throws if reason isn't a string; update the guard
around reason in this handler to first check typeof body.reason === "string" (or
coerce safely) before calling .trim(), and return the same 422
HttpResponse.json(apiError("Field 'reason' is required"), { status: 422 }) when
reason is missing, not a string, or when trimmed length < 3; locate this logic
around the body variable and the HttpResponse.json/apiError calls to apply the
fix.
In `@web/src/stores/approvals.ts`:
- Around line 273-276: The code accepts frames missing c.source and then
sanitizeApproval (via sanitizeWsEnum) silently coerces source to 'review_gate';
update the validation to require and validate source before treating the frame
as an ApprovalResponse by either adding a presence-and-enum check to
isApprovalShape or by having sanitizeApproval explicitly assert c.source exists
and is one of APPROVAL_SOURCE_VALUES (using sanitizeWsEnum only after that
check) so malformed/pre-upgrade frames are rejected or handled as a looser type
rather than being coerced to 'review_gate'. Reference symbols: isApprovalShape,
sanitizeApproval, sanitizeWsEnum, APPROVAL_SOURCE_VALUES, and c.source.
---
Outside diff comments:
In `@src/synthorg/security/trust/service.py`:
- Around line 329-335: check_decay currently does an unlocked read of
self._trust_states (state = self._trust_states.get(key)) and then writes back an
updated model after an await, so get_or_initialize_agent can race and overwrite
the new state; fix by re-checking and performing the read-modify-write under
self._state_lock after the await: after the await on line 325 acquire
self._state_lock, re-read state = self._trust_states.get(key), and if not None
create the updated model_copy with last_decay_check_at and assign it back to
self._trust_states[key]; reference check_decay, get_or_initialize_agent,
_trust_states, _state_lock and last_decay_check_at when locating and applying
the change.
In `@web/src/api/types/enums.ts`:
- Around line 17-73: The file re-exports APPROVAL_SOURCE_VALUES but omitted the
corresponding type; add "type ApprovalSource" to the export list so the
ApprovalSource type from enum-values.gen is surfaced alongside
APPROVAL_SOURCE_VALUES, matching the pattern used for other enums like
ApprovalRiskLevel/ type ApprovalRiskLevel and ApprovalStatus/ type
ApprovalStatus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 61e2fe6d-49d7-4a36-a9c8-cac2c19564ad
📒 Files selected for processing (61)
data/runtime_stats.yamldocs/design/security.mdscripts/_ghost_wiring_manifest.txtsrc/synthorg/api/app.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.pysrc/synthorg/api/state.pysrc/synthorg/core/approval.pysrc/synthorg/core/enums.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/hr/registry.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/security.pysrc/synthorg/observability/events/trust.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/persistence/postgres/revisions/20260518000001_approval_source.sqlsrc/synthorg/persistence/postgres/schema.sqlsrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/persistence/sqlite/revisions/20260518000001_approval_source.sqlsrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/security/autonomy/models.pysrc/synthorg/security/config.pysrc/synthorg/security/service.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/workers/execution_service.pysrc/synthorg/workers/runtime_builder.pytests/e2e/test_runtime_online_seam.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/api/controllers/test_autonomy.pytests/unit/api/test_guards.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_agent_engine_resume.pytests/unit/engine/test_agent_engine_trust.pytests/unit/engine/test_approval_gate.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/hr/test_registry_autonomy.pytests/unit/meta/test_approval_repo.pytests/unit/security/trust/test_enforcement.pytests/unit/workers/test_execution_service.pytests/unit/workers/test_runtime_builder.pyweb/src/__tests__/helpers/factories.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/enums.tsweb/src/api/types/openapi.gen.tsweb/src/mocks/handlers/agents.tsweb/src/mocks/handlers/approvals.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/stores/approvals.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Test Integration
- GitHub Check: Dashboard Test
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test E2E
- GitHub Check: Test Unit
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (18)
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Onlysrc/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Use DB > env > code default via SettingsService/ConfigResolver (Cat-1) or env > code default with read_only_post_init (Cat-2); Cat-3 bootstrap secrets are pure env at boot site. YAML is company-template ingestion format only, not a precedence tier. No os.environ.get outside startup; pre-init Cat-2 reads use settings.bootstrap_resolver.resolve_init_value
Use from synthorg.observability import get_logger; variable always named logger. Never import logging or use print() in app code. Event names from observability.events. constants; structured kwargs (logger.info(EVENT, key=value))
Error paths log WARNING/ERROR with context before raising; state transitions log INFO via *_STATUS_TRANSITIONED after persistence write
Never log error=str(exc) or interpolate {exc}; use error_type=type(exc).name + error=safe_error_description(exc). Never use exc_info=True. OTel: forbidden span.record_exception(exc); use span.set_attribute("exception.message", safe_error_description(exc)) + record_exception=False, set_status_on_exception=False. Enforced by check_logger_exception_str_exc.py
Files:
src/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/core/enums.pysrc/synthorg/core/approval.pysrc/synthorg/security/service.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pysrc/synthorg/observability/events/security.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/security/config.pysrc/synthorg/api/state.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/api/app.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/hr/registry.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Numerics live insettings/definitions/; allowlist 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal). Enforced by scripts/check_no_magic_numbers.py
Comments should explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py
Do not usefrom __future__ import annotations(Python 3.14 has PEP 649). Use PEP 758 except:except A, B:requires parens when binding
Type hints required on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines
Define errors as<Domain><Condition>Errorinheriting fromDomainError; never inherit from Exception/RuntimeError/etc directly. Enforced by check_domain_error_hierarchy.py
Pydantic v2: use frozen + extra="forbid" on every frozen model project-wide (gate check_frozen_model_extra_forbid.py;@computed_fieldauto-exempt, per-line # lint-allow: frozen-extra-forbid -- for extra="allow"/"ignore" boundaries). Use@computed_fieldfor derived fields. Use NotBlankStr for identifiers
Args models at every system boundary; parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py
Use immutability patterns: model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries
Use asyncio.TaskGroup for fan-out/fan-in async patterns; helpers must catch Exception (re-raise MemoryError/RecursionError)
Clock seam: includeclock: Clock | None = Noneparameter; tests inject FakeClock. Lifecycle: services own_lifecycle_lock; timed-out stops mark unrestartable
For untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML
Never use real vendor names in project code/tests; use example-provider, test-provider, example-{large,medium,small}-001. Allowed in .claude/, third-party imports, providers/presets.py, we...
Files:
src/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/core/enums.pysrc/synthorg/core/approval.pysrc/synthorg/security/service.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pysrc/synthorg/observability/events/security.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/security/config.pysrc/synthorg/api/state.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/api/app.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/hr/registry.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pysrc/synthorg/observability/events/approval_gate.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/core/enums.pysrc/synthorg/core/approval.pysrc/synthorg/security/service.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pysrc/synthorg/observability/events/security.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/security/config.pysrc/synthorg/api/state.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pysrc/synthorg/api/app.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/hr/registry.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.py
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/__tests__/helpers/factories.tsweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enum-values.gen.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/mocks/handlers/agents.tsweb/src/mocks/handlers/approvals.tsweb/src/api/types/dtos.gen.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/api/types/enums.tsweb/src/api/types/openapi.gen.tsweb/src/stores/approvals.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/__tests__/helpers/factories.tsweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enum-values.gen.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/mocks/handlers/agents.tsweb/src/mocks/handlers/approvals.tsweb/src/api/types/dtos.gen.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/api/types/enums.tsweb/src/api/types/openapi.gen.tsweb/src/stores/approvals.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design tokens only in the Web Dashboard; seeweb/CLAUDE.mdfor details
Files:
web/src/__tests__/helpers/factories.tsweb/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/api/types/enum-values.gen.tsweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/mocks/handlers/agents.tsweb/src/mocks/handlers/approvals.tsweb/src/api/types/dtos.gen.tsweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsxweb/src/api/types/enums.tsweb/src/api/types/openapi.gen.tsweb/src/stores/approvals.ts
web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{jsx,tsx}: Use@eslint-react/no-leaked-conditional-renderingto catch the{count && <Foo />}bug where0renders verbatim. ForReactNode | undefinedprops use{value != null && value !== false && <jsx>}; for compound truthiness useBoolean(...).
Use@eslint-react/globalsto restrictwindow/document/localStorage/ etc. inside render. Hoist offenders into auseCallbackevent handler, auseEffect, or auseSyncExternalStore-backed hook.
Files:
web/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsx
web/src/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Storybook 10 is ESM-only; essentials are built into core, but
@storybook/addon-docsis now separate; imports moved tostorybook/testandstorybook/actions
Files:
web/src/pages/approvals/ApprovalTimeline.stories.tsxweb/src/pages/approvals/ApprovalCard.stories.tsxweb/src/pages/approvals/ApprovalDetailDrawer.stories.tsx
web/src/api/types/**/*.gen.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Generated DTO types (MANDATORY): NEVER hand-edit
web/src/api/types/*.gen.ts. Regenerate withuv run python scripts/generate_dto_types_ts.py. Import DTOs via the barrel (import type { AgentConfig } from '@/api/types').
Files:
web/src/api/types/enum-values.gen.tsweb/src/api/types/dtos.gen.tsweb/src/api/types/openapi.gen.ts
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}. Async auto. Timeout 30s global. Coverage 80% min
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back
Test doubles: use FakeClock for Clock seam, mock_ofT for typed-boundary substitutions, SimpleNamespace for attribute-bags. Never use bare MagicMock at typed boundary (constructor/fn arg/annotated local/typed fixture return); blocked by scripts/check_mock_spec.py (zero-tolerance, no baseline). Import FakeClock and mock_of from tests._shared
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add@example(...))
Flaky tests: NEVER skip/xfail; fix fundamentally. Use asyncio.Event().wait() not sleep(large)
Files:
tests/unit/security/trust/test_enforcement.pytests/e2e/test_runtime_online_seam.pytests/unit/meta/test_approval_repo.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/hr/test_registry_autonomy.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_agent_engine_resume.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/api/test_guards.pytests/unit/engine/test_agent_engine_trust.pytests/unit/workers/test_runtime_builder.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_approval_gate.pytests/unit/workers/test_execution_service.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/security/trust/test_enforcement.pytests/e2e/test_runtime_online_seam.pytests/unit/meta/test_approval_repo.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/hr/test_registry_autonomy.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_agent_engine_resume.pytests/unit/api/controllers/test_autonomy.pytests/unit/engine/test_loop_helpers_approval.pytests/unit/api/test_guards.pytests/unit/engine/test_agent_engine_trust.pytests/unit/workers/test_runtime_builder.pytests/unit/engine/test_mcp_self_consumer.pytests/unit/api/controllers/test_approvals_helpers.pytests/unit/engine/test_approval_gate.pytests/unit/workers/test_execution_service.py
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/mocks/handlers/**/*.ts: MSW handlers (MANDATORY):web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler for every exported endpoint. UseonUnhandledRequest: 'error'in test setup; tests override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*').
Use typed envelope helpers (successFor,paginatedFor,voidSuccess) to keep MSW handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/agents.tsweb/src/mocks/handlers/approvals.ts
src/synthorg/observability/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Telemetry: opt-in, off by default. Every event property must be in _ALLOWED_PROPERTIES. See telemetry.md
Files:
src/synthorg/observability/events/approval_gate.pysrc/synthorg/observability/events/trust.pysrc/synthorg/observability/events/security.py
{README.md,docs/**/*.md,data/**/*.md}
📄 CodeRabbit inference engine (CLAUDE.md)
Numerics in README and public docs must be sourced from data/runtime_stats.yaml via markers
Files:
docs/design/security.md
docs/**/*.{md,d2}
📄 CodeRabbit inference engine (CLAUDE.md)
Use d2 for architecture/nested containers, mermaid for flowcharts/sequence/pipelines. Markdown tables for tabular data. D2 theme 200 (Dark Mauve), D2 CLI pinned to v0.7.1 in CI
Files:
docs/design/security.md
src/synthorg/{workers,api}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Runtime services: synthorg.workers.runtime_builder.build_runtime_services selects behind ONE provider-present switch, returning RuntimeServices (AgentEngineExecutionService + coordinator or NoProviderExecutionService + None). _install_runtime_services appends FIRST after persistence/SettingsService hooks; swap_worker_execution_service / swap_coordinator / swap_provider_registry hold a lock. Empty-company rejects task creation (AgentRuntimeNotConfiguredError 4014) and /coordinate 503s
Files:
src/synthorg/workers/runtime_builder.pysrc/synthorg/api/state.pysrc/synthorg/api/app.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.py
src/synthorg/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/persistence/**/*.py: New repository protocols must inherit from one or more generic categories inpersistence/_generics.py(SingletonRepository, IdKeyedRepository, FilteredQueryRepository, AppendOnlyRepository, StatefulRepository, MVCCRepository); bespoke methods allowed only under ADR-0001 D7 with real performance optimisation or domain invariant justification
Keep repository protocols as@runtime_checkable
Repository CRUD methods: save(entity), get(id), delete(id) -> bool, list_items(...), query(...) returning tuples
Datetime in persistence: use parse_iso_utc / format_iso_utc from persistence._shared (reject naive); use normalize_utc for already-typed datetimes
Files:
src/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/persistence/sqlite/approval_repo.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
API startup lifecycle: construction phase wires synchronous services; on_startup wires services needing connected persistence. Construction-phase invariants: agent_registry before auto_wire_meetings; tunnel_provider wired unconditionally. On-startup invariants: SettingsService auto-wire before WorkflowExecutionObserver registration; OntologyService wires after persistence.connect() via _wire_ontology_service
Files:
src/synthorg/api/state.pysrc/synthorg/api/app.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/api/controllers/autonomy.pysrc/synthorg/api/lifecycle_builder.py
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/stores/**/*.ts: List reads (fetch*) must seterror: string | nullon the store instead of toasting
Test teardown (MANDATORY): any new store that schedules timers or attaches event listeners must expose an equivalent cleanup hook and register it in the globalafterEach. The globalafterEachinweb/src/test-setup.tsxalready callsuseToastStore.getState().dismissAll(),cancelPendingPersist(), anduseThemeStore.getState().teardown().
Files:
web/src/stores/approvals.ts
web/src/{api/endpoints,stores}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Cursor pagination (MANDATORY): list endpoints must use opaque cursor-based paging via
PaginationMeta. Stores must keepnextCursor+hasMorein state (not offset arithmetic) and early-return when!hasMore || !nextCursor. Display counts must come fromdata.length.
Files:
web/src/stores/approvals.ts
🧠 Learnings (5)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/security/trust/service.pysrc/synthorg/tools/approval_tool.pytests/unit/security/trust/test_enforcement.pytests/e2e/test_runtime_online_seam.pysrc/synthorg/observability/events/approval_gate.pytests/unit/meta/test_approval_repo.pysrc/synthorg/security/trust/enforcement.pysrc/synthorg/core/enums.pysrc/synthorg/core/approval.pysrc/synthorg/security/service.pytests/unit/engine/test_approval_gate_wiring.pytests/unit/hr/test_registry_autonomy.pysrc/synthorg/workers/runtime_builder.pytests/unit/api/test_startup_wiring.pytests/unit/engine/test_agent_engine_resume.pysrc/synthorg/security/autonomy/models.pysrc/synthorg/observability/events/trust.pytests/unit/api/controllers/test_autonomy.pysrc/synthorg/observability/events/security.pysrc/synthorg/persistence/postgres/approval_repo.pysrc/synthorg/security/config.pytests/unit/engine/test_loop_helpers_approval.pysrc/synthorg/api/state.pytests/unit/api/test_guards.pytests/unit/engine/test_agent_engine_trust.pysrc/synthorg/engine/agent_engine_factories.pysrc/synthorg/engine/mcp_self_consumer.pytests/unit/workers/test_runtime_builder.pysrc/synthorg/api/app.pysrc/synthorg/engine/agent_engine.pysrc/synthorg/hr/registry.pytests/unit/engine/test_mcp_self_consumer.pysrc/synthorg/api/app_builders.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/workers/execution_service.pysrc/synthorg/engine/approval_gate.pysrc/synthorg/engine/agent_engine_resume.pysrc/synthorg/persistence/sqlite/approval_repo.pysrc/synthorg/api/controllers/autonomy.pytests/unit/api/controllers/test_approvals_helpers.pysrc/synthorg/api/lifecycle_builder.pytests/unit/engine/test_approval_gate.pytests/unit/workers/test_execution_service.py
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/design/security.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/design/security.md
🔇 Additional comments (50)
src/synthorg/tools/approval_tool.py (1)
161-172: LGTM!web/src/__tests__/helpers/factories.ts (1)
237-237: LGTM!web/src/pages/approvals/ApprovalTimeline.stories.tsx (1)
12-12: LGTM!tests/unit/security/trust/test_enforcement.py (1)
1-76: LGTM!web/src/pages/approvals/ApprovalCard.stories.tsx (1)
13-13: LGTM!tests/e2e/test_runtime_online_seam.py (1)
131-139: LGTM!web/src/mocks/handlers/approvals.ts (1)
23-23: LGTM!src/synthorg/observability/events/approval_gate.py (1)
19-20: LGTM!web/src/api/types/dtos.gen.ts (1)
144-144: LGTM!web/src/pages/approvals/ApprovalDetailDrawer.stories.tsx (1)
13-13: LGTM!src/synthorg/persistence/sqlite/revisions/20260518000001_approval_source.sql (1)
10-11: LGTM!tests/unit/meta/test_approval_repo.py (1)
25-25: LGTM!src/synthorg/security/trust/enforcement.py (1)
20-64: LGTM!src/synthorg/core/enums.py (1)
582-601: LGTM!src/synthorg/core/approval.py (1)
20-21: LGTM!Also applies to: 46-51, 63-63
src/synthorg/security/service.py (1)
589-590: LGTM!Also applies to: 598-601
scripts/_ghost_wiring_manifest.txt (1)
28-31: LGTM!src/synthorg/persistence/postgres/revisions/20260518000001_approval_source.sql (1)
10-11: LGTM!src/synthorg/persistence/postgres/schema.sql (1)
1152-1154: LGTM!tests/unit/engine/test_approval_gate_wiring.py (1)
61-99: LGTM!tests/unit/hr/test_registry_autonomy.py (1)
27-27: LGTM!Also applies to: 148-188
docs/design/security.md (1)
8-10: LGTM!Also applies to: 98-113
src/synthorg/workers/runtime_builder.py (1)
31-31: LGTM!Also applies to: 167-174
tests/unit/api/test_startup_wiring.py (1)
17-20: LGTM!Also applies to: 202-264
tests/unit/engine/test_agent_engine_resume.py (1)
1-99: LGTM!data/runtime_stats.yaml (1)
2-3: LGTM!Also applies to: 6-6, 10-10
src/synthorg/security/autonomy/models.py (2)
10-12: LGTM!Also applies to: 157-172
263-271: ⚡ Quick winNo client-controlled auto-approval risk:
granted_by_strategyis not exposed in the HTTP request model.
AutonomyLevelRequest(the HTTP boundary model) contains onlylevelandreasonfields, withextra="forbid"enforced. The controller constructsAutonomyUpdateinternally and populatesgranted_by_strategyexclusively from server-side strategy evaluation. Clients cannot inject or influence this field.> Likely an incorrect or invalid review comment.src/synthorg/observability/events/trust.py (1)
18-19: LGTM!tests/unit/api/controllers/test_autonomy.py (1)
1-7: LGTM!Also applies to: 9-19, 31-45, 71-104, 105-122, 123-130, 131-138, 144-144
src/synthorg/observability/events/security.py (1)
196-196: LGTM!src/synthorg/persistence/postgres/approval_repo.py (1)
22-22: LGTM!Also applies to: 47-47, 53-53, 60-60, 123-123, 188-188, 260-260
src/synthorg/security/config.py (1)
302-342: LGTM!Also applies to: 444-447
web/src/api/types/openapi.gen.ts (1)
5764-5764: LGTM!Also applies to: 5776-5795, 5975-5975, 5995-5996, 6030-6059
src/synthorg/persistence/sqlite/schema.sql (1)
1074-1076: LGTM!tests/unit/engine/test_loop_helpers_approval.py (1)
18-18: LGTM!Also applies to: 55-112
src/synthorg/api/state.py (1)
119-121: LGTM!Also applies to: 194-194, 351-352, 391-391, 1056-1067, 1301-1313
tests/unit/api/test_guards.py (1)
163-169: LGTM!Also applies to: 176-183, 185-185, 188-193
tests/unit/engine/test_agent_engine_trust.py (1)
1-102: LGTM!src/synthorg/engine/agent_engine_factories.py (1)
19-25: LGTM!Also applies to: 30-30, 46-50, 79-87, 191-229, 293-305
src/synthorg/engine/mcp_self_consumer.py (1)
1-139: LGTM!tests/unit/workers/test_runtime_builder.py (1)
70-70: LGTM!src/synthorg/engine/approval_gate.py (1)
29-29: LGTM!Also applies to: 353-374, 513-549, 562-566, 582-591
src/synthorg/engine/agent_engine_resume.py (1)
1-261: LGTM!src/synthorg/persistence/sqlite/approval_repo.py (1)
15-15: LGTM!Also applies to: 41-42, 44-45, 51-52, 121-121, 219-219, 285-285, 399-400, 436-437, 488-489, 557-558
src/synthorg/api/controllers/autonomy.py (1)
3-4: LGTM!Also applies to: 7-7, 14-18, 23-26, 29-34, 46-69, 136-143, 152-156, 158-158, 161-185, 187-214, 218-223, 229-230
tests/unit/api/controllers/test_approvals_helpers.py (1)
3-3: LGTM!Also applies to: 8-8, 28-28, 35-37, 41-45, 55-55, 61-69, 144-144, 152-159, 161-164, 170-174, 184-193, 195-201, 207-211, 221-224, 234-265, 267-302, 312-312, 340-340, 371-371, 392-392, 394-394, 401-401, 428-428
src/synthorg/api/lifecycle_builder.py (1)
198-264: LGTM!Also applies to: 700-727, 990-1003
tests/unit/engine/test_approval_gate.py (1)
10-10: LGTM!Also applies to: 182-217, 304-359, 410-441
tests/unit/workers/test_execution_service.py (1)
19-21: LGTM!Also applies to: 30-31, 302-437
trust/service check_decay: post-eval timestamp RMW now under _state_lock (it awaited evaluate_agent; a concurrent locked writer could be clobbered by the stale-base write -- a real lost-update, distinct from the safe immutable-read case). hr/registry strategy-grant: apply the autonomy mutation BEFORE persisting the terminal APPROVED approval item, so a failure in the await gap (agent unregistered) can no longer leave an APPROVED audit row for a promotion that never happened. PENDING (non-terminal) path unchanged. _approval_review_gate: AgentRuntimeNotConfiguredError from dispatch_resume now propagates instead of being swallowed-as-handled (parked run can never resume -> false success). Transient failures still swallowed + review-gate suppressed. New propagate-path test. web: agents.ts guards typeof reason before .trim() (non-string -> 422 not throw); stores/approvals.ts isApprovalShape requires source (pre-upgrade frame rejected, not coerced) + test; enums.ts re-exports type ApprovalSource.
registry: keep approval_id typed str (used as ApprovalItem.id); surface None in the result via 'approval_id if approval_enqueued else None' / result_id instead of reassigning the var to None (was: str|None widening -> ApprovalItem id arg-type errors). eslint: replace destructure-rename omit with an explicit Record copy + delete (no unused binding).
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/synthorg/api/controllers/_approval_review_gate.py`:
- Around line 84-115: Wrap the call to await
app_state.approval_store.get(approval_id) in a try/except that treats any
non-fatal exception like a missing row: catch Exception as exc, log a warning
using APPROVAL_GATE_RESUME_FAILED with approval_id,
error_type=type(exc).__name__, error=safe_error_description(exc) and a note
indicating the reread failed, then set item = None and continue to the existing
parked-context fallback (gate = app_state.approval_gate ...
gate.has_parked_context(approval_id)); do not re-raise so the flow uses the
missing-item probe instead of hard-failing.
In `@src/synthorg/hr/registry.py`:
- Around line 710-752: The current flow mutates self._agents and snapshots
before calling approval_store.add, which can leave the registry changed if the
approval write fails; instead create and persist the approval item first by
calling approval_store.add(...) with the _ApprovalItem (keeping the same
fields/status), set approval_enqueued = True only after that completes, and only
then acquire self._lock to update self._agents and call self._snapshot(applied,
...); reference the approval_store.add call, the _ApprovalItem construction,
self._lock, self._agents, and self._snapshot to locate where to reorder
operations so the registry is only mutated after the approval write succeeds.
In `@web/src/mocks/handlers/agents.ts`:
- Around line 77-95: The handler should normalize the parsed payload from
request.json() before accessing properties: after calling request.json() (the
variable body), coerce non-object results (null, arrays, primitives) to an empty
object so reads like body.level won't throw; then perform the existing level
presence check to return the 400, and keep the existing typeof body.reason ===
'string' guard to compute reason and enforce the 3-char 422 validation; update
the code around the request.json() call and the body/ reason handling in
web/src/mocks/handlers/agents.ts accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d3cb376-907b-4646-8ad1-780f24cbd006
📒 Files selected for processing (8)
src/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/hr/registry.pysrc/synthorg/security/trust/service.pytests/unit/api/controllers/test_approvals_helpers.pyweb/src/__tests__/stores/approvals.test.tsweb/src/api/types/enums.tsweb/src/mocks/handlers/agents.tsweb/src/stores/approvals.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Build Backend
- GitHub Check: Test E2E
- GitHub Check: Test Unit
- GitHub Check: Test Integration
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Dashboard Test
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (12)
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/__tests__/stores/approvals.test.tsweb/src/api/types/enums.tsweb/src/mocks/handlers/agents.tsweb/src/stores/approvals.ts
web/src/{stores,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (web/CLAUDE.md)
Active-handle gate (MANDATORY): every unit test runs under
web/test-infra/active-handle-tracker.ts, which fails any test that leaks an event-loop-holding resource. A new store that schedules timers / attaches listeners MUST expose a teardown hook and register it in the globalafterEach; otherwise the gate fails the first test that triggers the schedule.
Files:
web/src/__tests__/stores/approvals.test.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/__tests__/stores/approvals.test.tsweb/src/api/types/enums.tsweb/src/mocks/handlers/agents.tsweb/src/stores/approvals.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design tokens only in the Web Dashboard; seeweb/CLAUDE.mdfor details
Files:
web/src/__tests__/stores/approvals.test.tsweb/src/api/types/enums.tsweb/src/mocks/handlers/agents.tsweb/src/stores/approvals.ts
web/src/mocks/handlers/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/mocks/handlers/**/*.ts: MSW handlers (MANDATORY):web/src/mocks/handlers/must mirrorweb/src/api/endpoints/*.ts1:1 with a default happy-path handler for every exported endpoint. UseonUnhandledRequest: 'error'in test setup; tests override per-case viaserver.use(...), nevervi.mock('@/api/endpoints/*').
Use typed envelope helpers (successFor,paginatedFor,voidSuccess) to keep MSW handlers in lockstep with endpoint return types
Files:
web/src/mocks/handlers/agents.ts
web/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/stores/**/*.ts: List reads (fetch*) must seterror: string | nullon the store instead of toasting
Test teardown (MANDATORY): any new store that schedules timers or attaches event listeners must expose an equivalent cleanup hook and register it in the globalafterEach. The globalafterEachinweb/src/test-setup.tsxalready callsuseToastStore.getState().dismissAll(),cancelPendingPersist(), anduseThemeStore.getState().teardown().
Files:
web/src/stores/approvals.ts
web/src/{api/endpoints,stores}/**/*.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Cursor pagination (MANDATORY): list endpoints must use opaque cursor-based paging via
PaginationMeta. Stores must keepnextCursor+hasMorein state (not offset arithmetic) and early-return when!hasMore || !nextCursor. Display counts must come fromdata.length.
Files:
web/src/stores/approvals.ts
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Onlysrc/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Use DB > env > code default via SettingsService/ConfigResolver (Cat-1) or env > code default with read_only_post_init (Cat-2); Cat-3 bootstrap secrets are pure env at boot site. YAML is company-template ingestion format only, not a precedence tier. No os.environ.get outside startup; pre-init Cat-2 reads use settings.bootstrap_resolver.resolve_init_value
Use from synthorg.observability import get_logger; variable always named logger. Never import logging or use print() in app code. Event names from observability.events. constants; structured kwargs (logger.info(EVENT, key=value))
Error paths log WARNING/ERROR with context before raising; state transitions log INFO via *_STATUS_TRANSITIONED after persistence write
Never log error=str(exc) or interpolate {exc}; use error_type=type(exc).name + error=safe_error_description(exc). Never use exc_info=True. OTel: forbidden span.record_exception(exc); use span.set_attribute("exception.message", safe_error_description(exc)) + record_exception=False, set_status_on_exception=False. Enforced by check_logger_exception_str_exc.py
Files:
src/synthorg/hr/registry.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/security/trust/service.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Numerics live insettings/definitions/; allowlist 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal). Enforced by scripts/check_no_magic_numbers.py
Comments should explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py
Do not usefrom __future__ import annotations(Python 3.14 has PEP 649). Use PEP 758 except:except A, B:requires parens when binding
Type hints required on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines
Define errors as<Domain><Condition>Errorinheriting fromDomainError; never inherit from Exception/RuntimeError/etc directly. Enforced by check_domain_error_hierarchy.py
Pydantic v2: use frozen + extra="forbid" on every frozen model project-wide (gate check_frozen_model_extra_forbid.py;@computed_fieldauto-exempt, per-line # lint-allow: frozen-extra-forbid -- for extra="allow"/"ignore" boundaries). Use@computed_fieldfor derived fields. Use NotBlankStr for identifiers
Args models at every system boundary; parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py
Use immutability patterns: model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries
Use asyncio.TaskGroup for fan-out/fan-in async patterns; helpers must catch Exception (re-raise MemoryError/RecursionError)
Clock seam: includeclock: Clock | None = Noneparameter; tests inject FakeClock. Lifecycle: services own_lifecycle_lock; timed-out stops mark unrestartable
For untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML
Never use real vendor names in project code/tests; use example-provider, test-provider, example-{large,medium,small}-001. Allowed in .claude/, third-party imports, providers/presets.py, we...
Files:
src/synthorg/hr/registry.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/security/trust/service.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/hr/registry.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/security/trust/service.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}. Async auto. Timeout 30s global. Coverage 80% min
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back
Test doubles: use FakeClock for Clock seam, mock_ofT for typed-boundary substitutions, SimpleNamespace for attribute-bags. Never use bare MagicMock at typed boundary (constructor/fn arg/annotated local/typed fixture return); blocked by scripts/check_mock_spec.py (zero-tolerance, no baseline). Import FakeClock and mock_of from tests._shared
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add@example(...))
Flaky tests: NEVER skip/xfail; fix fundamentally. Use asyncio.Event().wait() not sleep(large)
Files:
tests/unit/api/controllers/test_approvals_helpers.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/api/controllers/test_approvals_helpers.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
API startup lifecycle: construction phase wires synchronous services; on_startup wires services needing connected persistence. Construction-phase invariants: agent_registry before auto_wire_meetings; tunnel_provider wired unconditionally. On-startup invariants: SettingsService auto-wire before WorkflowExecutionObserver registration; OntologyService wires after persistence.connect() via _wire_ontology_service
Files:
src/synthorg/api/controllers/_approval_review_gate.py
src/synthorg/{workers,api}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Runtime services: synthorg.workers.runtime_builder.build_runtime_services selects behind ONE provider-present switch, returning RuntimeServices (AgentEngineExecutionService + coordinator or NoProviderExecutionService + None). _install_runtime_services appends FIRST after persistence/SettingsService hooks; swap_worker_execution_service / swap_coordinator / swap_provider_registry hold a lock. Empty-company rejects task creation (AgentRuntimeNotConfiguredError 4014) and /coordinate 503s
Files:
src/synthorg/api/controllers/_approval_review_gate.py
🧠 Learnings (1)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/hr/registry.pytests/unit/api/controllers/test_approvals_helpers.pysrc/synthorg/api/controllers/_approval_review_gate.pysrc/synthorg/security/trust/service.py
_approval_review_gate: wrap the post-decision approval reread (extracted to _reread_approval_item helper to keep complexity <=10) -- a failed approval_store.get() degrades to the parked-context probe instead of 500ing after the decision is persisted.
hr/registry dual-write resolution: round-6 asked mutate-then-add; round-7 asks add-then-mutate. Both orderings only move the dual-write window. Keep mutate-then-add but soft-fail the APPROVED audit write -- the autonomy mutation is the source of truth (already snapshotted), so a failed best-effort audit row is logged loudly (new SECURITY_AUTONOMY_PROMOTION_AUDIT_FAILED event) and reported not-enqueued, never rolled back nor 5xx'd. Rolling back a correct promotion is worse than a missing audit row. Soft-fail test added; happy path unchanged.
web/mocks/handlers/agents.ts: normalize request.json() (null/array/primitive -> {}) before property reads.
no-review-origin-in-code gate: the dual-write comment cited round-6/round-7. Rewrite WHY-only (ordering-agnostic soft-fail rationale); the round history stays in the commit body where it belongs.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
<!-- HIGHLIGHTS_START --> ## Highlights > _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub Models). Commit-based changelog below._ ### What you'll notice - Multi-agent coordination is now active immediately on startup for smoother operation. - Governance rules are fully enforced during use, ensuring compliance at all times. - Coordination metrics update live, giving real-time insights into system activity. - Review agents are now reliably processed, preventing silent drops in tasks. - Sandbox containers can be reused for agents and tasks, speeding up execution and reducing overhead. ### What's new - Agents support online runtime with a minimal safety framework to improve stability. - Recorded LLM interactions can be deterministically replayed at the provider interface. - Distributed path validation has been enhanced for more robust data routing. - A client-simulation runtime was added for end-to-end testing of the IntakeEngine. - A new work pipeline spine architecture has been introduced to streamline task processing. ### Under the hood - Infrastructure, Python, and web dependencies have all been updated to latest versions. - Updated apko lockfiles in the CI/CD pipeline improve build consistency. <!-- HIGHLIGHTS_END --> :robot: I have created a release *beep* *boop* --- ## [0.8.6](v0.8.5...v0.8.6) (2026-05-19) ### Features * agent runtime online + minimal safety spine (runtime root) ([#2003](#2003)) ([e5eef1a](e5eef1a)), closes [#1956](#1956) * deterministic recorded-LLM cassette replay at the provider chokepoint ([#2010](#2010)) ([cabf55d](cabf55d)) * distributed path validation + hardening ([#2011](#2011)) ([a382e4a](a382e4a)), closes [#1966](#1966) * wire IntakeEngine via boot client-simulation runtime (e2e test harness) ([#2006](#2006)) ([6a9c0aa](6a9c0aa)), closes [#1961](#1961) * work pipeline spine ([#1960](#1960)) ([#2013](#2013)) ([29b64e3](29b64e3)) ### Bug Fixes * bring the multi-agent coordinator online at boot ([#2007](#2007)) ([180b38a](180b38a)), closes [#1958](#1958) * full governance enforcement online ([#1957](#1957)) ([#2005](#2005)) ([4140fc5](4140fc5)) * harden anti-ghost-wiring gate and fix silently-dropped review agents ([#2000](#2000)) ([89b57ce](89b57ce)) * make coordination metrics live ([#1959](#1959)) ([#2012](#2012)) ([c4775e2](c4775e2)) * sandbox lifecycle dispatch (per-agent / per-task container reuse) ([#2008](#2008)) ([03d2587](03d2587)), closes [#1965](#1965) ### Documentation * add GitButler concept-only concurrency research ([#1978](#1978)) ([#2009](#2009)) ([9e4f5c1](9e4f5c1)) * honest-hybrid refresh of README, site, and design specs ([#2001](#2001)) ([f485bea](f485bea)) ### CI/CD * update apko lockfiles ([#2004](#2004)) ([e2b9eee](e2b9eee)) ### Maintenance * Update Infrastructure dependencies ([#2014](#2014)) ([0b16bdf](0b16bdf)) * Update Python dependencies ([#2015](#2015)) ([a7224bb](a7224bb)) * Update Web dependencies ([#2016](#2016)) ([7a7fe76](7a7fe76)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 participant
Summary
Wires the four GHOST governance subsystems from EPIC #1955 so they actually run on a live agent behind the provider-present switch (the runtime root #1956/#2003 only wired the minimal safety spine).
ApprovalGate+ real park/resume. One gate constructed at boot inlifecycle_builder._wire_approval_gate(backed by the dual-backendParkedContextRepository), attached toAppStateand injected intoAgentEngineso the engine parks and/approvalsresumes on the same gate. NewAgentEngine.resume_parked_runrestores the parkedAgentContext, injects the approval decision (D21; SYSTEM-message follow-up since the loop appends the escalated tool result before the park check), and continues_execute.AgentEngineExecutionService.dispatch_resumeruns the resume as a trackedBackgroundTaskRegistrytask off the approve/reject request path, drained on shutdown.TrustServicethreaded into the tool-invoker seam; effective tool permissions are narrowed to the more restrictive of identity level and earned trust level (auto-initialised on first sight).DISABLEDstrategy is a no-op; switching strategy changes enforcement.engine/mcp_self_consumer.py(pluggable: StrEnum mode + frozen config + factory + safeDISABLEDdefault) exposes SynthOrg's own MCP tools to a running agent, trust-scoped viaMCPToolScoper, withactor=identitythreaded sorequire_admin_guardrailsfails closed on admin tools.config.autonomy.change_strategy(defaultHUMAN_ONLY), attached toAppState; the autonomy controller now enforces the D6 seniority rule, consults the strategy, and enqueues a real approval (replacing the always-deny stub).scripts/_ghost_wiring_manifest.txtflipsApprovalGate,TrustService,build_mcp_self_consumer,build_autonomy_change_strategytoENFORCEDin this PR.docs/design/security.mdupdated to current runtime-enforcement state.Test plan
test_agent_engine_resume.py,test_agent_engine_trust.py,test_mcp_self_consumer.py,test_enforcement.py; extended approval-gate / runtime-builder / execution-service / approvals-helper / autonomy / startup-wiring / guards tests; conversation-shape lock for the resume-injection invariant.uv run ruff check src/ tests/+ruff format+uv run mypy src/ tests/: clean (3903 files)./approvals→ resume on approve/reject; trust-strategy switch changes the permitted tool set; agent invokes a SynthOrg MCP read tool; admin tool fails closed without confirm/reason/actor; autonomy request routes through the strategy.Review coverage
Pre-PR reviewed by 22 review + audit-mini-pass agents (issue-resolution-verifier: all 5 acceptance criteria RESOLVED; ghost-wiring: PASS). 11 valid findings triaged and all implemented (
_audit/pre-pr-review/triage.md): fail-safe_cleanup_parked(no silent duplicate resume), louder no-parked-context path,wrap_untrustedfor the injected decision reason, self-validatingAutonomyLevelRequest.reason+ regenerated TS types, MSWreasonvalidation,NamedTupletrust result, logging coverage, function-length extraction, larger resume drain budget, tighter guard test, added tests. The ~20 "except A, B:syntax error" flags were dismissed as a confirmed PEP 758 false positive (CLAUDE.md-mandated; all gates green).Closes #1957