feat: implement shutdown strategies and SUSPENDED task status#1151
feat: implement shutdown strategies and SUSPENDED task status#1151
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new non‑terminal task status SUSPENDED with transitions ASSIGNED → SUSPENDED, IN_PROGRESS → SUSPENDED, and SUSPENDED → ASSIGNED (resume). Introduces three explicit shutdown strategies and a factory in a new module: ImmediateCancelStrategy, FinishCurrentToolStrategy (with tool_timeout_seconds), and CheckpointAndStopStrategy (using a CheckpointSaver). Tightens GracefulShutdownConfig.strategy to enumerated literals, adds ShutdownResult.tasks_suspended, refactors shutdown helpers/logging, updates task transition maps, kanban/off‑board mappings, decomposition rollups, observability event constants, frontend types/UI mappings, and related tests. No public removals. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR extends the engine’s graceful shutdown subsystem by adding three additional shutdown strategies and introducing a new non-terminal SUSPENDED task status to support checkpoint-based shutdown/resume, along with corresponding docs, config, and tests.
Changes:
- Added
SUSPENDEDtoTaskStatus, updated valid transition map, and mappedSUSPENDEDoff-board in Kanban. - Implemented additional shutdown strategies (
immediate,finish_tool,checkpoint), addedtasks_suspendedtoShutdownResult, and introduced abuild_shutdown_strategyfactory. - Expanded observability events, configuration (
tool_timeout_seconds), and updated design documentation + unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/engine/test_shutdown.py | Adds unit coverage for new strategies, factory, and tasks_suspended field. |
| tests/unit/core/test_task_transitions.py | Extends transition tests to include SUSPENDED transitions and invariants. |
| tests/unit/core/test_enums.py | Updates enum cardinality/value tests for TaskStatus.SUSPENDED. |
| src/synthorg/observability/events/execution.py | Adds shutdown-related event constants for new strategies/checkpointing. |
| src/synthorg/engine/workflow/kanban_columns.py | Maps SUSPENDED to off-board (None) and updates status commentary. |
| src/synthorg/engine/shutdown.py | Implements new shutdown strategies, shared helpers, result field, and factory. |
| src/synthorg/engine/init.py | Re-exports new shutdown-related symbols from synthorg.engine. |
| src/synthorg/core/task_transitions.py | Adds SUSPENDED transitions into the global transition map. |
| src/synthorg/core/enums.py | Adds TaskStatus.SUSPENDED and updates transition summary documentation. |
| src/synthorg/config/schema.py | Adds tool_timeout_seconds to GracefulShutdownConfig. |
| docs/design/engine.md | Updates lifecycle diagram + shutdown strategy documentation (incl. SUSPENDED vs INTERRUPTED). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -70,6 +80,13 @@ class ShutdownResult(BaseModel): | |||
| ge=0, | |||
| description="Number of tasks that exited cooperatively", | |||
| ) | |||
| tasks_suspended: int = Field( | |||
| ge=0, | |||
| default=0, | |||
| description=( | |||
| "Number of tasks checkpointed before stop (checkpoint strategy only)" | |||
| ), | |||
| ) | |||
| cleanup_completed: bool = Field( | |||
There was a problem hiding this comment.
ShutdownResult gained a tasks_suspended field, but the class docstring’s Attributes list above still omits it. Update the docstring so it reflects the current model fields (including what tasks_suspended means and when it’s non-zero).
src/synthorg/engine/shutdown.py
Outdated
| if task_set: | ||
| logger.info( | ||
| EXECUTION_SHUTDOWN_IMMEDIATE_CANCEL, | ||
| running_tasks=tasks_interrupted, | ||
| ) | ||
| for task in task_set: | ||
| task.cancel() | ||
| cancel_done, _ = await asyncio.wait( | ||
| task_set, | ||
| timeout=self._CANCEL_PROPAGATION_TIMEOUT, | ||
| ) | ||
| _log_post_cancel_exceptions(cancel_done) | ||
|
|
||
| cleanup_completed = await _run_cleanup(cleanup_callbacks, self._cleanup_seconds) | ||
|
|
||
| return ShutdownResult( | ||
| strategy_type=self.get_strategy_type(), | ||
| tasks_interrupted=tasks_interrupted, | ||
| tasks_completed=0, | ||
| cleanup_completed=cleanup_completed, | ||
| duration_seconds=time.monotonic() - start, | ||
| ) |
There was a problem hiding this comment.
ImmediateCancelStrategy.execute_shutdown() returns a result but never emits the standard EXECUTION_SHUTDOWN_COMPLETE event that CooperativeTimeoutStrategy (and CheckpointAndStopStrategy) emit. For consistent observability across strategies, log the completion event with the same key fields (strategy, tasks_interrupted/completed/suspended, cleanup_completed, duration_seconds).
src/synthorg/engine/shutdown.py
Outdated
| logger.info( | ||
| EXECUTION_SHUTDOWN_TOOL_WAIT, | ||
| tool_timeout_seconds=self._tool_timeout_seconds, | ||
| running_tasks=len(running_tasks), | ||
| ) | ||
|
|
||
| if not running_tasks: | ||
| cleanup_completed = await _run_cleanup( | ||
| cleanup_callbacks, self._cleanup_seconds | ||
| ) | ||
| return ShutdownResult( | ||
| strategy_type=self.get_strategy_type(), | ||
| tasks_interrupted=0, | ||
| tasks_completed=0, | ||
| cleanup_completed=cleanup_completed, | ||
| duration_seconds=time.monotonic() - start, | ||
| ) | ||
|
|
||
| task_set = set(running_tasks.values()) | ||
| done, pending = await asyncio.wait( | ||
| task_set, | ||
| timeout=self._tool_timeout_seconds, | ||
| ) | ||
|
|
||
| # Count cooperative exits (exclude errored tasks). | ||
| tasks_completed = 0 | ||
| for task in done: | ||
| if task.cancelled(): | ||
| continue | ||
| try: | ||
| exc = task.exception() | ||
| except asyncio.InvalidStateError: | ||
| logger.debug( | ||
| exc = task.exception() | ||
| if exc is not None: | ||
| logger.warning( | ||
| EXECUTION_SHUTDOWN_TASK_ERROR, | ||
| error="Failed to inspect post-cancel task: InvalidStateError", | ||
| task_name=task.get_name(), | ||
| error=f"Task raised during shutdown: {type(exc).__name__}: {exc}", | ||
| ) | ||
| else: | ||
| if exc is not None: | ||
| logger.debug( | ||
| EXECUTION_SHUTDOWN_TASK_ERROR, | ||
| error=( | ||
| f"Post-cancel task exception: {type(exc).__name__}: {exc}" | ||
| ), | ||
| task_name=task.get_name(), | ||
| ) | ||
| tasks_completed += 1 | ||
|
|
||
| # Force-cancel stragglers. | ||
| if pending: | ||
| logger.warning( | ||
| EXECUTION_SHUTDOWN_FORCE_CANCEL, | ||
| pending_tasks=len(pending), | ||
| ) | ||
| for task in pending: | ||
| task.cancel() | ||
| cancel_done, _ = await asyncio.wait( | ||
| pending, | ||
| timeout=self._CANCEL_PROPAGATION_TIMEOUT, | ||
| ) | ||
| _log_post_cancel_exceptions(cancel_done) | ||
|
|
||
| cleanup_completed = await _run_cleanup(cleanup_callbacks, self._cleanup_seconds) | ||
|
|
||
| return ShutdownResult( | ||
| strategy_type=self.get_strategy_type(), | ||
| tasks_interrupted=len(pending), | ||
| tasks_completed=tasks_completed, | ||
| cleanup_completed=cleanup_completed, | ||
| duration_seconds=time.monotonic() - start, | ||
| ) |
There was a problem hiding this comment.
FinishCurrentToolStrategy.execute_shutdown() returns without logging EXECUTION_SHUTDOWN_COMPLETE, unlike the other strategies. This makes shutdown metrics/log queries strategy-dependent; add the completion log with the same fields as the other strategies.
src/synthorg/engine/shutdown.py
Outdated
| if self._checkpoint_saver is None: | ||
| return False | ||
| try: | ||
| await asyncio.wait_for( | ||
| _run_all(), | ||
| timeout=self._cleanup_seconds, | ||
| saved = await self._checkpoint_saver(task_id) | ||
| except Exception as exc: | ||
| logger.exception( | ||
| EXECUTION_SHUTDOWN_CHECKPOINT_FAILED, | ||
| task_id=task_id, | ||
| error_type=type(exc).__name__, | ||
| ) | ||
| return False | ||
| if saved: | ||
| logger.info( | ||
| EXECUTION_SHUTDOWN_CHECKPOINT_SAVE, | ||
| task_id=task_id, | ||
| ) |
There was a problem hiding this comment.
_try_checkpoint() awaits checkpoint_saver(task_id) with no timeout. If the saver hangs (I/O stall, dead DB connection), shutdown can block indefinitely even though grace/cleanup phases are time-bounded. Wrap the saver call in a bounded timeout (e.g., derived from cleanup_seconds or a dedicated checkpoint timeout) and treat timeouts as a failed checkpoint (count as interrupted + log).
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/config/schema.py`:
- Around line 469-474: Update the Google-style docstring for the class
GracefulShutdownConfig to include the new public field tool_timeout_seconds in
the "Attributes" section: add an entry like "tool_timeout_seconds (float):
Per-tool timeout for finish_tool strategy (default 60.0, gt 0, le 300)." Ensure
the type and brief description match the Field(...) definition in the class so
the generated docs reflect the updated model surface.
In `@src/synthorg/core/enums.py`:
- Line 255: TaskStatus.SUSPENDED has been added but assignment flow still
restricts allowed transitions via _ASSIGNABLE_STATUSES in assignment/service.py
(the set currently contains CREATED, FAILED, INTERRUPTED), preventing SUSPENDED
-> ASSIGNED; update the _ASSIGNABLE_STATUSES set in assignment/service.py to
include TaskStatus.SUSPENDED (reference the enum symbol SUSPENDED and the enum
class TaskStatus in src/synthorg/core/enums.py) so suspended tasks can be
resumed, and add/adjust any related tests or comments to reflect the new
resumable status.
- Line 255: The rollup logic currently treats suspended subtasks as IN_PROGRESS
and loses checkpointed state; update the rollup/status-aggregation code to
explicitly handle TaskStatus.SUSPENDED: in the function that aggregates child
statuses in rollup.py (the status rollup/aggregate_status routine) add a branch
for TaskStatus.SUSPENDED so suspended children preserve their state instead of
being considered IN_PROGRESS, and in the DecompositionTask model methods in
models.py that compute or reconcile parent status/checkpoints (the
compute_status/update_from_children or equivalent) add logic to propagate
SUSPENDED to parents per your intended semantics (e.g., parent becomes SUSPENDED
if all children are SUSPENDED or preserve checkpoint info when any child is
SUSPENDED), ensuring checkpointed-state fields are not overwritten by fallback
IN_PROGRESS handling.
In `@src/synthorg/engine/shutdown.py`:
- Around line 595-600: The except block that handles checkpoint saver failures
currently logs via logger.exception(EXECUTION_SHUTDOWN_CHECKPOINT_FAILED,
task_id=task_id, error_type=type(exc).__name__) but swallows the error by
returning False; change this to log the exception as currently done and then
re-raise the exception (use raise) so fatal errors (e.g., MemoryError) are
propagated; alternatively, if returning False is intended for recoverable cases,
narrow the except to the specific expected exception types instead of catching
bare Exception in the function handling checkpoint saves.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7abbe9e-e625-4c55-868a-53688cb50457
📒 Files selected for processing (11)
docs/design/engine.mdsrc/synthorg/config/schema.pysrc/synthorg/core/enums.pysrc/synthorg/core/task_transitions.pysrc/synthorg/engine/__init__.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/workflow/kanban_columns.pysrc/synthorg/observability/events/execution.pytests/unit/core/test_enums.pytests/unit/core/test_task_transitions.pytests/unit/engine/test_shutdown.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). (8)
- GitHub Check: Deploy Preview
- GitHub Check: Agent
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) -- PEP 758 except syntax, enforced by ruff on Python 3.14
All public functions must have type hints. Use mypy strict mode.
Google style docstrings are required on public classes and functions. Enforced by ruff D rules.
Use immutability patterns: create new objects, never mutate existing ones. For non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping.
Use frozen Pydantic models for config/identity; separate mutable-via-copy models for runtime state. Never mix static config fields with mutable runtime fields.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Includeallow_inf_nan=Falsein allConfigDictdeclarations.
Use@computed_fieldfor derived values instead of storing + validating redundant fields
UseNotBlankStr(fromcore.types) for all identifier/name fields, including optional and tuple variants, instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code
Line length must be 88 characters. Enforced by ruff.
Functions must be less than 50 lines, files must be less than 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Always use variable namelogger(not_logger, notlog) for logger instances
Files:
tests/unit/core/test_enums.pysrc/synthorg/config/schema.pysrc/synthorg/engine/workflow/kanban_columns.pytests/unit/core/test_task_transitions.pysrc/synthorg/core/enums.pysrc/synthorg/engine/__init__.pysrc/synthorg/observability/events/execution.pysrc/synthorg/engine/shutdown.pysrc/synthorg/core/task_transitions.pytests/unit/engine/test_shutdown.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Useasyncio_mode = 'auto'for async tests -- no manual@pytest.mark.asyncioneeded
Global test timeout is 30 seconds per test in pyproject.toml. Non-default overrides liketimeout(60)are allowed, but do not add per-filepytest.mark.timeout(30)markers.
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing with@given+@settingsdecorators
Never skip, dismiss, or ignore flaky tests. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For indefinite blocks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/core/test_enums.pytests/unit/core/test_task_transitions.pytests/unit/engine/test_shutdown.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/core/test_enums.pytests/unit/core/test_task_transitions.pytests/unit/engine/test_shutdown.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST importfrom synthorg.observability import get_loggerand setlogger = get_logger(__name__)
Never useimport loggingorlogging.getLogger()orprint()in application code. Exceptions: observability/setup.py, observability/sinks.py, observability/syslog_handler.py, observability/http_handler.py, observability/otlp_handler.py may use stdlib logging and stderr print for handler construction.
Event names must always use constants from domain-specific modules undersynthorg.observability.events. Import directly from specific domain modules.
Use structured kwargs for logging: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
DEBUG logging should be used for object creation, internal flow, and entry/exit of key functions
RetryConfig and RateLimiterConfig are set per-provider inProviderConfig
Files:
src/synthorg/config/schema.pysrc/synthorg/engine/workflow/kanban_columns.pysrc/synthorg/core/enums.pysrc/synthorg/engine/__init__.pysrc/synthorg/observability/events/execution.pysrc/synthorg/engine/shutdown.pysrc/synthorg/core/task_transitions.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,test-provider,test-small-001, etc.
Files:
src/synthorg/config/schema.pysrc/synthorg/engine/workflow/kanban_columns.pysrc/synthorg/core/enums.pysrc/synthorg/engine/__init__.pysrc/synthorg/observability/events/execution.pysrc/synthorg/engine/shutdown.pysrc/synthorg/core/task_transitions.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/config/schema.pysrc/synthorg/engine/workflow/kanban_columns.pysrc/synthorg/core/enums.pysrc/synthorg/engine/__init__.pysrc/synthorg/observability/events/execution.pysrc/synthorg/engine/shutdown.pysrc/synthorg/core/task_transitions.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect approved deviations from the spec
Files:
docs/design/engine.md
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
📚 Learning: 2026-04-02T20:17:33.300Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T20:17:33.300Z
Learning: Applies to tests/**/*.py : 30-second global timeout per test (set in `pyproject.toml`); non-default overrides like `timeout(60)` are allowed
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to {pyproject.toml,src/synthorg/__init__.py} : Update version in `pyproject.toml` (`[tool.commitizen].version`) and `src/synthorg/__init__.py` (`__version__`)
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/engine/__init__.pydocs/design/engine.mdsrc/synthorg/engine/shutdown.pytests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-21T12:54:22.557Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T12:54:22.557Z
Learning: Version bumping (pre-1.0): `fix:` = patch, `feat:` = patch, `feat!:` = minor, `BREAKING CHANGE` trailer = minor. Update version in `pyproject.toml` (`[tool.commitizen].version`) and `src/synthorg/__init__.py` (`__version__`)
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/synthorg/api/**/*.py : Litestar API must include setup wizard, auth/, auto-wiring, and lifecycle management
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
src/synthorg/engine/__init__.pydocs/design/engine.mdsrc/synthorg/engine/shutdown.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/execution.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/core/task_transitions.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO level
Applied to files:
src/synthorg/core/task_transitions.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_shutdown.py
🪛 markdownlint-cli2 (0.22.0)
docs/design/engine.md
[warning] 54-54: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (26)
tests/unit/core/test_enums.py (1)
68-70: TaskStatus enum tests are correctly updated.Count and value checks now match the new 10-member contract including
SUSPENDED.Also applies to: 160-160
src/synthorg/engine/workflow/kanban_columns.py (1)
54-67: Kanban bridge update forSUSPENDEDis consistent.Treating
SUSPENDEDas off-board (None) aligns with checkpoint/shutdown behavior and keeps mapping completeness intact.tests/unit/core/test_task_transitions.py (1)
37-43: Transition tests forSUSPENDEDare solid.The new valid/invalid cases and map-completeness assertions properly lock down the intended lifecycle behavior.
Also applies to: 70-74, 129-137
src/synthorg/engine/__init__.py (1)
170-180: Engine re-exports are correctly synchronized.The new shutdown strategy symbols and factory are consistently imported and exposed in
__all__.Also applies to: 254-258, 296-302, 394-394
src/synthorg/core/task_transitions.py (1)
5-18:SUSPENDEDtransition map updates are internally consistent.Allowed edges and documentation align (
ASSIGNED/IN_PROGRESS -> SUSPENDED,SUSPENDED -> ASSIGNED).Also applies to: 39-39, 49-49, 63-63
src/synthorg/observability/events/execution.py (1)
51-56: New shutdown observability constants look good.The added event keys are clear, domain-consistent, and already consumed by shutdown strategy logging.
docs/design/engine.md (3)
18-49: LGTM - State diagram correctly updated.The Mermaid state diagram properly shows the new
SUSPENDEDstate with:
ASSIGNED --> SUSPENDED : checkpoint shutdownIN_PROGRESS --> SUSPENDED : checkpoint shutdownSUSPENDED --> ASSIGNED : resume from checkpointThis matches the task transition rules documented in the PR objectives.
51-62: LGTM - Non-terminal states documentation is clear.The addition of
SUSPENDEDto the non-terminal states list with proper cross-reference to Strategy 4 in the Graceful Shutdown section provides good traceability.
1188-1237: LGTM - Shutdown strategies documentation is comprehensive.The new Strategy 2/3/4 documentation:
- Clearly explains each strategy's behavior
- Provides appropriate YAML configuration examples
- The "SUSPENDED vs INTERRUPTED" info box (lines 1226-1230) effectively distinguishes the two non-terminal states
The documentation aligns well with the implementation in
shutdown.py.tests/unit/engine/test_shutdown.py (8)
12-23: LGTM - Imports correctly updated.The imports properly include all new strategy classes (
CheckpointAndStopStrategy,FinishCurrentToolStrategy,ImmediateCancelStrategy), the type alias (CheckpointSaver), and the refactored helper (_log_post_cancel_exceptions) and factory (build_shutdown_strategy).
427-455: LGTM - Tests correctly updated for refactored helper.The
TestLogPostCancelExceptionsclass now calls the module-level_log_post_cancel_exceptions({task})directly instead of the previous instance method, matching the refactoring inshutdown.py.
506-541: LGTM -tasks_suspendedfield tests are thorough.Tests cover:
- Default value of 0
- Explicit setting
- Frozen model validation
This validates the backward-compatible addition of the new field.
547-628: LGTM -ImmediateCancelStrategytests are comprehensive.Coverage includes:
- Protocol compliance (runtime checkable, shutdown state, strategy type)
- Force-cancel behavior (all tasks cancelled immediately)
- Empty task handling
- Cleanup callback execution
- Constructor validation for invalid parameters
633-725: LGTM -FinishCurrentToolStrategytests are comprehensive.Coverage includes:
- Protocol compliance
- Cooperative exit within timeout
- Straggler cancellation after timeout
- Cleanup callbacks
- Empty task handling
- Constructor validation
730-911: LGTM -CheckpointAndStopStrategytests are thorough.Excellent coverage of checkpoint scenarios:
- Cooperative exit counted as suspended (line 756)
- Straggler checkpointed then cancelled (line 773)
- Checkpoint failure counted as interrupted (line 791)
- No checkpoint saver falls back to interrupted (line 809)
- Checkpoint saver exception handling (line 824)
- Mixed cooperative and straggler scenarios (line 846)
- Cleanup callbacks and empty tasks
- Constructor validation
916-970: LGTM -build_shutdown_strategyfactory tests are complete.Tests verify:
- All four strategy types map correctly
- Unknown strategy raises
ValueError- Configuration parameters propagate (grace_seconds, cleanup_seconds)
tool_timeout_secondspropagates toFinishCurrentToolStrategycheckpoint_saveris injected intoCheckpointAndStopStrategy
976-982: LGTM - Helper function is well-designed.The
_make_saverhelper creates a simple async checkpoint saver stub with configurable success/failure, making tests readable and avoiding duplication.src/synthorg/engine/shutdown.py (9)
51-52: LGTM - CheckpointSaver type alias is well-documented.The type alias
Callable[[str], Coroutine[Any, Any, bool]]with the docstring clearly communicates the contract: takes a task_id, returns True on success.
83-89: LGTM - tasks_suspended field properly added.The field:
- Has
ge=0constraint- Has
default=0for backward compatibility- Has a clear description indicating it's specific to checkpoint strategy
- Is part of a frozen model (immutable)
268-331: LGTM -ImmediateCancelStrategyimplementation is clean.The strategy:
- Validates constructor parameters
- Sets shutdown event before cancelling
- Logs the immediate cancel action
- Properly waits for cancellation to propagate
- Calls shared
_log_post_cancel_exceptionsand_run_cleanuphelpers
334-443: LGTM -FinishCurrentToolStrategyimplementation is correct.The strategy:
- Uses configurable
tool_timeout_seconds(default 60s)- Properly counts cooperative vs interrupted tasks
- Reuses the same force-cancel pattern for stragglers
- Shares helpers with other strategies
543-555: Minor: Consider logging unknown task_id at WARNING instead of DEBUG.When a task cannot be found in the reverse map during checkpoint (line 544-549), this indicates a potential bug or race condition in task tracking. Logging at DEBUG may cause this issue to go unnoticed in production.
However, this was noted in the PR objectives as an addressed finding ("debug log for unknown task_id lookup"), so this appears to be an intentional design decision to avoid noisy logs for edge cases that don't affect correctness.
585-613: LGTM -_try_checkpointerror handling is robust.The method:
- Returns
Falseearly if no saver is configured- Catches all exceptions and logs with
logger.exception(includes traceback) anderror_type- Logs success at INFO and failure (saver returned False) at WARNING
- Never raises, ensuring checkpoint failures don't crash shutdown
619-643: LGTM -_log_post_cancel_exceptionshelper is well-extracted.The helper:
- Handles
InvalidStateErrorgracefully (task not in a state where exception can be retrieved)- Logs at DEBUG level (appropriate for post-cancel diagnostics)
- Includes task name for traceability
- Prevents "Task exception was never retrieved" warnings
646-691: LGTM -_run_cleanuphelper is well-designed.The helper:
- Handles empty callbacks list efficiently (returns True immediately)
- Logs cleanup start with callback count and timeout
- Runs callbacks sequentially (preserving order guarantees)
- Isolates callback failures (one failure doesn't prevent others)
- Uses
logger.exceptionfor failed callbacks (includes traceback)- Handles timeout with clear logging
- Returns boolean to indicate success/failure
869-912: LGTM -build_shutdown_strategyfactory is well-implemented.The factory:
- Uses a dictionary of lambdas for clean strategy mapping
- Propagates all relevant config parameters to each strategy
- Provides a clear error message with known strategies listed
- Accepts optional
checkpoint_saverfor the checkpoint strategy only- Uses
TYPE_CHECKINGimport forGracefulShutdownConfigto avoid circular imports
| BLOCKED = "blocked" | ||
| FAILED = "failed" | ||
| INTERRUPTED = "interrupted" | ||
| SUSPENDED = "suspended" |
There was a problem hiding this comment.
SUSPENDED cannot currently be resumed through assignment flow.
TaskStatus.SUSPENDED is introduced as resumable, but src/synthorg/engine/assignment/service.py:32-34 still restricts _ASSIGNABLE_STATUSES to {CREATED, FAILED, INTERRUPTED}. That blocks SUSPENDED -> ASSIGNED operationally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/core/enums.py` at line 255, TaskStatus.SUSPENDED has been added
but assignment flow still restricts allowed transitions via _ASSIGNABLE_STATUSES
in assignment/service.py (the set currently contains CREATED, FAILED,
INTERRUPTED), preventing SUSPENDED -> ASSIGNED; update the _ASSIGNABLE_STATUSES
set in assignment/service.py to include TaskStatus.SUSPENDED (reference the enum
symbol SUSPENDED and the enum class TaskStatus in src/synthorg/core/enums.py) so
suspended tasks can be resumed, and add/adjust any related tests or comments to
reflect the new resumable status.
Decomposition rollups are not yet SUSPENDED-aware.
src/synthorg/engine/decomposition/rollup.py:61-65 and src/synthorg/engine/decomposition/models.py:232-259 omit explicit TaskStatus.SUSPENDED handling, so suspended subtasks collapse into fallback IN_PROGRESS behavior and lose checkpointed-state fidelity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/core/enums.py` at line 255, The rollup logic currently treats
suspended subtasks as IN_PROGRESS and loses checkpointed state; update the
rollup/status-aggregation code to explicitly handle TaskStatus.SUSPENDED: in the
function that aggregates child statuses in rollup.py (the status
rollup/aggregate_status routine) add a branch for TaskStatus.SUSPENDED so
suspended children preserve their state instead of being considered IN_PROGRESS,
and in the DecompositionTask model methods in models.py that compute or
reconcile parent status/checkpoints (the compute_status/update_from_children or
equivalent) add logic to propagate SUSPENDED to parents per your intended
semantics (e.g., parent becomes SUSPENDED if all children are SUSPENDED or
preserve checkpoint info when any child is SUSPENDED), ensuring
checkpointed-state fields are not overwritten by fallback IN_PROGRESS handling.
src/synthorg/engine/shutdown.py
Outdated
| except Exception as exc: | ||
| logger.exception( | ||
| EXECUTION_SHUTDOWN_CHECKPOINT_FAILED, | ||
| task_id=task_id, | ||
| error_type=type(exc).__name__, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Python best practices for catching exceptions in async callbacks - should MemoryError propagate?
💡 Result:
Async callbacks: what to catch (and what to let through)
1) Don’t rely on “propagation” from callbacks.
In asyncio, exceptions raised inside event-loop callbacks (e.g., functions scheduled with loop.call_soon, timer callbacks, some add_done_callback handlers) are typically logged and ignored by the loop rather than being raised back to the code that scheduled them. (peps.python.org)
So “let it propagate” often just means “it gets logged and the loop keeps running”, unless you explicitly escalate it (e.g., stop the loop, fail a supervisor task, call your monitoring, etc.).
2) Use except Exception: (not bare except: / BaseException) for callback wrappers.
This avoids accidentally swallowing process-/control-flow exceptions.
3) Always let cancellation through.
asyncio.CancelledError is a subclass of BaseException since Python 3.8, specifically so generic except Exception: handlers won’t block cancellation. (docs.python.org)
Should MemoryError propagate?
In almost all application code: yes—treat MemoryError as non-recoverable and do not “handle and continue” (especially not inside an async callback), because the process may be in a compromised state (allocations can fail in surprising places, logging/formatting can allocate, etc.).
However, since callback exceptions may be “logged and ignored” by default in asyncio, you often want to escalate MemoryError (and similar “can’t safely continue” errors) to a fail-fast path rather than merely logging.
A practical pattern for async callback wrappers
import asyncio
import logging
log = logging.getLogger(__name__)
def safe_callback(fn, *args, loop: asyncio.AbstractEventLoop | None = None, **kwargs):
try:
return fn(*args, **kwargs)
except (SystemExit, KeyboardInterrupt):
raise # always propagate
except asyncio.CancelledError:
raise # always propagate (BaseException subclass) # py>=3.8 ([docs.python.org](https://docs.python.org/3.12/library/asyncio-exceptions.html?utm_source=openai))
except MemoryError:
# don't attempt heavy logging/formatting; fail fast
raise
except Exception:
# expected “normal” failures: log + escalate in a controlled way
log.exception("Exception in async callback")
if loop is not None:
# optional: route to the loop's exception handler / monitoring hook
loop.call_exception_handler({
"message": "Unhandled exception in callback",
"exception": asyncio.get_running_loop(), # or the actual exception via sys.exc_info()
})
# often: return / swallow; or set a Future/Task result to notify supervisorRule of thumb
- Catch and handle only exceptions you can actually recover from (often domain errors,
OSError, protocol/IO issues). - Let through / fail fast on:
CancelledError,KeyboardInterrupt,SystemExit, and typicallyMemoryError. - If you need exceptions from background work to be actionable, prefer Tasks you
await(or TaskGroups) rather than “naked callbacks”, because awaiting is what re-raises task exceptions at a well-defined boundary (callbacks often won’t). (peps.python.org)
Citations:
- 1: https://peps.python.org/pep-3156/?utm_source=openai
- 2: https://docs.python.org/3.12/library/asyncio-exceptions.html?utm_source=openai
- 3: https://docs.python.org/3.12/library/asyncio-exceptions.html?utm_source=openai
- 4: https://peps.python.org/pep-3156/?utm_source=openai
🏁 Script executed:
# Get the full context around the checkpoint saver error handling
cd src/synthorg/engine && sed -n '590,610p' shutdown.pyRepository: Aureliolo/synthorg
Length of output: 719
Re-raise the exception after logging checkpoint saver failures.
The except Exception block silently swallows the error by returning False instead of re-raising. Per project guidelines, all error paths must log at WARNING or ERROR with context before raising. This applies especially to MemoryError and other non-recoverable errors.
Refactor to:
except Exception as exc:
logger.exception(
EXECUTION_SHUTDOWN_CHECKPOINT_FAILED,
task_id=task_id,
error_type=type(exc).__name__,
)
raiseIf checkpoint saver failures should be recoverable (i.e., return False is intentional), catch only the specific expected exception types instead of bare Exception.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/shutdown.py` around lines 595 - 600, The except block
that handles checkpoint saver failures currently logs via
logger.exception(EXECUTION_SHUTDOWN_CHECKPOINT_FAILED, task_id=task_id,
error_type=type(exc).__name__) but swallows the error by returning False; change
this to log the exception as currently done and then re-raise the exception (use
raise) so fatal errors (e.g., MemoryError) are propagated; alternatively, if
returning False is intended for recoverable cases, narrow the except to the
specific expected exception types instead of catching bare Exception in the
function handling checkpoint saves.
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'SUSPENDED' task status and implements several new graceful shutdown strategies: 'immediate', 'finish_tool', and 'checkpoint'. These changes include updates to the engine design documentation, task transition logic, configuration schemas, and observability events. I have provided feedback on reducing code duplication in the shutdown strategies and improving the robustness of the checkpointing logic for straggler tasks.
src/synthorg/engine/shutdown.py
Outdated
| # Count cooperative exits (exclude errored tasks). | ||
| tasks_completed = 0 | ||
| for task in done: | ||
| if task.cancelled(): | ||
| continue | ||
| try: | ||
| exc = task.exception() | ||
| except asyncio.InvalidStateError: | ||
| logger.debug( | ||
| exc = task.exception() | ||
| if exc is not None: | ||
| logger.warning( | ||
| EXECUTION_SHUTDOWN_TASK_ERROR, | ||
| error="Failed to inspect post-cancel task: InvalidStateError", | ||
| task_name=task.get_name(), | ||
| error=f"Task raised during shutdown: {type(exc).__name__}: {exc}", | ||
| ) | ||
| else: | ||
| if exc is not None: | ||
| logger.debug( | ||
| EXECUTION_SHUTDOWN_TASK_ERROR, | ||
| error=( | ||
| f"Post-cancel task exception: {type(exc).__name__}: {exc}" | ||
| ), | ||
| task_name=task.get_name(), | ||
| ) | ||
| tasks_completed += 1 |
There was a problem hiding this comment.
There's an opportunity to reduce code duplication. The logic for counting cooperative exits (iterating through done tasks, checking for exceptions, and incrementing a counter) is repeated in CooperativeTimeoutStrategy._wait_and_cancel, FinishCurrentToolStrategy.execute_shutdown, and CheckpointAndStopStrategy.execute_shutdown.
Consider extracting this logic into a shared helper function:
def _count_cooperative_exits(done_tasks: set[asyncio.Task[Any]]) -> int:
"""Counts tasks that finished without errors."""
count = 0
for task in done_tasks:
if task.cancelled():
continue
exc = task.exception()
if exc is not None:
logger.warning(
EXECUTION_SHUTDOWN_TASK_ERROR,
error=f"Task raised during shutdown: {type(exc).__name__}: {exc}",
)
else:
count += 1
return countYou can then replace the duplicated blocks with a call to this new helper, like tasks_completed = _count_cooperative_exits(done).
src/synthorg/engine/shutdown.py
Outdated
| task_id = task_to_id.get(task, "unknown") | ||
| if task_id == "unknown": | ||
| logger.debug( | ||
| EXECUTION_SHUTDOWN_TASK_ERROR, | ||
| error="Task not found in reverse map during checkpoint", | ||
| ) | ||
| saved = await self._try_checkpoint(task_id) | ||
| if saved: | ||
| tasks_suspended += 1 | ||
| else: | ||
| tasks_interrupted += 1 | ||
| task.cancel() |
There was a problem hiding this comment.
To improve robustness, it's better to avoid calling _try_checkpoint with the placeholder "unknown" task ID. You can refactor this loop to handle the 'unknown' case separately, ensuring the checkpoint_saver is only called with valid task identifiers.
| task_id = task_to_id.get(task, "unknown") | |
| if task_id == "unknown": | |
| logger.debug( | |
| EXECUTION_SHUTDOWN_TASK_ERROR, | |
| error="Task not found in reverse map during checkpoint", | |
| ) | |
| saved = await self._try_checkpoint(task_id) | |
| if saved: | |
| tasks_suspended += 1 | |
| else: | |
| tasks_interrupted += 1 | |
| task.cancel() | |
| task_id = task_to_id.get(task, "unknown") | |
| saved = False | |
| if task_id == "unknown": | |
| logger.debug( | |
| EXECUTION_SHUTDOWN_TASK_ERROR, | |
| error="Task not found in reverse map during checkpoint", | |
| ) | |
| else: | |
| saved = await self._try_checkpoint(task_id) | |
| if saved: | |
| tasks_suspended += 1 | |
| else: | |
| tasks_interrupted += 1 | |
| task.cancel() |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1151 +/- ##
==========================================
+ Coverage 88.86% 88.87% +0.01%
==========================================
Files 856 857 +1
Lines 50208 50408 +200
Branches 5047 5071 +24
==========================================
+ Hits 44619 44802 +183
- Misses 4631 4643 +12
- Partials 958 963 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 (1)
src/synthorg/config/schema.py (1)
456-477:⚠️ Potential issue | 🟠 MajorValidate
strategyinsideGracefulShutdownConfig.
build_shutdown_strategy()now rejects unknown names, but this model still accepts any nonblank string. That lets invalid YAML survive parsing and fail later at runtime instead of at the config boundary. This should be a closed set here as well, likeTaskAssignmentConfig.Suggested fix
class GracefulShutdownConfig(BaseModel): @@ + _VALID_STRATEGIES: ClassVar[frozenset[str]] = frozenset( + { + "cooperative_timeout", + "immediate", + "finish_tool", + "checkpoint", + } + ) + strategy: NotBlankStr = Field( default="cooperative_timeout", description="Shutdown strategy name", ) @@ tool_timeout_seconds: float = Field( default=60.0, gt=0, le=300, description="Per-tool timeout for finish_tool strategy", ) + + `@model_validator`(mode="after") + def _validate_strategy_name(self) -> Self: + """Ensure strategy is a known shutdown strategy name.""" + if self.strategy not in self._VALID_STRATEGIES: + msg = ( + f"Unknown shutdown strategy {self.strategy!r}. " + f"Valid strategies: {sorted(self._VALID_STRATEGIES)}" + ) + logger.warning( + CONFIG_VALIDATION_FAILED, + model="GracefulShutdownConfig", + error=msg, + strategy=self.strategy, + ) + raise ValueError(msg) + return selfAs per coding guidelines, "Validate at system boundaries in Python (user input, external APIs, config files)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/config/schema.py` around lines 456 - 477, The GracefulShutdownConfig currently allows any non-blank string for strategy; change it to validate against the closed set used by build_shutdown_strategy by replacing the free NotBlankStr with a Literal or Enum of the allowed strategy names (matching build_shutdown_strategy's accepted names) so invalid YAML fails at parsing; update the Field default to a valid member and, if you prefer, add a pydantic validator on GracefulShutdownConfig.strategy to assert membership (mirroring TaskAssignmentConfig's approach) to ensure config-time validation rather than runtime rejection in build_shutdown_strategy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/assignment/service.py`:
- Around line 32-38: The code added TaskStatus.SUSPENDED to the
_ASSIGNABLE_STATUSES set but the unit test parametrization that verifies
resumable statuses only covers CREATED/FAILED/INTERRUPTED; update that test's
parameter list to include TaskStatus.SUSPENDED so the new branch is exercised.
Locate the parametrized test in tests that target assignment service (the one
currently listing CREATED, FAILED, INTERRUPTED) and add TaskStatus.SUSPENDED to
the inputs, ensuring any assertions that resume behavior are executed for
SUSPENDED as well. Make sure to import/qualify TaskStatus in the test if not
already present.
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 400-421: In _checkpoint_and_cancel_pending(), don’t await
_try_checkpoint() serially; instead fan out checkpoint attempts for all pending
tasks using asyncio.TaskGroup (or asyncio.gather) so checkpointing runs
concurrently: for each task in pending, look up task_to_id, skip/log missing ids
and increment tasks_interrupted, otherwise start a TaskGroup child that calls
self._try_checkpoint(task_id) and record each task_id result to increment
tasks_suspended or tasks_interrupted accordingly after the group completes;
after the group finishes, cancel the original pending tasks, then run the
existing asyncio.wait(...) with self._CANCEL_PROPAGATION_TIMEOUT and call
_log_post_cancel_exceptions(cancel_done) as before. Ensure you still call
task.cancel() for missing task_ids and for all original tasks after
checkpointing and preserve the existing log messages
(EXECUTION_SHUTDOWN_TASK_ERROR) and counter updates.
---
Outside diff comments:
In `@src/synthorg/config/schema.py`:
- Around line 456-477: The GracefulShutdownConfig currently allows any non-blank
string for strategy; change it to validate against the closed set used by
build_shutdown_strategy by replacing the free NotBlankStr with a Literal or Enum
of the allowed strategy names (matching build_shutdown_strategy's accepted
names) so invalid YAML fails at parsing; update the Field default to a valid
member and, if you prefer, add a pydantic validator on
GracefulShutdownConfig.strategy to assert membership (mirroring
TaskAssignmentConfig's approach) to ensure config-time validation rather than
runtime rejection in build_shutdown_strategy.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: b80c5010-2581-4482-a608-937fdb83ded0
📒 Files selected for processing (14)
CLAUDE.mdsrc/synthorg/config/schema.pysrc/synthorg/engine/__init__.pysrc/synthorg/engine/assignment/service.pysrc/synthorg/engine/decomposition/models.pysrc/synthorg/engine/decomposition/rollup.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/shutdown_strategies.pytests/unit/engine/test_shutdown.pytests/unit/engine/workflow/test_kanban_columns.pyweb/src/api/types.tsweb/src/components/ui/task-status-indicator.stories.tsxweb/src/utils/constants.tsweb/src/utils/tasks.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). (6)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
No
from __future__ import annotationsin Python code — Python 3.14 has PEP 649 native lazy annotationsUse
except A, B:syntax (no parentheses) for exception handling in Python 3.14, as enforced by ruffAll public functions in Python must have type hints; mypy strict mode is enforced
Use Google style docstrings on public classes and functions in Python; required by ruff D rules
Use immutability patterns: create new objects rather than mutate existing ones; use
copy.deepcopy()for non-Pydantic internal collections and wrap withMappingProxyTypefor read-only enforcementUse frozen Pydantic models for config/identity; use separate mutable-via-copy models for runtime state
Use Pydantic v2 with
allow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation timeUse
@computed_fieldfor derived values in Pydantic models instead of storing and validating redundant fieldsUse
NotBlankStrfromcore.typesfor all identifier/name fields in Python, including optional and tuple variants, instead of manual whitespace validatorsPrefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in Python (e.g., multiple tool invocations, parallel agent calls) over barecreate_taskPython code must follow 88 character line length limit, enforced by ruff
Python functions must be less than 50 lines; Python files must be less than 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Use the variable name
logger(not_loggerorlog) for logging instances in PythonUse structured kwargs in Python logging: always
logger.info(EVENT, key=value), neverlogger.info("msg %s", val)All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG level logging for object creation, internal flow, and entry/exit of key f...
Files:
tests/unit/engine/workflow/test_kanban_columns.pysrc/synthorg/engine/assignment/service.pysrc/synthorg/engine/decomposition/rollup.pysrc/synthorg/config/schema.pysrc/synthorg/engine/__init__.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/decomposition/models.pytests/unit/engine/test_shutdown.pysrc/synthorg/engine/shutdown_strategies.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python test files must use
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkersPython tests must maintain 80% minimum code coverage, enforced in CI via
--cov-fail-under=80Prefer
@pytest.mark.parametrizefor testing similar cases in Python testsNever skip, dismiss, or ignore flaky Python tests — always fix them fully and fundamentally; use deterministic mocking for timing-sensitive tests
Files:
tests/unit/engine/workflow/test_kanban_columns.pytests/unit/engine/test_shutdown.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/workflow/test_kanban_columns.pytests/unit/engine/test_shutdown.py
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/logger-- never bareconsole.warn/console.error/console.debugin application code
Logger variable name must always beconst log(e.g.const log = createLogger('module-name'))
Pass dynamic/untrusted values as separate arguments to logger methods (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
Files:
web/src/components/ui/task-status-indicator.stories.tsxweb/src/utils/constants.tsweb/src/utils/tasks.tsweb/src/api/types.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-*)) for colors; NEVER hardcode hex values in.tsx/.tsfiles
Usefont-sansorfont-mono(Geist tokens) for typography; NEVER setfontFamilydirectly in.tsx/.tsfiles
Use density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing in components
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders; NEVER hardcode values in.tsx/.tsfiles
Use@/lib/motionpresets for Framer Motion transition durations; NEVER hardcode transition durations
CSS side-effect imports in TypeScript 6 require type declarations -- add/// <reference types="vite/client" />at the top of files with CSS importsWeb dashboard code in
web/src/must always reuse existing components fromweb/src/components/ui/before creating new onesWeb dashboard code must never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions; use design tokens and
@/lib/motionpresetsRun ESLint via pre-commit hook on web dashboard code with zero warnings; conditional on
web/src/**/*.{ts,tsx}
Files:
web/src/components/ui/task-status-indicator.stories.tsxweb/src/utils/constants.tsweb/src/utils/tasks.tsweb/src/api/types.ts
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/components/ui/**/*.{ts,tsx}: New shared components must be placed inweb/src/components/ui/with descriptive kebab-case filenames
Export component props as a TypeScript interface for all new components
Importcnfrom@/lib/utilsfor conditional class merging in component files
For Base UI primitives, import from specific subpaths (e.g.import { Dialog } from '@base-ui/react/dialog') and use the component'srenderprop for polymorphism
Usedata-[open],data-[closed],data-[starting-style],data-[ending-style]animation state attributes for Base UI primitives in Tailwind CSS (notdata-[state=open]/data-[state=closed])
In Tailwind v4, transition properties must list each one explicitly:transition-[opacity,translate]ortransition-[opacity,scale](not justtransition-[opacity,transform])
For Base UI Dialog, AlertDialog, and Popover components, compose withPortal+Backdrop+Popup. Popover and Menu additionally require aPositionerwrapper withside/align/sideOffsetprops
For Tabs Base UI components, usedata-[active]attribute selector (notdata-[state=active]) for animation state styling
Files:
web/src/components/ui/task-status-indicator.stories.tsx
web/src/components/ui/**/*.stories.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Create a
.stories.tsxfile alongside each new shared component with all states (default, hover, loading, error, empty)
Files:
web/src/components/ui/task-status-indicator.stories.tsx
web/src/**/*.stories.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.stories.tsx: For Storybook stories withtags: ['autodocs'], ensure@storybook/addon-docsis installed and added to addons
Usestorybook/testandstorybook/actionsimport paths in Storybook stories (not@storybook/testor@storybook/addon-actions)
Files:
web/src/components/ui/task-status-indicator.stories.tsx
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Every Python module with business logic must import
from synthorg.observability import get_loggerand definelogger = get_logger(__name__)Never use
import logging,logging.getLogger(), orprint()in Python application code except in observability setup modulesAlways use event name constants from domain-specific modules under
synthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api); import directly viafrom synthorg.observability.events.<domain> import EVENT_CONSTANTNever use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names like
example-provider,test-provider,test-small-001
Files:
src/synthorg/engine/assignment/service.pysrc/synthorg/engine/decomposition/rollup.pysrc/synthorg/config/schema.pysrc/synthorg/engine/__init__.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/decomposition/models.pysrc/synthorg/engine/shutdown_strategies.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/engine/assignment/service.pysrc/synthorg/engine/decomposition/rollup.pysrc/synthorg/config/schema.pysrc/synthorg/engine/__init__.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/decomposition/models.pysrc/synthorg/engine/shutdown_strategies.py
🧠 Learnings (46)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Always read the relevant `docs/design/` page before implementing any feature or planning any issue, as outlined in DESIGN_SPEC.md
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Alert the user and explain when implementation deviates from the design spec; do not silently diverge
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Always run pytest locally with `-n 8` for parallelism via pytest-xdist; never run tests sequentially
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Git commits must use format `<type>: <description>` where types are: feat, fix, refactor, docs, test, chore, perf, ci; enforced by commitizen via commit-msg hook
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Signed commits (GPG/SSH) are required on `main` branch via branch protection
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use branch naming format `<type>/<slug>` from main (e.g., `feat/agent-memory-v2`)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Always use `/pre-pr-review` to create PRs; `gh pr create` is blocked by hookify; use `/pre-pr-review quick` for trivial/docs-only changes
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Fix all valid findings from review agents, including pre-existing issues in surrounding code and suggestions adjacent to PR changes; never skip or defer
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use `uv run ruff check src/ tests/` for linting Python code; use `uv run ruff check src/ tests/ --fix` for auto-fix
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use `uv run ruff format src/ tests/` to format Python code
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use `uv run mypy src/ tests/` to type-check Python code in strict mode
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use `uv run pytest tests/ -n 8` to run the full Python test suite in parallel
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Never use `cd` in Bash commands — the working directory is already set to project root; use absolute paths or run commands directly
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Never use Bash to write or modify files — use Write or Edit tools; do not use `cat >`, `echo >`, `sed -i`, or similar
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Prioritize issues by dependency order, not priority labels; unblocked dependencies come first
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: At every phase of planning and implementation, be critical and actively look for ways to improve design in the spirit of robustness, correctness, simplicity, and future-proofing
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Surface improvements as suggestions, not silent changes — user decides on approval
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: After finishing an issue implementation, always create a feature branch, commit, and push — do NOT create a PR automatically; do NOT leave work uncommitted on main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use `go -C cli` (never `cd cli`) for Go CLI binary commands; referenced in `cli/CLAUDE.md`
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T18:00:25.294Z
Learning: Use `bash -c "cd <dir> && <cmd>"` for tools without a `-C` flag (safe child process execution with no cwd side effects)
📚 Learning: 2026-04-06T06:45:22.965Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-06T06:45:22.965Z
Learning: Applies to web/src/components/ui/**/*.stories.tsx : Create a `.stories.tsx` file alongside each new shared component with all states (default, hover, loading, error, empty)
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Create new shared components in `web/src/components/ui/` with `.stories.tsx` Storybook file covering all states (default, hover, loading, error, empty)
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: import from `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`)
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use storybook/test (not storybook/test) and storybook/actions (not storybook/addon-actions) import paths
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: Use `parameters.a11y.test: 'error' | 'todo' | 'off'` for a11y testing configuration (replaces old `.element` and `.manual`)
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-04-06T06:45:22.965Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-06T06:45:22.965Z
Learning: Applies to web/src/**/*.stories.tsx : Use `storybook/test` and `storybook/actions` import paths in Storybook stories (not `storybook/test` or `storybook/addon-actions`)
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.a11y.test: 'error' | 'todo' | 'off' for a11y testing (replaces old .element and .manual); set globally in preview.tsx to enforce WCAG compliance on all stories
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: Import from `storybook/test` instead of `storybook/test`
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.backgrounds.options (object keyed by name) + initialGlobals.backgrounds.value for background options (replaces old default + values array)
Applied to files:
web/src/components/ui/task-status-indicator.stories.tsx
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to **/*.py : Google style docstrings are required on public classes and functions. Enforced by ruff D rules.
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-03T07:50:58.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:50:58.367Z
Learning: Applies to **/*.py : Use Google-style docstrings, required on all public classes and functions (enforced by ruff D rules)
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/engine/__init__.pytests/unit/engine/test_shutdown.pysrc/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/engine/__init__.pysrc/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to {pyproject.toml,src/synthorg/__init__.py} : Update version in `pyproject.toml` (`[tool.commitizen].version`) and `src/synthorg/__init__.py` (`__version__`)
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/security/**/*.py : Security package (security/): SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume)
Applied to files:
src/synthorg/engine/__init__.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-26T15:18:16.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-26T15:18:16.848Z
Learning: Applies to src/**/*.py : Handle errors explicitly, never silently swallow them. All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-04-01T09:39:21.584Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:39:21.584Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : For tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)` -- it is cancellation-safe
Applied to files:
src/synthorg/engine/shutdown.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-06T16:35:12.934Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-06T16:35:12.934Z
Learning: All project conventions, commands, and standards are defined in CLAUDE.md - refer to it for project structure, package layout, code conventions, quick commands, git workflow, testing standards, design specifications, logging, resilience, and security patterns
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-02T12:07:44.443Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T12:07:44.443Z
Learning: Applies to src/synthorg/**/*.py : Always use structured logging: `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Use structured logging: always `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-02T20:17:33.300Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T20:17:33.300Z
Learning: Applies to tests/**/*.py : 30-second global timeout per test (set in `pyproject.toml`); non-default overrides like `timeout(60)` are allowed
Applied to files:
tests/unit/engine/test_shutdown.py
🔇 Additional comments (11)
web/src/api/types.ts (1)
5-15: LGTM!The
'suspended'status is correctly added to theTaskStatusunion type, aligning with the backendTaskStatus.SUSPENDEDenum member. The placement before'cancelled'maintains logical grouping of non-terminal states.web/src/components/ui/task-status-indicator.stories.tsx (1)
16-19: LGTM!The
'suspended'status is correctly added toALL_STATUSES, ensuring the Storybook story showcases all task statuses including the new one. The ordering is consistent withTaskStatustype definition andTASK_STATUS_ORDERin constants.web/src/utils/constants.ts (2)
24-35: LGTM!The
'suspended'status is correctly added toTASK_STATUS_ORDERfor Kanban column ordering, maintaining consistency with theTaskStatustype definition.
41-52: LGTM!The
VALID_TRANSITIONSmap correctly implements the PR-specified transitions:
assigned → suspended✓in_progress → suspended✓suspended → assigned(resume from checkpoint) ✓The transitions align with the Engine design spec §6.7 requirements for checkpoint-based shutdown.
web/src/utils/tasks.ts (4)
6-17: LGTM!The
'suspended'status is correctly mapped to'warning'color, which is consistent with'interrupted'(both are non-terminal pause states that can resume).
25-36: LGTM!The
'Suspended'label follows the established capitalization convention for status labels.
101-109: LGTM!Adding
'suspended'to theterminalcolumn's statuses is appropriate. This aligns with the backendSTATUS_TO_COLUMNmapping whereSUSPENDEDmaps toNone(off-board). The Kanban board correctly treats suspended tasks as terminal/archive items rather than active workflow states, since suspension is a system-controlled state triggered by checkpoint shutdown rather than a user-driven transition.
193-204: LGTM!The
VALID_TRANSITIONScorrectly implements the PR-specified transitions for'suspended':
assigned → suspended✓in_progress → suspended✓suspended → assigned(resume from checkpoint) ✓src/synthorg/engine/decomposition/rollup.py (1)
66-67:SUSPENDEDis correctly included in rollup aggregation.Good change: suspended subtasks are now explicitly counted and forwarded to
SubtaskStatusRollup, keeping rollup data consistent with the updated task lifecycle.Also applies to: 76-76
src/synthorg/engine/decomposition/models.py (1)
182-187:SubtaskStatusRollupchanges are coherent and complete forSUSPENDED.The new
suspendedfield is documented, validated in aggregate counts, and reflected inderived_parent_status, which keeps model semantics consistent.Also applies to: 201-201, 213-225, 255-257
src/synthorg/engine/shutdown_strategies.py (1)
13-17:⚠️ Potential issue | 🟠 MajorMove runtime-visible annotation imports out of
TYPE_CHECKING.
Mapping,Sequence,Callable, andGracefulShutdownConfigare imported only underif TYPE_CHECKING:, but appear in public method and function annotations. Withoutfrom __future__ import annotations, Python evaluates these at runtime using the module namespace, causingNameErroron access. Comparesrc/synthorg/engine/shutdown.py:20, which importstypesat module level with a# noqa: TC003comment to document runtime-visible annotation usage.Move imports to module level:
from collections.abc import Callable, Mapping, Sequencefrom synthorg.config.schema import GracefulShutdownConfigAlso applies to: 105–110, 193–198, 311–316, 472–476
⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-17T22:08:13.456Z Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-19T07:12:14.508Z Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-08T15:24:49.467Z Learning: Applies to **/*.py : No `from __future__ import annotations` -- Python 3.14 has PEP 649 native lazy annotationsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-21T11:08:01.542Z Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T14:31:11.894Z Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotationsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T14:18:02.890Z Learning: Applies to src/**/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/synthorg/engine/shutdown_strategies.py (1)
209-220:⚠️ Potential issue | 🟠 MajorEmit
EXECUTION_SHUTDOWN_COMPLETEonly from completed shutdown paths.Line 543 logs the completion event while the factory is still building a strategy, and Line 214 / Line 332 return from the real zero-task shutdown path without logging it at all. That produces a false completion before shutdown starts and misses the actual completion event for empty shutdowns.
Also applies to: 327-339, 542-548
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/shutdown_strategies.py` around lines 209 - 220, The code currently emits the EXECUTION_SHUTDOWN_COMPLETE event in the factory while building a strategy and omits it on actual completed shutdown paths (e.g., the zero-task path that returns a ShutdownResult from _run_cleanup); update the shutdown flow so the EXECUTION_SHUTDOWN_COMPLETE event is only emitted after a real shutdown completes. Specifically, ensure any early-return paths that construct and return a ShutdownResult (refer to _run_cleanup, ShutdownResult, and get_strategy_type) also trigger the completion event before returning, and remove/move the emission from the factory/build phase so the event is emitted consistently from the actual shutdown-completion locations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 45-70: _count_cooperative_exits currently only returns the number
of successful (non-cancelled, non-errored) tasks and drops errored tasks from
ShutdownResult totals; change it to return both the cooperative count and the
list (or set) of tasks that raised exceptions (e.g., return (count,
errored_tasks)), keep logging as-is, then update callers in
FinishCurrentToolStrategy and CheckpointAndStopStrategy to include those errored
tasks when building/aggregating the ShutdownResult (increment the failed/errored
counters and the total tracked tasks accordingly instead of omitting them) so
errored tasks are preserved in the final shutdown totals.
- Around line 11-16: Move the annotation types out of the TYPE_CHECKING block so
runtime introspection works: replace the conditional imports by adding top-level
imports for Mapping and Sequence (from collections.abc) and for
GracefulShutdownConfig (from synthorg.config.schema), and keep or add
Callable/Any as needed; then remove them from the TYPE_CHECKING block. Update
the module imports in src/synthorg/engine/shutdown_strategies.py so the names
Mapping, Sequence, Callable and GracefulShutdownConfig exist in module globals
(this will fix failures when functions that use these annotations — e.g., the
public functions referencing Mapping/Sequence/GracefulShutdownConfig in this
file — are introspected via get_type_hints or similar).
In `@web/src/__tests__/utils/tasks.test.ts`:
- Around line 24-27: The parameterized tests omit assertions for the 'suspended'
status even though ALL_STATUSES and the utilities include it; update the test
cases that call getTaskStatusColor and getTaskStatusLabel to include an explicit
assertion for the 'suspended' status (matching the expected color and label
defined in web/src/utils/tasks.ts) so both functions are exercised for that
status; locate the parameterized arrays/it.each blocks in
web/src/__tests__/utils/tasks.test.ts and add the suspended case to each
expected-results list to ensure parity with ALL_STATUSES.
---
Duplicate comments:
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 209-220: The code currently emits the EXECUTION_SHUTDOWN_COMPLETE
event in the factory while building a strategy and omits it on actual completed
shutdown paths (e.g., the zero-task path that returns a ShutdownResult from
_run_cleanup); update the shutdown flow so the EXECUTION_SHUTDOWN_COMPLETE event
is only emitted after a real shutdown completes. Specifically, ensure any
early-return paths that construct and return a ShutdownResult (refer to
_run_cleanup, ShutdownResult, and get_strategy_type) also trigger the completion
event before returning, and remove/move the emission from the factory/build
phase so the event is emitted consistently from the actual shutdown-completion
locations.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2626d00c-659f-42cf-b7fa-3ffeba748b2c
📒 Files selected for processing (5)
src/synthorg/config/schema.pysrc/synthorg/engine/shutdown_strategies.pytests/unit/engine/test_assignment_service.pytests/unit/engine/test_shutdown.pyweb/src/__tests__/utils/tasks.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use pytest markers:
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow. Maintain 80% coverage minimum (enforced in CI). Useasyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded. Global timeout is 30 seconds per test (configured inpyproject.toml) -- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed. Run tests in parallel with-n 8-- ALWAYS include-n 8when running pytest locally, never run tests sequentially. Prefer@pytest.mark.parametrizefor testing similar cases.Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases, ortest-provider,test-small-001in tests. Vendor names may only appear in: (1) Operations design page (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths/module names, (4) provider presets (src/synthorg/providers/presets.py).Use property-based testing with Hypothesis (
@given+@settings). Hypothesis profiles configured intests/conftest.py:ci(deterministic,max_examples=10+derandomize=True),dev(1000 examples),fuzz(10,000 examples, no deadline),extreme(500,000 examples, no deadline). Control viaHYPOTHESIS_PROFILEenv var..hypothesis/is gitignored. Failing examples are persisted to~/.synthorg/hypothesis-examples/via_WriteOnlyDatabase. When Hypothesis finds a failure, it is a real bug -- fix the underlying bug and add an explicit@example(...)decorator so the case is permanently covered in CI. Never skip or dismiss flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic. For tasks that must block indefinitely until cance...
Files:
tests/unit/engine/test_assignment_service.pytests/unit/engine/test_shutdown.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_assignment_service.pytests/unit/engine/test_shutdown.py
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/logger-- never bareconsole.warn/console.error/console.debugin application code
Logger variable name must always beconst log(e.g.const log = createLogger('module-name'))
Pass dynamic/untrusted values as separate arguments to logger methods (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
Files:
web/src/__tests__/utils/tasks.test.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-*)) for colors; NEVER hardcode hex values in.tsx/.tsfiles
Usefont-sansorfont-mono(Geist tokens) for typography; NEVER setfontFamilydirectly in.tsx/.tsfiles
Use density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing in components
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders; NEVER hardcode values in.tsx/.tsfiles
Use@/lib/motionpresets for Framer Motion transition durations; NEVER hardcode transition durations
CSS side-effect imports in TypeScript 6 require type declarations -- add/// <reference types="vite/client" />at the top of files with CSS importsAlways reuse existing components from
web/src/components/ui/before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and@/lib/motionpresets. Seeweb/CLAUDE.mdfor the full component inventory, design token rules, and post-training references (TS6, Storybook 10).A PostToolUse hook (
scripts/check_web_design_system.py) enforces design system rules on every Edit/Write toweb/src/. The hook prevents hardcoded colors, spacing, and transitions.
Files:
web/src/__tests__/utils/tasks.test.ts
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
No
from __future__ import annotationsin Python files. Python 3.14 has PEP 649 native lazy annotations.Use PEP 758 except syntax: use
except A, B:(no parentheses) for multiple exceptions. Ruff enforces this on Python 3.14.All public functions require type hints. Strict mypy type checking is enforced.
Google-style docstrings are required on all public classes and functions. Ruff enforces this via D rules.
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,
BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement. Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).Use frozen Pydantic models for config/identity. Use separate mutable-via-copy models (using
model_copy(update=...)) for runtime state that evolves (e.g. agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.Use Pydantic v2 conventions:
BaseModel,model_validator,computed_field,ConfigDict. Useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time. Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens). UseNotBlankStrfromcore.typesfor all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.Prefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.Maximum line length is 88 characters. Ruff enforces this.
Functions mus...
Files:
src/synthorg/config/schema.pysrc/synthorg/engine/shutdown_strategies.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/config/schema.pysrc/synthorg/engine/shutdown_strategies.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, or comments. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Vendor names may only appear in: (1) Operations design page (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths/module names, (4) provider presets (src/synthorg/providers/presets.py).
Files:
src/synthorg/config/schema.pysrc/synthorg/engine/shutdown_strategies.py
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: At every phase of planning and implementation, be critical -- actively look for ways to improve the design in the spirit of what we're building (robustness, correctness, simplicity, future-proofing where it's free). Surface improvements as suggestions, not silent changes -- user decides.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Prioritize issues by dependency order, not priority labels -- unblocked dependencies come first.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Commits must follow format: `<type>: <description>`. Valid types: feat, fix, refactor, docs, test, chore, perf, ci. Enforced by commitizen (commit-msg hook). Signed commits are required on `main` via branch protection -- all commits must be GPG/SSH signed.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Use feature branches: `<type>/<slug>` from main. Pre-commit hooks are enforced: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint, golangci-lint + go vet (CLI), no-em-dashes, no-redundant-timeout, eslint-web (web dashboard). Pre-push hooks enforce: mypy type-check + pytest unit tests (affected modules only) + golangci-lint + go vet + go test (CLI) + eslint-web (web dashboard). Foundational module changes (core, config, observability) or conftest changes trigger full runs.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Pre-commit.ci: autoupdate disabled (`autoupdate_schedule: never`) -- Dependabot owns hook version bumps via `pre-commit` ecosystem.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Use `gh issue list` via Bash for GitHub issue queries (not MCP tools) -- MCP `list_issues` has unreliable field data.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Merge strategy: squash merge -- PR body becomes the squash commit message on main. Trailers (e.g. `Release-As`, `Closes `#N``) must be in the PR body to land in the final commit. Preserve existing `Closes `#NNN`` references -- never remove unless explicitly asked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: After finishing an issue implementation, always create a feature branch (`<type>/<slug>`), commit, and push -- do NOT create a PR automatically. Do NOT leave work uncommitted on main -- branch, commit, push immediately after finishing.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: NEVER create a PR directly -- `gh pr create` is blocked by hookify. ALWAYS use `/pre-pr-review` to create PRs -- it runs automated checks + review agents + fixes before creating the PR. For trivial/docs-only changes: `/pre-pr-review quick` skips agents but still runs automated checks. After the PR exists, use `/aurelio-review-pr` to handle external reviewer feedback. The `/commit-push-pr` command is effectively blocked.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all. No deferring, no "out of scope" skipping.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Never use `cd` in Bash commands -- the working directory is already set to the project root. Use absolute paths or run commands directly. Exception: `bash -c "cd <dir> && <cmd>"` is safe (runs in a child process, no cwd side effects). Use this for tools without a `-C` flag.
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:00:44.623Z
Learning: Never use Bash to write or modify files -- use the Write or Edit tools. Do not use `cat >`, `cat << EOF`, `echo >`, `echo >>`, `sed -i`, `python -c "open(...).write(...)"`, or `tee` to create or modify files (read-only/inspection uses like piping to stdout are fine). This applies to all files (plan files, config files, source code) and all subagents.
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.{ts,tsx} : Vitest unit tests must use coverage scoped to files changed vs origin/main branch
Applied to files:
web/src/__tests__/utils/tasks.test.ts
📚 Learning: 2026-04-03T07:50:58.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:50:58.367Z
Learning: Applies to **/*.py : Use Google-style docstrings, required on all public classes and functions (enforced by ruff D rules)
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to **/*.py : Google style docstrings are required on public classes and functions. Enforced by ruff D rules.
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-02T11:49:21.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T11:49:21.202Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`) with `allow_inf_nan=False` in all `ConfigDict` declarations
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to **/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Include `allow_inf_nan=False` in all `ConfigDict` declarations.
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to src/**/*.py : Use Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `allow_inf_nan=False` in all `ConfigDict` declarations to reject `NaN`/`Inf` in numeric fields at validation time
Applied to files:
src/synthorg/config/schema.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
tests/unit/engine/test_shutdown.pysrc/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-02T20:17:33.300Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T20:17:33.300Z
Learning: Applies to tests/**/*.py : 30-second global timeout per test (set in `pyproject.toml`); non-default overrides like `timeout(60)` are allowed
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T09:39:21.584Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:39:21.584Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T15:24:49.467Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T15:24:49.467Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : For tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)` -- it is cancellation-safe
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
src/synthorg/engine/shutdown_strategies.py
🔇 Additional comments (1)
web/src/__tests__/utils/tasks.test.ts (1)
98-104: Status coverage assertion update is solid.This check now correctly validates Kanban coverage against the 10-status domain.
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Callable, Mapping, Sequence | ||
|
|
||
| from synthorg.config.schema import GracefulShutdownConfig |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import ast
from pathlib import Path
path = Path("src/synthorg/engine/shutdown_strategies.py")
tree = ast.parse(path.read_text())
type_checking_imports = set()
runtime_imports = set()
annotation_names = set()
for node in tree.body:
if isinstance(node, ast.If) and isinstance(node.test, ast.Name) and node.test.id == "TYPE_CHECKING":
for stmt in node.body:
if isinstance(stmt, ast.ImportFrom):
for alias in stmt.names:
type_checking_imports.add(alias.asname or alias.name)
elif isinstance(node, ast.ImportFrom):
for alias in node.names:
if alias.name != "TYPE_CHECKING":
runtime_imports.add(alias.asname or alias.name)
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
for arg in (*node.args.args, *node.args.kwonlyargs):
if arg.annotation is not None:
annotation_names.update(
n.id for n in ast.walk(arg.annotation) if isinstance(n, ast.Name)
)
if node.returns is not None:
annotation_names.update(
n.id for n in ast.walk(node.returns) if isinstance(n, ast.Name)
)
problem_names = sorted((annotation_names & type_checking_imports) - runtime_imports)
print(problem_names)
PYRepository: Aureliolo/synthorg
Length of output: 111
🏁 Script executed:
cat -n src/synthorg/engine/shutdown_strategies.py | head -120 | tail -30Repository: Aureliolo/synthorg
Length of output: 1235
🏁 Script executed:
cat -n src/synthorg/engine/shutdown_strategies.py | sed -n '100,110p; 190,200p; 310,320p; 490,500p'Repository: Aureliolo/synthorg
Length of output: 1560
🏁 Script executed:
rg 'get_type_hints|__annotations__|typing\.get_type_hints' src/synthorg/ --type pyRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
python -c "import src.synthorg.engine.shutdown_strategies" 2>&1 || trueRepository: Aureliolo/synthorg
Length of output: 321
🏁 Script executed:
head -20 src/synthorg/engine/shutdown_strategies.pyRepository: Aureliolo/synthorg
Length of output: 638
🏁 Script executed:
grep -r "python_requires\|version" setup.py pyproject.toml 2>/dev/null | head -10Repository: Aureliolo/synthorg
Length of output: 313
🏁 Script executed:
cat pyproject.toml 2>/dev/null | grep -A 5 "\[project\]"Repository: Aureliolo/synthorg
Length of output: 274
🏁 Script executed:
grep -i "3.14\|pep.649\|lazy" README.md CLAUDE.md .github/workflows/*.yml 2>/dev/null | head -20Repository: Aureliolo/synthorg
Length of output: 642
🏁 Script executed:
rg -A 3 -B 3 'get_type_hints|__annotations__|inspect\.(signature|signature\()' src/synthorg/engine/ --type pyRepository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
grep -r "get_type_hints\|__annotations__" src/synthorg --type py | wc -lRepository: Aureliolo/synthorg
Length of output: 198
🏁 Script executed:
cat -n src/synthorg/engine/shutdown_strategies.py | sed -n '1,30p'Repository: Aureliolo/synthorg
Length of output: 1157
Move annotation types out of TYPE_CHECKING for runtime safety and type checker clarity.
Mapping, Sequence, and GracefulShutdownConfig are referenced in public function signatures (lines 108, 196, 314, 497) but are only imported within the TYPE_CHECKING block. While Python 3.14's PEP 649 prevents automatic annotation evaluation at runtime, any explicit runtime annotation introspection (e.g., via get_type_hints()) will fail because these names are not available in module globals. Moving them to regular imports ensures both runtime safety and proper type checker visibility.
🛠️ Minimal fix
-import asyncio
-import time
-from typing import TYPE_CHECKING, Any
-
-if TYPE_CHECKING:
- from collections.abc import Callable, Mapping, Sequence
-
- from synthorg.config.schema import GracefulShutdownConfig
+import asyncio
+import time
+from collections.abc import Callable, Mapping, Sequence
+from typing import Any
+
+from synthorg.config.schema import GracefulShutdownConfig🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/shutdown_strategies.py` around lines 11 - 16, Move the
annotation types out of the TYPE_CHECKING block so runtime introspection works:
replace the conditional imports by adding top-level imports for Mapping and
Sequence (from collections.abc) and for GracefulShutdownConfig (from
synthorg.config.schema), and keep or add Callable/Any as needed; then remove
them from the TYPE_CHECKING block. Update the module imports in
src/synthorg/engine/shutdown_strategies.py so the names Mapping, Sequence,
Callable and GracefulShutdownConfig exist in module globals (this will fix
failures when functions that use these annotations — e.g., the public functions
referencing Mapping/Sequence/GracefulShutdownConfig in this file — are
introspected via get_type_hints or similar).
…al strategy, assignment test)
…mpletion logging, test coverage)
a2903a1 to
aadf566
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/synthorg/engine/shutdown_strategies.py (1)
11-16:⚠️ Potential issue | 🟠 MajorImport public annotation types at runtime, not only under
TYPE_CHECKING.
Mapping,Sequence, andGracefulShutdownConfigare used in public signatures here, so runtime annotation introspection viatyping.get_type_hints()can still fail withNameErrorif those names stay behindif TYPE_CHECKING:; the Python 3.14 docs call outTYPE_CHECKINGimports as a concrete failure mode. (docs.python.org)🛠️ Minimal fix
import asyncio import time -from typing import TYPE_CHECKING, Any +from collections.abc import Mapping, Sequence +from typing import TYPE_CHECKING, Any + +from synthorg.config.schema import GracefulShutdownConfig if TYPE_CHECKING: - from collections.abc import Callable, Mapping, Sequence - - from synthorg.config.schema import GracefulShutdownConfig + from collections.abc import Callable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/shutdown_strategies.py` around lines 11 - 16, The public annotation types Mapping, Sequence and the GracefulShutdownConfig class must be imported at runtime (not only under the TYPE_CHECKING guard) so runtime introspection like typing.get_type_hints() won't raise NameError; move or duplicate the imports for Mapping and Sequence from collections.abc and GracefulShutdownConfig from synthorg.config.schema out of the if TYPE_CHECKING: block (keep any purely type-only imports under TYPE_CHECKING if needed), ensuring the names Mapping, Sequence, and GracefulShutdownConfig are available at import time for functions/methods that reference them in signatures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/engine.md`:
- Around line 1224-1239: The docs currently conflict: the AgentEngine pipeline
claims SHUTDOWN -> INTERRUPTED unconditionally while Strategy 4 introduces
SHUTDOWN -> SUSPENDED when checkpointing succeeds; update the text so the
pipeline and the "SUSPENDED vs INTERRUPTED" note both state that SHUTDOWN
transitions are conditional — on graceful shutdown a successful checkpoint moves
the task to SUSPENDED, a failed or missing checkpoint moves it to INTERRUPTED —
and adjust the AgentEngine pipeline description (and any occurrences of the
unconditional SHUTDOWN -> INTERRUPTED mapping) to reflect this conditional
behavior for Strategy 4 and other shutdown strategies.
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 487-500: The catch-all except block in the checkpoint saving flow
is suppressing critical system exceptions (e.g., MemoryError, RecursionError) —
update the except Exception as exc handler in shutdown_strategies.py (the block
that logs EXECUTION_SHUTDOWN_CHECKPOINT_FAILED for task_id) to detect and
re-raise system-level exceptions (at minimum MemoryError and RecursionError, and
optionally SystemExit/KeyboardInterrupt) before doing the logger.exception and
returning False; implement this by checking isinstance(exc, (MemoryError,
RecursionError, SystemExit, KeyboardInterrupt)) and raising exc if true,
otherwise proceed with the existing logging and return False.
In `@tests/unit/engine/test_shutdown.py`:
- Around line 421-435: Combine the four separate tests into a single
parameterized test using pytest.mark.parametrize to cover the matrix for
tool_timeout_seconds: include cases for default (no arg -> expects 60.0), zero
(0 -> expects ValidationError), upper bound (301 -> expects ValidationError),
and at-max (300 -> expects 300.0); implement a single test function (e.g.,
test_tool_timeout_boundaries) that accepts the input value and expected
outcome/exception and asserts either the produced
GracefulShutdownConfig.tool_timeout_seconds equals the expected float or that
instantiating GracefulShutdownConfig raises ValidationError; reuse the existing
GracefulShutdownConfig and ValidationError symbols and ensure pytest is used for
parameterization.
- Around line 693-706: The test test_stragglers_cancelled_after_timeout relies
on a real 0.1s wall-clock wait; make it deterministic by mocking the time and
sleep boundary used by FinishCurrentToolStrategy.execute_shutdown: patch
asyncio.sleep to a no-op async function that advances a virtual time counter and
patch time.monotonic to return that virtual time (e.g., base + increments when
sleep is called) so the strategy's timeout logic sees time advance instantly;
implement these patches in the test (use pytest monkeypatch or similar) before
creating the long_tool task and invoking execute_shutdown, referencing
FinishCurrentToolStrategy, execute_shutdown, and the long_tool task to keep the
behavior identical without real delays.
In `@web/src/utils/tasks.ts`:
- Line 108: The Terminal column definition (the object with id 'terminal' in the
statuses array) currently lists 'suspended' as a terminal state; remove
'suspended' from that statuses list so suspended tasks are not shown as final.
Keep the existing resumable transition (the mapping that performs 'suspended ->
assigned' at the resumable/transitions logic) intact so suspended tasks remain
resumable and are not displayed in the Terminal column.
---
Duplicate comments:
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 11-16: The public annotation types Mapping, Sequence and the
GracefulShutdownConfig class must be imported at runtime (not only under the
TYPE_CHECKING guard) so runtime introspection like typing.get_type_hints() won't
raise NameError; move or duplicate the imports for Mapping and Sequence from
collections.abc and GracefulShutdownConfig from synthorg.config.schema out of
the if TYPE_CHECKING: block (keep any purely type-only imports under
TYPE_CHECKING if needed), ensuring the names Mapping, Sequence, and
GracefulShutdownConfig are available at import time for functions/methods that
reference them in signatures.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: 68151422-6923-4341-b281-0188e0b16579
📒 Files selected for processing (33)
CLAUDE.mddocs/design/engine.mdsrc/synthorg/config/schema.pysrc/synthorg/core/enums.pysrc/synthorg/core/task_transitions.pysrc/synthorg/engine/__init__.pysrc/synthorg/engine/assignment/service.pysrc/synthorg/engine/decomposition/models.pysrc/synthorg/engine/decomposition/rollup.pysrc/synthorg/engine/shutdown.pysrc/synthorg/engine/shutdown_strategies.pysrc/synthorg/engine/workflow/kanban_columns.pysrc/synthorg/observability/events/execution.pytests/unit/core/test_enums.pytests/unit/core/test_task_transitions.pytests/unit/engine/test_assignment_service.pytests/unit/engine/test_shutdown.pytests/unit/engine/workflow/test_kanban_columns.pyweb/src/__tests__/components/layout/StatusBar.test.tsxweb/src/__tests__/hooks/useDashboardData.test.tsweb/src/__tests__/integration/ws-dashboard.test.tsxweb/src/__tests__/pages/DashboardPage.test.tsxweb/src/__tests__/pages/budget/ThresholdAlerts.test.tsxweb/src/__tests__/stores/analytics.test.tsweb/src/__tests__/utils/dashboard.property.test.tsweb/src/__tests__/utils/dashboard.test.tsweb/src/__tests__/utils/tasks.test.tsweb/src/api/types.tsweb/src/components/ui/task-status-indicator.stories.tsxweb/src/pages/budget/ThresholdAlerts.stories.tsxweb/src/pages/dashboard/DashboardPage.stories.tsxweb/src/utils/constants.tsweb/src/utils/tasks.ts
| except TimeoutError: | ||
| logger.warning( | ||
| EXECUTION_SHUTDOWN_CHECKPOINT_FAILED, | ||
| task_id=task_id, | ||
| reason="checkpoint timed out", | ||
| ) | ||
| return False | ||
| except Exception as exc: | ||
| logger.exception( | ||
| EXECUTION_SHUTDOWN_CHECKPOINT_FAILED, | ||
| task_id=task_id, | ||
| error_type=type(exc).__name__, | ||
| ) | ||
| return False |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/synthorg/engine/shutdown_strategies.py | sed -n '480,510p'Repository: Aureliolo/synthorg
Length of output: 1300
🏁 Script executed:
head -20 src/synthorg/engine/shutdown_strategies.pyRepository: Aureliolo/synthorg
Length of output: 638
🏁 Script executed:
sed -n '1,35p' src/synthorg/engine/shutdown_strategies.pyRepository: Aureliolo/synthorg
Length of output: 1129
🏁 Script executed:
rg "except.*MemoryError|except.*RecursionError" src/synthorgRepository: Aureliolo/synthorg
Length of output: 27046
🏁 Script executed:
rg "except builtins\." src/synthorgRepository: Aureliolo/synthorg
Length of output: 1827
Re-raise system-level exceptions before the catch-all.
The broad except Exception as exc: catches MemoryError and RecursionError from checkpoint_saver and treats them as normal checkpoint failures, returning False instead of propagating. These critical system errors should never be suppressed.
🛠️ Minimal fix
+import builtins
import asyncio
import time
from typing import TYPE_CHECKING, Any except TimeoutError:
logger.warning(
EXECUTION_SHUTDOWN_CHECKPOINT_FAILED,
task_id=task_id,
reason="checkpoint timed out",
)
return False
+ except builtins.MemoryError, RecursionError:
+ raise
except Exception as exc:
logger.exception(
EXECUTION_SHUTDOWN_CHECKPOINT_FAILED,
task_id=task_id,
error_type=type(exc).__name__,
)
return False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/engine/shutdown_strategies.py` around lines 487 - 500, The
catch-all except block in the checkpoint saving flow is suppressing critical
system exceptions (e.g., MemoryError, RecursionError) — update the except
Exception as exc handler in shutdown_strategies.py (the block that logs
EXECUTION_SHUTDOWN_CHECKPOINT_FAILED for task_id) to detect and re-raise
system-level exceptions (at minimum MemoryError and RecursionError, and
optionally SystemExit/KeyboardInterrupt) before doing the logger.exception and
returning False; implement this by checking isinstance(exc, (MemoryError,
RecursionError, SystemExit, KeyboardInterrupt)) and raising exc if true,
otherwise proceed with the existing logging and return False.
| async def test_stragglers_cancelled_after_timeout(self) -> None: | ||
| strategy = FinishCurrentToolStrategy(tool_timeout_seconds=0.1) | ||
|
|
||
| async def long_tool() -> None: | ||
| await asyncio.Event().wait() | ||
|
|
||
| task = asyncio.create_task(long_tool()) | ||
| result = await strategy.execute_shutdown( | ||
| running_tasks={"t1": task}, | ||
| cleanup_callbacks=[], | ||
| ) | ||
|
|
||
| assert result.tasks_completed == 0 | ||
| assert result.tasks_interrupted == 1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make the timeout-path test deterministic.
This case now depends on a real 0.1s wall-clock timeout, which is scheduler-sensitive and slows the suite. Prefer patching the timeout/wait boundary instead of waiting for real time.
As per coding guidelines, "tests/**/*.py: For timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/engine/test_shutdown.py` around lines 693 - 706, The test
test_stragglers_cancelled_after_timeout relies on a real 0.1s wall-clock wait;
make it deterministic by mocking the time and sleep boundary used by
FinishCurrentToolStrategy.execute_shutdown: patch asyncio.sleep to a no-op async
function that advances a virtual time counter and patch time.monotonic to return
that virtual time (e.g., base + increments when sleep is called) so the
strategy's timeout logic sees time advance instantly; implement these patches in
the test (use pytest monkeypatch or similar) before creating the long_tool task
and invoking execute_shutdown, referencing FinishCurrentToolStrategy,
execute_shutdown, and the long_tool task to keep the behavior identical without
real delays.
…f-board suspended, test parametrize)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/design/engine.md (1)
1162-1168:⚠️ Potential issue | 🟡 MinorKeep the Strategy 1 YAML shape aligned with the implemented config.
GracefulShutdownConfigandbuild_shutdown_strategy()use flatgrace_seconds/cleanup_secondsfields, but this example nests them undercooperative_timeout:. Copy-pasting the docs currently describes a different config shape than the code and tests in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/engine.md` around lines 1162 - 1168, The example YAML nests grace_seconds and cleanup_seconds under cooperative_timeout, but the implemented config and build_shutdown_strategy expect a flat shape; update the docs so graceful_shutdown uses strategy: "cooperative_timeout" and places grace_seconds and cleanup_seconds at the same level (e.g., graceful_shutdown.grace_seconds and graceful_shutdown.cleanup_seconds) to match GracefulShutdownConfig and the build_shutdown_strategy logic.
♻️ Duplicate comments (3)
tests/unit/engine/test_shutdown.py (1)
705-718:⚠️ Potential issue | 🟡 MinorMake the timeout-path tests deterministic.
These new cases still rely on real
0.1sgrace/tool timeouts viaasyncio.wait(..., timeout=...), which slows the suite and makes CI scheduler-sensitive. Please stub the wait boundary (or otherwise virtualize loop time) so the timeout path resolves immediately instead of waiting on real wall-clock time.Based on learnings, timing-sensitive tests should be made deterministic instead of relying on real timeouts.
Also applies to: 803-904
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/engine/test_shutdown.py` around lines 705 - 718, The test relies on real wall-clock time via FinishCurrentToolStrategy(tool_timeout_seconds=0.1) triggering asyncio.wait(...) timeouts in execute_shutdown, making it flaky; stub or monkeypatch asyncio.wait (or the specific wait boundary used by FinishCurrentToolStrategy.execute_shutdown) in test_stragglers_cancelled_after_timeout (and the similar cases around 803-904) to immediately return the timed-out result instead of sleeping, so the timeout path resolves deterministically; ensure the stub preserves the returned (done, pending) tuple shape that execute_shutdown expects and restore the original asyncio.wait after the test.src/synthorg/engine/shutdown_strategies.py (2)
86-89:⚠️ Potential issue | 🟡 MinorLog invalid shutdown config before raising.
These branches still raise
ValueErrorwithout any WARNING/ERROR context, so bad shutdown config disappears unless the caller logs it upstream. Please emit a structured validation event before each raise.As per coding guidelines, "All error paths must log at WARNING or ERROR with context before raising."
Also applies to: 171-176, 299-304, 558-564
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/shutdown_strategies.py` around lines 86 - 89, Add a WARNING/ERROR log call with structured context immediately before each ValueError raise in the shutdown strategies constructors/validators (e.g., the __init__ that checks cleanup_seconds, and the other checks at the blocks around lines shown). For each check (function/method names: __init__ and any validation helper used in this module), call the module logger (e.g., logger.warning or logger.error) with a clear message and key context like {"param": "cleanup_seconds", "value": cleanup_seconds, "reason": "must be positive"} (or analogous keys for the other checks) and then raise the existing ValueError unchanged; ensure the log appears just before the raise so the error path is always recorded.
11-16:⚠️ Potential issue | 🟠 MajorImport annotation types at runtime.
Mapping,Sequence, andGracefulShutdownConfigare referenced in public signatures but only exist insideTYPE_CHECKING. Any runtime annotation introspection on these APIs will fail once those annotations are evaluated.Read-only verification script
#!/bin/bash python - <<'PY' import ast from pathlib import Path path = Path("src/synthorg/engine/shutdown_strategies.py") tree = ast.parse(path.read_text()) type_checking_imports = set() runtime_imports = set() annotation_names = set() for node in tree.body: if isinstance(node, ast.If) and isinstance(node.test, ast.Name) and node.test.id == "TYPE_CHECKING": for stmt in node.body: if isinstance(stmt, ast.ImportFrom): for alias in stmt.names: type_checking_imports.add(alias.asname or alias.name) elif isinstance(node, ast.ImportFrom): for alias in node.names: runtime_imports.add(alias.asname or alias.name) for node in ast.walk(tree): if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): annotations = [node.returns] annotations.extend( arg.annotation for arg in ( *node.args.posonlyargs, *node.args.args, *node.args.kwonlyargs, ) ) for ann in annotations: if ann is not None: annotation_names.update( n.id for n in ast.walk(ann) if isinstance(n, ast.Name) ) print(sorted((annotation_names & type_checking_imports) - runtime_imports)) PYExpected result:
Mapping,Sequence, andGracefulShutdownConfigshow up as annotation-only names that are not present in runtime imports.Also applies to: 107-112, 195-200, 322-327, 520-524
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/shutdown_strategies.py` around lines 11 - 16, The module currently places Mapping, Sequence, and GracefulShutdownConfig inside the TYPE_CHECKING block so runtime annotation introspection fails; move the needed annotation imports out of the TYPE_CHECKING guard so they are available at runtime (e.g. import Mapping, Sequence, and Callable from collections.abc and import GracefulShutdownConfig from synthorg.config.schema at module top-level), or alternatively enable from __future__ import annotations if you prefer postponed evaluation, and apply the same change for the other referenced blocks (the occurrences around the other ranges noted) so signatures using Mapping, Sequence, Callable, and GracefulShutdownConfig resolve at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/utils/tasks.ts`:
- Around line 111-118: STATUS_TO_COLUMN currently omits off-board statuses
(e.g., 'suspended') causing groupTasksByColumn to drop them; change the mapping
and consumer contract so off-board statuses are explicit: update
STATUS_TO_COLUMN (and any typing) to map TaskStatus to KanbanColumnId | null (or
create a separate EXCLUDED_STATUS_BUCKET) and ensure creation from
KANBAN_COLUMNS assigns null for OFF_BOARD_STATUSES (the OFF_BOARD_STATUSES
constant, STATUS_TO_COLUMN, and the groupTasksByColumn function should be
updated), and make TaskBoardPage consume the new mapping/returned excluded
bucket so it can account for and render counts for excluded/off-board tasks
instead of silently dropping them.
---
Outside diff comments:
In `@docs/design/engine.md`:
- Around line 1162-1168: The example YAML nests grace_seconds and
cleanup_seconds under cooperative_timeout, but the implemented config and
build_shutdown_strategy expect a flat shape; update the docs so
graceful_shutdown uses strategy: "cooperative_timeout" and places grace_seconds
and cleanup_seconds at the same level (e.g., graceful_shutdown.grace_seconds and
graceful_shutdown.cleanup_seconds) to match GracefulShutdownConfig and the
build_shutdown_strategy logic.
---
Duplicate comments:
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 86-89: Add a WARNING/ERROR log call with structured context
immediately before each ValueError raise in the shutdown strategies
constructors/validators (e.g., the __init__ that checks cleanup_seconds, and the
other checks at the blocks around lines shown). For each check (function/method
names: __init__ and any validation helper used in this module), call the module
logger (e.g., logger.warning or logger.error) with a clear message and key
context like {"param": "cleanup_seconds", "value": cleanup_seconds, "reason":
"must be positive"} (or analogous keys for the other checks) and then raise the
existing ValueError unchanged; ensure the log appears just before the raise so
the error path is always recorded.
- Around line 11-16: The module currently places Mapping, Sequence, and
GracefulShutdownConfig inside the TYPE_CHECKING block so runtime annotation
introspection fails; move the needed annotation imports out of the TYPE_CHECKING
guard so they are available at runtime (e.g. import Mapping, Sequence, and
Callable from collections.abc and import GracefulShutdownConfig from
synthorg.config.schema at module top-level), or alternatively enable from
__future__ import annotations if you prefer postponed evaluation, and apply the
same change for the other referenced blocks (the occurrences around the other
ranges noted) so signatures using Mapping, Sequence, Callable, and
GracefulShutdownConfig resolve at runtime.
In `@tests/unit/engine/test_shutdown.py`:
- Around line 705-718: The test relies on real wall-clock time via
FinishCurrentToolStrategy(tool_timeout_seconds=0.1) triggering asyncio.wait(...)
timeouts in execute_shutdown, making it flaky; stub or monkeypatch asyncio.wait
(or the specific wait boundary used by
FinishCurrentToolStrategy.execute_shutdown) in
test_stragglers_cancelled_after_timeout (and the similar cases around 803-904)
to immediately return the timed-out result instead of sleeping, so the timeout
path resolves deterministically; ensure the stub preserves the returned (done,
pending) tuple shape that execute_shutdown expects and restore the original
asyncio.wait after the test.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c43ae08-a2d0-499b-a70c-386001955b66
📒 Files selected for processing (5)
docs/design/engine.mdsrc/synthorg/engine/shutdown_strategies.pytests/unit/engine/test_shutdown.pyweb/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.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). (5)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/logger-- never bareconsole.warn/console.error/console.debugin application code
Logger variable name must always beconst log(e.g.const log = createLogger('module-name'))
Pass dynamic/untrusted values as separate arguments to logger methods (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
Files:
web/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-*)) for colors; NEVER hardcode hex values in.tsx/.tsfiles
Usefont-sansorfont-mono(Geist tokens) for typography; NEVER setfontFamilydirectly in.tsx/.tsfiles
Use density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing in components
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders; NEVER hardcode values in.tsx/.tsfiles
Use@/lib/motionpresets for Framer Motion transition durations; NEVER hardcode transition durations
CSS side-effect imports in TypeScript 6 require type declarations -- add/// <reference types="vite/client" />at the top of files with CSS importsNever hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and
@/lib/motionpresetsUse React 19 with design system components; see
web/CLAUDE.mdfor component inventory and design token rulesWeb dashboard code is validated by PostToolUse hook (
scripts/check_web_design_system.py) on every Edit/Write toweb/src/
Files:
web/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from
web/src/components/ui/before creating new ones
Files:
web/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.ts
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue, as the design spec is the starting point for architecture, data models, and behaviorDo not use
from __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotationsUse
except A, B:syntax (no parentheses) instead ofexcept (A, B):-- PEP 758 except syntax enforced by ruff on Python 3.14All public functions must have type hints and Google-style docstrings (enforced by ruff D rules and mypy strict mode)
Create new objects for immutability, never mutate existing ones; for non-Pydantic internal collections, use
copy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcementUse frozen Pydantic models for config/identity; use separate mutable-via-copy models with
model_copy(update=...)for runtime state that evolvesUse Pydantic v2 (
BaseModel,model_validator,computed_field,ConfigDict); useallow_inf_nan=Falsein allConfigDictdeclarations; use@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStrfromcore.typesfor all identifier/name fieldsPrefer
asyncio.TaskGroupfor fan-out/fan-in parallel operations in new code; prefer structured concurrency over barecreate_taskFunctions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
All provider calls must go through
BaseCompletionProviderwhich applies retry and rate limiting automatically -- never implement retry logic in driver subclasses or calling codeRetryConfig and RateLimiterConfig are set per-provider in
ProviderConfig; retryable errors (RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError) are auto-retried;RetryExhaustedErrorsignals that all retries failedLine length must be 88 characters (enforced by ...
Files:
src/synthorg/engine/shutdown_strategies.pytests/unit/engine/test_shutdown.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Every module with business logic must include:
from synthorg.observability import get_loggerthenlogger = get_logger(__name__)Never use
import logging,logging.getLogger(), orprint()in application code (exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py,observability/http_handler.py, andobservability/otlp_handler.pymay use stdlibloggingfor bootstrap)Use event constants from domain-specific modules under
synthorg.observability.events(import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT)Always use structured logging:
logger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Files:
src/synthorg/engine/shutdown_strategies.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/engine/shutdown_strategies.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Mark all tests with
@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowMaintain 80% minimum code coverage (enforced in CI)
Use
asyncio_mode = "auto"in pytest configuration -- no manual@pytest.mark.asyncioneeded on individual testsAlways run pytest with
-n 8for parallelism locally; never run tests sequentiallyPrefer
@pytest.mark.parametrizefor testing similar casesNever use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests or project-owned code; use generic names:
example-provider,test-provider,example-large-001,test-small-001, etc.For timing-sensitive tests, mock
time.monotonic()andasyncio.sleep()to make them deterministic; for tasks that must block indefinitely until cancelled, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Files:
tests/unit/engine/test_shutdown.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_shutdown.py
🧠 Learnings (44)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: At every phase of planning and implementation, be critical and actively look for ways to improve the design; surface improvements as suggestions, not silent changes
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Prioritize issues by dependency order, not priority labels -- unblocked dependencies come first
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: If implementation deviates from the design spec (better approach found, scope evolved, etc.), alert the user and explain why -- user decides whether to proceed or update the spec; do NOT silently diverge
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: When a spec topic is referenced (e.g. 'the Agents page' or 'the Engine page's Crash Recovery section'), read the relevant `docs/design/` page before coding
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Never use `cd` in Bash commands -- the working directory is already set to the project root; use absolute paths or run commands directly
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Never use Bash to write or modify files -- use the Write or Edit tools instead; do not use `cat >`, `cat << EOF`, `echo >`, `echo >>`, `sed -i`, or `tee` for file creation/modification
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Use conventional commits: `<type>: <description>` -- types: feat, fix, refactor, docs, test, chore, perf, ci (enforced by commitizen commit-msg hook)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: All commits on `main` must be GPG/SSH signed (enforced by branch protection)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Use branch names: `<type>/<slug>` from main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Pre-commit hooks enforce: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint, golangci-lint + go vet (CLI), no-em-dashes, no-redundant-timeout, eslint-web (web dashboard)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Pre-push hooks run: mypy type-check (affected modules only) + pytest unit tests (affected modules only) + golangci-lint + go vet + go test (CLI) + eslint-web (web dashboard); skipped in pre-commit.ci but run in dedicated CI jobs
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: GitHub issue queries: use `gh issue list` via Bash (not MCP tools) -- MCP `list_issues` has unreliable field data
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Merge strategy: squash merge -- PR body becomes the squash commit message on main; trailers (e.g. `Release-As`, `Closes `#N``) must be in the PR body to land in the final commit
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Preserve existing `Closes `#NNN`` references in PR issue references -- never remove unless explicitly asked
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: After finishing an issue implementation: always create a feature branch (`<type>/<slug>`), commit, and push -- do NOT create a PR automatically; do NOT leave work uncommitted on main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: Never create a PR directly with `gh pr create` (blocked by hookify) -- always use `/pre-pr-review` to create PRs; for trivial/docs-only changes use `/pre-pr-review quick` to skip agents but still run automated checks
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: After the PR exists, use `/aurelio-review-pr` to handle external reviewer feedback
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T19:48:32.279Z
Learning: When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all -- never skip, no deferring, no 'out of scope' skipping
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.{ts,tsx} : Vitest unit tests must use coverage scoped to files changed vs origin/main branch
Applied to files:
web/src/__tests__/utils/tasks.test.ts
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
docs/design/engine.mdsrc/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/engine/shutdown_strategies.pytests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T09:39:21.584Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:39:21.584Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : For tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)` -- it is cancellation-safe
Applied to files:
src/synthorg/engine/shutdown_strategies.pytests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-31T14:18:02.890Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:18:02.890Z
Learning: Applies to src/**/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Do NOT use `from __future__ import annotations` in Python code—Python 3.14 has PEP 649 native lazy annotations
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T09:09:43.948Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:09:43.948Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to src/**/*.py : All error paths in Python code must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to src/**/*.py : `RetryExhaustedError` in Python signals all retries failed—engine layer catches this to trigger fallback chains
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T17:49:14.133Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T17:49:14.133Z
Learning: Applies to src/synthorg/{providers,engine}/**/*.py : Retryable errors are `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`; non-retryable errors raise immediately; `RetryExhaustedError` signals all retries failed
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Retryable errors (`is_retryable=True`): `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`. Non-retryable errors raise immediately without retry.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-02T21:38:30.127Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T21:38:30.127Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-08T19:05:17.824Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.824Z
Learning: Python tests have a 30-second global timeout per test (in `pyproject.toml`)—do not add per-file `pytest.mark.timeout(30)` markers; non-default overrides like `timeout(60)` are allowed
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-02T20:17:33.300Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T20:17:33.300Z
Learning: Applies to tests/**/*.py : 30-second global timeout per test (set in `pyproject.toml`); non-default overrides like `timeout(60)` are allowed
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-16T07:02:24.531Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:02:24.531Z
Learning: Applies to tests/**/*.py : Test timeout: 30 seconds per test.
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Parametrize: Prefer pytest.mark.parametrize for testing similar cases.
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases.
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-31T20:29:10.177Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:29:10.177Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases in Python tests
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; mock `time.monotonic()` and `asyncio.sleep()` for determinism; use `asyncio.Event().wait()` for indefinite blocking instead of `asyncio.sleep(large_number)`
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to tests/**/*.py : Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mocking `time.monotonic()` and `asyncio.sleep()` for determinism or using `asyncio.Event().wait()` for indefinite blocking
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : Flaky tests: NEVER skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/engine/test_shutdown.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky Python tests—always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` instead of widening timing margins. Use `asyncio.Event().wait()` for indefinite blocking instead of `asyncio.sleep(large_number)`
Applied to files:
tests/unit/engine/test_shutdown.py
🪛 markdownlint-cli2 (0.22.0)
docs/design/engine.md
[warning] 54-54: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
… YAML config, validation logging)
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/synthorg/engine/shutdown_strategies.py (1)
588-594:⚠️ Potential issue | 🟡 MinorLog before raising
ValueErrorfor unknown strategy.The factory raises
ValueErrorwithout a preceding WARNING/ERROR log. As per coding guidelines, "All error paths must log at WARNING or ERROR with context before raising."🛠️ Proposed fix
builder = strategies.get(config.strategy) if builder is None: msg = ( f"Unknown shutdown strategy: {config.strategy!r}. " f"Known strategies: {sorted(strategies)}" ) + logger.warning( + EXECUTION_SHUTDOWN_TASK_ERROR, + error=msg, + strategy=config.strategy, + ) raise ValueError(msg)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/engine/shutdown_strategies.py` around lines 588 - 594, Before raising the ValueError for an unknown shutdown strategy in shutdown_strategies.py, log the error with context: call the module logger (e.g., logger.warning or logger.error) immediately after builder = strategies.get(config.strategy) when builder is None, including config.strategy and the list of known strategies (sorted(strategies)) in the message; then raise the existing ValueError(msg) as before so the condition is both logged and raised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/synthorg/engine/shutdown_strategies.py`:
- Around line 588-594: Before raising the ValueError for an unknown shutdown
strategy in shutdown_strategies.py, log the error with context: call the module
logger (e.g., logger.warning or logger.error) immediately after builder =
strategies.get(config.strategy) when builder is None, including config.strategy
and the list of known strategies (sorted(strategies)) in the message; then raise
the existing ValueError(msg) as before so the condition is both logged and
raised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 95cb450a-2347-4680-8d66-890ab9bdd425
📒 Files selected for processing (4)
docs/design/engine.mdsrc/synthorg/engine/shutdown_strategies.pyweb/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.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). (5)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/logger-- never bareconsole.warn/console.error/console.debugin application code
Logger variable name must always beconst log(e.g.const log = createLogger('module-name'))
Pass dynamic/untrusted values as separate arguments to logger methods (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
Files:
web/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Use Tailwind semantic classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-*)) for colors; NEVER hardcode hex values in.tsx/.tsfiles
Usefont-sansorfont-mono(Geist tokens) for typography; NEVER setfontFamilydirectly in.tsx/.tsfiles
Use density-aware tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing; NEVER hardcode pixel values for layout spacing in components
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders; NEVER hardcode values in.tsx/.tsfiles
Use@/lib/motionpresets for Framer Motion transition durations; NEVER hardcode transition durations
CSS side-effect imports in TypeScript 6 require type declarations -- add/// <reference types="vite/client" />at the top of files with CSS importsALWAYS reuse existing components from
web/src/components/ui/before creating new onesNEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and
@/lib/motionpresets
Files:
web/src/__tests__/utils/tasks.test.tsweb/src/utils/tasks.ts
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update the relevant
docs/design/page to reflect any approved deviations from the design spec
Files:
docs/design/engine.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always read the relevant
docs/design/page before implementing any feature or planning any issue -- the DESIGN_SPEC.md is a pointer file linking to 12 design pages that serve as the starting point for architecture, data models, and behaviorIf implementation deviates from the design spec (better approach found, scope evolved, etc.), alert the user and explain why before proceeding -- every deviation needs explicit user approval
When approved deviations occur, update the relevant
docs/design/page to reflect the new realityDo not use
from __future__ import annotations-- Python 3.14 has PEP 649 native lazy annotationsUse
except A, B:syntax (no parentheses) for exception handling in Python 3.14 per PEP 758All public functions must have type hints; mypy strict mode is required
Use Google style docstrings on all public classes and functions -- required and enforced by ruff D rules
Create new objects instead of mutating existing ones (immutability principle); for non-Pydantic internal collections use
copy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcementFor
dict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using
model_copy(update=...)) for runtime state that evolves -- never mix static config fields with mutable runtime fields in one modelUse Pydantic v2 with
BaseModel,model_validator,computed_field, andConfigDict; useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Infin numeric fields at validation timeUse
@computed_fieldfor derived values instead of storing and validating redundant fields (e.g.,TokenUsage.total_tokens)Use
NotBlankStrfromcore.typesfor all identifier/name fields --...
Files:
src/synthorg/engine/shutdown_strategies.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Every module with business logic MUST have
from synthorg.observability import get_loggerfollowed bylogger = get_logger(__name__)-- the logger variable must always be namedloggerNever use
import logging/logging.getLogger()/print()in application code; exceptions areobservability/setup.py,observability/sinks.py,observability/syslog_handler.py,observability/http_handler.py, andobservability/otlp_handler.pywhich may use stdlibloggingandprint(..., file=sys.stderr)for handler construction and bootstrapAlways use constants from domain-specific modules under
synthorg.observability.eventsfor event names (e.g.,API_REQUEST_STARTEDfromevents.api,TOOL_INVOKE_STARTfromevents.tool) -- import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANTUse structured logging with
logger.info(EVENT, key=value)format -- never uselogger.info('msg %s', val)All error paths must log at WARNING or ERROR level with context before raising an exception
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Files:
src/synthorg/engine/shutdown_strategies.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/engine/shutdown_strategies.py
🧠 Learnings (30)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: All provider calls go through `BaseCompletionProvider` which applies retry and rate limiting automatically -- never implement retry logic in driver subclasses or calling code
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: `RetryConfig` and `RateLimiterConfig` are set per-provider in `ProviderConfig` -- retryable errors (`RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`) are retried automatically, non-retryable errors raise immediately, `RetryExhaustedError` signals all retries failed for engine-layer fallback chains
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: Every implementation plan must be presented to the user for accept/deny before coding starts
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: At every phase of planning and implementation, be critical and actively look for ways to improve the design in the spirit of robustness, correctness, simplicity, and future-proofing -- surface improvements as suggestions, not silent changes
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: Prioritize issues by dependency order (unblocked dependencies first), not by priority labels
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: After finishing an issue implementation, always create a feature branch (`<type>/<slug>`), commit, and push -- do NOT create a PR automatically; do NOT leave work uncommitted on main
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: NEVER create a PR directly -- `gh pr create` is blocked by hookify; ALWAYS use `/pre-pr-review` to create PRs (or `/pre-pr-review quick` for trivial/docs-only changes)
Learnt from: CR
URL:
File: CLAUDE.md:undefined-undefined
Timestamp: 2026-04-08T20:08:35.347Z
Learning: When review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes), fix them all -- no deferring, no 'out of scope' skipping
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.{ts,tsx} : Vitest unit tests must use coverage scoped to files changed vs origin/main branch
Applied to files:
web/src/__tests__/utils/tasks.test.ts
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
docs/design/engine.mdsrc/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T09:39:21.584Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:39:21.584Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T18:52:05.142Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T18:52:05.142Z
Learning: Applies to **/*.py : Async concurrency: prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to tests/**/*.py : For tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use `asyncio.Event().wait()` instead of `asyncio.sleep(large_number)` -- it is cancellation-safe
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-31T14:18:02.890Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:18:02.890Z
Learning: Applies to src/**/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-21T11:08:01.542Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T11:08:01.542Z
Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to **/*.py : No `from __future__ import annotations` — Python 3.14 has PEP 649 native lazy annotations
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Do NOT use `from __future__ import annotations` in Python code—Python 3.14 has PEP 649 native lazy annotations
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T09:09:43.948Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T09:09:43.948Z
Learning: Applies to **/*.py : Use `copy.deepcopy()` at construction and `MappingProxyType` wrapping for read-only enforcement in non-Pydantic internal collections (registries, BaseTool)
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to src/**/*.py : All error paths in Python code must log at WARNING or ERROR with context before raising
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-08T19:05:17.823Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-08T19:05:17.823Z
Learning: Applies to src/**/*.py : `RetryExhaustedError` in Python signals all retries failed—engine layer catches this to trigger fallback chains
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-01T17:49:14.133Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T17:49:14.133Z
Learning: Applies to src/synthorg/{providers,engine}/**/*.py : Retryable errors are `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`; non-retryable errors raise immediately; `RetryExhaustedError` signals all retries failed
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Retryable errors (`is_retryable=True`): `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`. Non-retryable errors raise immediately without retry.
Applied to files:
src/synthorg/engine/shutdown_strategies.py
📚 Learning: 2026-04-02T21:38:30.127Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T21:38:30.127Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/engine/shutdown_strategies.py
🪛 markdownlint-cli2 (0.22.0)
docs/design/engine.md
[warning] 54-54: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (13)
web/src/utils/tasks.ts (2)
111-121: LGTM! Off-board status handling is now explicit.The
OFF_BOARD_STATUSESset and explicitnullmapping inSTATUS_TO_COLUMNaddress the previous concern about implicit undefined behavior. Tasks withsuspendedstatus are now intentionally excluded from the Kanban board while remaining discoverable through the explicit mapping contract.
201-211: LGTM! Transition rules align with the backend.The
VALID_TRANSITIONScorrectly implement the SUSPENDED lifecycle:
assigned→suspended✓in_progress→suspended✓suspended→assigned(resume) ✓This matches the backend
VALID_TRANSITIONSinsrc/synthorg/core/task_transitions.py.web/src/__tests__/utils/tasks.test.ts (3)
25-28: LGTM! Comprehensive test coverage forsuspendedstatus.The test suite now properly covers the
suspendedstatus:
- Added to
ALL_STATUSESarraygetTaskStatusColormaps to'warning'getTaskStatusLabelmaps to'Suspended'This addresses the previous review feedback.
Also applies to: 42-42, 61-61
101-108: LGTM! Off-board status assertions are well-structured.The tests correctly validate:
KANBAN_COLUMNScovers only on-board statuses (filtering outOFF_BOARD_STATUSES)STATUS_TO_COLUMNreturnsnullfor off-board statuses and defined values for on-boardAlso applies to: 119-127
175-183: LGTM! Task count assertion correctly excludes off-board statuses.The
preserves all on-board taskstest now accurately expects the grouped total to equal only on-board statuses, validating thatgroupTasksByColumnintentionally excludes suspended tasks.src/synthorg/engine/shutdown_strategies.py (4)
11-16: TYPE_CHECKING imports are acceptable with Python 3.14's PEP 649.The types
Mapping,Sequence, andGracefulShutdownConfigare used only in function signatures, not at runtime. With PEP 649's native lazy annotation evaluation, these types are not resolved at function definition time, so keeping them underTYPE_CHECKINGis safe unless explicit runtime introspection (e.g.,get_type_hints()) is performed elsewhere.If runtime introspection is needed in the future, these should be moved to regular imports. As per coding guidelines, "No
from __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations."
45-72: LGTM! Cooperative exit counting now tracks errored tasks.The
_count_cooperative_exitshelper correctly returns both completed and errored counts, addressing the previous concern about errored tasks being dropped fromShutdownResulttotals.
469-490: LGTM! Concurrent checkpointing withasyncio.TaskGroup.The
_checkpoint_and_cancel_pendingmethod now fans out checkpoint attempts concurrently usingasyncio.TaskGroup, addressing the previous O(n × 30s) serialization concern. Based on learnings, "Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code."
524-525: LGTM! System-level exceptions are properly re-raised.The
except MemoryError, RecursionError: raisepattern correctly propagates critical system errors before the catch-all handler, preventing them from being masked as checkpoint failures. This follows the PEP 758 syntax per coding guidelines.docs/design/engine.md (4)
28-34: LGTM! State diagram accurately reflects SUSPENDED transitions.The Mermaid diagram correctly shows:
ASSIGNED --> SUSPENDED : checkpoint shutdownIN_PROGRESS --> SUSPENDED : checkpoint shutdownSUSPENDED --> ASSIGNED : resume from checkpointThis aligns with the
VALID_TRANSITIONSinsrc/synthorg/core/task_transitions.py.Also applies to: 45-46
1157-1160: LGTM! Conditional INTERRUPTED/SUSPENDED behavior is now clarified.The documentation correctly explains that Strategy 1 (cooperative timeout) marks all terminated tasks as
INTERRUPTED, while Strategy 4 (checkpoint) usesSUSPENDEDfor successfully checkpointed tasks. This addresses the previous inconsistency concern.
1237-1241: LGTM! SUSPENDED vs INTERRUPTED distinction is well-documented.The callout clearly explains the semantic difference:
SUSPENDED: checkpointed, can resume from exact interruption pointINTERRUPTED: stopped without checkpoint, requires full reassignmentBoth are correctly marked as non-terminal with
→ ASSIGNEDtransitions.
1199-1248: LGTM! Shutdown strategies 2-4 are comprehensively documented.The documentation covers:
- Strategy 2 (immediate): force-cancel, mark INTERRUPTED
- Strategy 3 (finish_tool): per-tool timeout, mark INTERRUPTED
- Strategy 4 (checkpoint): cooperative checkpointing, SUSPENDED on success, INTERRUPTED on failure
YAML examples are properly formatted and align with
GracefulShutdownConfigschema.
🤖 I have created a release *beep* *boop* --- ## [0.6.5](v0.6.4...v0.6.5) (2026-04-09) ### Features * add control-plane API endpoints batch ([#1118](#1118), [#1119](#1119), [#1120](#1120), [#1121](#1121)) ([#1138](#1138)) ([af11f0a](af11f0a)) * engine intelligence v2 -- trace enrichment, compaction, versioning eval ([#1139](#1139)) ([ed57dfa](ed57dfa)), closes [#1123](#1123) [#1125](#1125) [#1113](#1113) * generalize versioning to VersionSnapshot[T] for all entity types ([#1155](#1155)) ([5f563ce](5f563ce)), closes [#1131](#1131) [#1132](#1132) [#1133](#1133) * implement auxiliary tool categories -- design, communication, analytics ([#1152](#1152)) ([b506ba4](b506ba4)) * implement multi-project support -- engine orchestration ([#242](#242)) ([#1153](#1153)) ([74f1362](74f1362)) * implement SharedKnowledgeStore append-only + MVCC consistency model (Phase 1.5) ([#1134](#1134)) ([965d3a1](965d3a1)), closes [#1130](#1130) * implement shutdown strategies and SUSPENDED task status ([#1151](#1151)) ([6a0db11](6a0db11)) * persistent cost aggregation for project-lifetime budgets ([#1173](#1173)) ([5c212c5](5c212c5)), closes [#1156](#1156) * Prometheus /metrics endpoint and OTLP exporter ([#1122](#1122)) ([#1135](#1135)) ([aaeaae9](aaeaae9)), closes [#1124](#1124) * Prometheus metrics -- daily budget %, per-agent cost, per-agent budget % ([#1154](#1154)) ([581c494](581c494)), closes [#1148](#1148) ### Bug Fixes * communication hardening -- meeting cooldown, circuit breaker backoff, debate fallback ([#1140](#1140)) ([fe82894](fe82894)), closes [#1115](#1115) [#1116](#1116) [#1117](#1117) ### CI/CD * bump wrangler from 4.80.0 to 4.81.0 in /.github in the all group ([#1144](#1144)) ([b7c0945](b7c0945)) ### Maintenance * bump python from `6869258` to `5e59aae` in /docker/backend in the all group ([#1141](#1141)) ([01e99c2](01e99c2)) * bump python from `6869258` to `5e59aae` in /docker/sandbox in the all group ([#1143](#1143)) ([ea755bd](ea755bd)) * bump python from `6869258` to `5e59aae` in /docker/web in the all group ([#1142](#1142)) ([5416dd9](5416dd9)) * bump the all group across 1 directory with 2 updates ([#1181](#1181)) ([d3d5adf](d3d5adf)) * bump the all group across 1 directory with 3 updates ([#1146](#1146)) ([c609e6c](c609e6c)) * bump the all group in /cli with 2 updates ([#1177](#1177)) ([afd9cde](afd9cde)) * bump the all group in /site with 3 updates ([#1178](#1178)) ([7cff82a](7cff82a)) * bump the all group with 2 updates ([#1180](#1180)) ([199a1a8](199a1a8)) * bump vitest from 4.1.2 to 4.1.3 in /site in the all group ([#1145](#1145)) ([a8c1194](a8c1194)) * consolidated web deps (11 packages + hono security + test fixes) ([#1150](#1150)) ([63a9390](63a9390)), closes [#1147](#1147) [#1136](#1136) [#1137](#1137) * pin Docker Python base image to 3.14.x ([#1182](#1182)) ([8ffdd86](8ffdd86)) --- 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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Implements the three remaining shutdown strategies from the Engine design spec (section 6.7) and adds the
SUSPENDEDtask status for checkpoint-based shutdown.Closes #203
Closes #202
Changes
SUSPENDED task status (#203)
SUSPENDED = "suspended"toTaskStatusenum (10 members now)ASSIGNED -> SUSPENDED,IN_PROGRESS -> SUSPENDED,SUSPENDED -> ASSIGNED(resume from checkpoint)SUSPENDED: Noneto KanbanSTATUS_TO_COLUMN(off-board)Shutdown strategies (#202)
"immediate"): Force-cancel all tasks immediately with no grace period. Fastest shutdown, highest data loss."finish_tool"): Waittool_timeout_seconds(default 60s) for current tool invocation to complete, then cancel stragglers."checkpoint"): Grace period for cooperative checkpointing, checkpoint stragglers viaCheckpointSavercallback, reporttasks_suspendedcount.tasks_suspendedfield toShutdownResult(default 0, backward compatible)CheckpointSavertype alias andbuild_shutdown_strategyfactory functiontool_timeout_secondstoGracefulShutdownConfig_run_cleanupand_log_post_cancel_exceptionshelpers to reduce duplicationDesign spec
Test plan
Review coverage
Pre-reviewed by: code-reviewer, async-concurrency-reviewer, type-design-analyzer, docs-consistency, issue-resolution-verifier, silent-failure-hunter. 5 valid findings implemented (explicit post-cancel exception logging, error_type in checkpoint logs, missing error path test, full exception messages in task error logs, debug log for unknown task_id lookup).