feat(workspace): persistent project workspace + pluggable git backend + per-project push queue#2021
Conversation
Apply review findings from /pre-pr-review on the persistent project workspace + pluggable git backend substrate: - Redact embedded URL credentials before logging (SEC-1) via _redact_args helper applied to all run_git_subprocess log sites. - Parameterise the git() helper event constant so push/fetch failures log under the correct workspace event. - Defensive _reject_if_nested_in_parent_worktree + post-init .git existence check on EmbeddedGitBackend to prevent main-repo poisoning when git init silently no-ops. - Tighten path-traversal defence in ProjectWorkspaceService._workspace_path (reject /, \, ..). - Move dict reads inside locks in ProjectWorkspaceService._lock_for and WorkspaceIsolationService._get_or_create_queue (close double-checked-locking TOCTOU). - Document PushQueueCoordinator + persistent per-project workspace in docs/design/coordination.md. - Add gitlab/gitea/forgejo Connection Types row in docs/design/integrations.md. - Add 4014 / 4015 error codes to docs/reference/errors.md. - Add Git backend storage strategy section to docs/reference/pluggable-subsystems.md.
The previous round of guards in EmbeddedGitBackend/LocalPathGitBackend fired spuriously under the --count=2 isolation gate: - _reject_if_nested_in_parent_worktree now compares the resolved git toplevel against the requested path and only refuses when the toplevel is a strict parent (idempotent provision on an already- initialised path no longer trips the guard). - EmbeddedGitBackend: drop the bare-dir nested check (the bare is created under our config-controlled base_root, and 'git config' / 'git commit' only ever run inside the working tree, which still has the guard). Drop the post-init .git existence check; the pre-init guard plus is_git_repo short-circuit is now sufficient. - LocalPathGitBackend: gain the same pre-init nested-worktree guard so a misconfigured local_repo_path nested inside another worktree is still refused. Drop the brittle post-init .git existence check. - git init invocations stop passing a redundant trailing '.' (the cwd is already the target). - Test mypy fix-ups: replace removed 'type: ignore' on a fake repo pass-through with a no-any-return ignore on the mock_of helper, and add attr-defined ignores on the MagicMock-typed assert_awaited_once call sites.
When this code runs from inside a git pre-push hook (or any caller whose own cwd is a working tree) the hook process exports GIT_DIR / GIT_WORK_TREE pointing at the parent repo and any child git subprocess inherits them. is_git_repo() then reports True for a fresh tmp_path because git resolves against the parent repo's .git instead of the freshly-mkdir'd cwd, and tests under the --count=2 isolation gate fail with 'DID NOT RAISE' / 'remote already exists'. Strip GIT_* env vars in run_git_subprocess so all git calls in the backend strategies (and the embedded-backend test helper) resolve purely via cwd path discovery.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🧰 Additional context used📓 Path-based instructions (9)web/src/**/*.{js,jsx,ts,tsx,mts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/src/api/types/**/*.gen.ts📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/src/**/*.{ts,tsx,mts}📄 CodeRabbit inference engine (web/CLAUDE.md)
Files:
web/src/**/*.{ts,tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{py,md}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/api/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.py⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-05-05T09:04:46.195ZApplied to files:
🔇 Additional comments (4)
WalkthroughImplements a persistent, per-project git-backed workspace with a pluggable GitBackend (embedded, local path, external remote), a ProjectWorkspace domain and repositories (Postgres/SQLite) with migrations, and a PushQueueCoordinator to serialize merge-then-push per project. Wires services into app startup and worker execution, adds errors and observability events, extends integrations (GitLab, Gitea, Forgejo), updates generated web types/UI, and provides comprehensive unit/integration tests and design/docs updates. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a robust, persistent workspace substrate for projects, enabling durable git-backed working trees that survive across agent execution sessions. It introduces a pluggable git backend architecture, allowing for flexible storage configurations, and implements a serial push queue to ensure thread-safe, collision-free merges and pushes to git backends in concurrent agent environments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a persistent per-project workspace substrate, enabling git-backed working trees that survive across sessions, supported by a pluggable backend system and a serial push queue to prevent forge collisions. The review identifies a security vulnerability regarding unencoded tokens in authenticated URLs and a potential hang in the push queue worker during fatal exceptions. Furthermore, the backend-switching mechanism is currently functional only in configuration, as the implementation fails to re-initialize the repository when the backend type changes for an existing workspace path.
| netloc = f"{_TOKEN_USER}:{token}@{split.netloc}" | ||
| path = f"{split.path}/{project_id}.git" | ||
| return urlunsplit((split.scheme, netloc, path, "", "")) |
There was a problem hiding this comment.
The authenticated URL construction has two issues:
- The token is injected directly into the URL without encoding. If the token contains special characters (like @ or :), it will break the URL parsing.
- split.netloc is used directly, which might already contain userinfo if base_url was configured with credentials. This would result in invalid double-userinfo in the final URL.
Using split.hostname and split.port ensures a clean host part, and urllib.parse.quote handles special characters in the token. Additionally, ensure that this URL is redacted in all output channels and log messages to prevent sensitive data leakage, as per repository rules.
from urllib.parse import quote
host = split.hostname
if not host:
msg = f"External git connection {self._connection_name!r} has invalid base_url"
raise GitBackendConfigError(msg)
if split.port:
host = f"{host}:{split.port}"
netloc = f"{_TOKEN_USER}:{quote(token, safe='')}@{host}"
path = f"{split.path}/{project_id}.git"
return urlunsplit((split.scheme, netloc, path, "", ""))References
- Ensure that URLs containing potential credentials are redacted in all output channels, including tool execution results and log messages, to prevent sensitive data leakage.
| async def _worker_loop(self) -> None: | ||
| """Process queued merges strictly one at a time (FIFO).""" | ||
| queue = self._queue | ||
| if queue is None: # pragma: no cover - start() always assigns | ||
| return | ||
| # lint-allow: long-running-loop-kill-switch -- stop() puts a None sentinel | ||
| while True: | ||
| item = await queue.get() | ||
| if item is None: | ||
| return | ||
| try: | ||
| await self._process(item) | ||
| except MemoryError, RecursionError: | ||
| raise | ||
| except Exception as exc: | ||
| # A bug in _process must not kill the worker and strand | ||
| # every later caller; surface it to this caller and | ||
| # keep draining. | ||
| logger.error( | ||
| WORKSPACE_PUSH_QUEUE_WORKER_FAILED, | ||
| project_id=self._project_id, | ||
| error_type=type(exc).__name__, | ||
| error=safe_error_description(exc), | ||
| ) | ||
| if not item.future.done(): | ||
| item.future.set_exception(exc) | ||
|
|
There was a problem hiding this comment.
If the worker loop dies due to an unhandled BaseException (like MemoryError or RecursionError), any pending items in the queue will remain there forever, causing their callers to hang indefinitely. Adding a finally block to the worker loop ensures that any stranded requests are failed with an appropriate exception. The implementation correctly uses safe_error_description in the log message to avoid leaking sensitive data, adhering to repository redaction rules.
async def _worker_loop(self) -> None:
"""Process queued merges strictly one at a time (FIFO)."""
queue = self._queue
if queue is None: # pragma: no cover - start() always assigns
return
try:
# lint-allow: long-running-loop-kill-switch -- stop() puts a None sentinel
while True:
item = await queue.get()
if item is None:
return
try:
await self._process(item)
except (MemoryError, RecursionError):
raise
except Exception as exc:
# A bug in _process must not kill the worker and strand
# every later caller; surface it to this caller and
# keep draining.
logger.error(
WORKSPACE_PUSH_QUEUE_WORKER_FAILED,
project_id=self._project_id,
error_type=type(exc).__name__,
error=safe_error_description(exc),
)
if not item.future.done():
item.future.set_exception(exc)
finally:
# Worker is dying (e.g. MemoryError). Fail any pending items to avoid hangs.
while not queue.empty():
item = queue.get_nowait()
if item is not None and not item.future.done():
item.future.set_exception(
WorkspaceError("PushQueueCoordinator worker died")
)References
- Ensure that sensitive data is redacted in log messages to prevent leakage to logs or user interfaces.
| if row is not None and row.git_backend_kind != kind: | ||
| logger.warning( | ||
| WORKSPACE_BACKEND_KIND_CHANGED, | ||
| project_id=project_id, | ||
| from_backend=row.git_backend_kind.value, | ||
| to_backend=kind.value, | ||
| ) | ||
| return await self._provision(project_id, row, kind) |
There was a problem hiding this comment.
The backend-switching logic (Acceptance #3) appears to be non-functional on disk because the GitBackend.provision implementations (Embedded, LocalPath, ExternalRemote) all short-circuit if is_git_repo(workspace_path) is true. Since the workspace path is preserved during a switch, the new backend will see the old backend's .git directory and skip its own initialization logic (e.g., setting up the correct remote or bare repo). Additionally, ExternalRemoteGitBackend uses git clone ... ., which will fail if the directory is not empty. To support backend switching while keeping the path, the service should likely clear the existing directory (or at least the .git subdirectory) when a kind change is detected.
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/synthorg/engine/coordination/factory.py (1)
119-162:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast on
git_backend-only wiring.If
git_backendis passed withoutworkspace_strategyandworkspace_config,_build_workspace_service()just returnsNone, silently disabling the new queueing path. Reject that combination up front so bootstrap misconfigurations surface immediately.Proposed fix
def _build_workspace_service( workspace_strategy: WorkspaceIsolationStrategy | None, workspace_config: WorkspaceIsolationConfig | None, git_backend: GitBackend | None = None, ) -> WorkspaceIsolationService | None: @@ + if git_backend is not None and ( + workspace_strategy is None or workspace_config is None + ): + msg = ( + "git_backend requires both workspace_strategy and " + "workspace_config" + ) + raise ValueError(msg) if workspace_strategy is not None and workspace_config is not None: from synthorg.engine.workspace.service import ( # noqa: PLC0415 WorkspaceIsolationService, )Also applies to: 175-176, 265-267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/synthorg/engine/coordination/factory.py` around lines 119 - 162, The function _build_workspace_service currently allows git_backend to be provided alone and returns None, silently disabling workspace isolation; update _build_workspace_service to treat the combination where git_backend is not None while both workspace_strategy and workspace_config are None as an invalid configuration: log a suitable COORDINATION_FACTORY_BUILT warning (similar to the existing mismatched deps log) and raise a ValueError with a clear message indicating git_backend cannot be supplied without workspace_strategy and workspace_config; apply the same validation and error behavior to the other similar factory blocks that accept git_backend (the other two code paths referenced in the review) so any git_backend-only wiring fails fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/schema_drift_baseline.txt`:
- Around line 108-109: The two baseline entries for project_workspaces
timestamps use placeholder audit text; replace the placeholder justifications
for the columns "project_workspaces.created_at" and
"project_workspaces.updated_at" with the established TIMESTAMPTZ justification
used elsewhere in the baseline (i.e., match the standard audit justification
string used for other TIMESTAMPTZ columns) so the schema_drift_baseline entries
are consistent and no placeholder instructions remain.
In `@src/synthorg/api/app.py`:
- Around line 1046-1078: The project workspace service is being registered via
app_state.set_project_workspace_service(...) before the fallible
build_runtime_services(...) runs, causing subsequent startup retries to hit the
_set_once guard and fail; move the call that sets the ProjectWorkspaceService
(the set_project_workspace_service invocation that constructs
ProjectWorkspaceService with GitBackendConfig/git_backend) so it executes only
after build_runtime_services(...) completes successfully, or alternatively wrap
the registration in the same try/except that surrounds
build_runtime_services(...) and only call set_project_workspace_service on
success, ensuring _runtime_services_installed remains false on failures and
allowing clean retry.
In `@src/synthorg/engine/workspace/git_backend/external_remote.py`:
- Around line 79-81: When calling
self._catalog.get_credentials(self._connection_name) in external_remote.py,
guard against a None return before dereferencing .get(): check if credentials is
falsy (None) and raise a GitBackendConfigError with a clear message that
includes the connection name (so the typed backend error path is used) otherwise
proceed to extract token via credentials.get("token"); update the logic around
the credentials lookup (the call to get_credentials and subsequent token
extraction) to implement this check and raise GitBackendConfigError when
missing.
In `@src/synthorg/engine/workspace/git_backend/local_path.py`:
- Around line 58-60: The _non_empty_non_repo_dir method currently calls
self._repo_path.iterdir() which raises NotADirectoryError if _repo_path is a
file; update this to first check self._repo_path.exists() and
self._repo_path.is_dir() before calling it, and if it's a file (exists but not
is_dir()) raise the typed GitBackendConfigError with a clear message; apply the
same guard/behavior to the other place that directly calls it or uses it (the
second occurrence referenced in the review) so all non-directory paths produce
GitBackendConfigError instead of leaking raw NotADirectoryError.
- Around line 46-53: The constructor currently sets a single self._repo_path and
provision() ignores the workspace_path, causing all projects to share one repo;
change the design so repo paths are computed per workspace/project instead of
reused: stop storing a single repo root in self._repo_path (or rename to
self._base_path) and update provision(workspace_path, ...) to derive a
project-specific Path (e.g. Path(workspace_path) / project_id or similar) and
use that local path for clone/init/push/pull operations; update all places that
reference self._repo_path (including methods referenced in the diff and the
other occurrences) to use the computed per-call repo path or a per-project
mapping, ensuring no global reuse and preserving _cmd_timeout and _clock usage.
In `@src/synthorg/engine/workspace/project_workspace_service.py`:
- Around line 91-97: Before raising GitBackendConfigError in the project_id
path-check block, emit a warning/error log that includes the offending
project_id and context (e.g., self._base_root and _PROJECTS_SUBDIR) and attach
the appropriate observability event constant (from
observability.events.<domain>, e.g., workspace or projects) so the rejection is
auditable; then raise GitBackendConfigError as before. Use the instance logger
(e.g., self.logger or the module logger) and call logger.warning or logger.error
with a message and the event constant, then raise
GitBackendConfigError(project_id message) unchanged.
- Around line 147-149: The code recomputes workspace_path even when a prior
provision exists, which can move the project tree; change the assignment so that
if prior is not None you reuse prior.workspace_path (e.g., workspace_path =
prior.workspace_path) otherwise call self._workspace_path(project_id), and
continue to mkdir on that chosen workspace_path; update any references in
ProjectWorkspaceService methods that assume the recomputed path to use the
selected workspace_path instead.
In `@src/synthorg/engine/workspace/push_queue.py`:
- Around line 98-139: Introduce and use a shared "closing" state to prevent new
items after stop() is started: add a boolean attribute (e.g. self._closing =
False) initialized when the coordinator is created; in stop() set self._closing
= True before putting the None sentinel into self._queue and then proceed to
await the worker; in enqueue_merge_push() check the flag (if self._closing:
raise WorkspaceError("PushQueueCoordinator is stopping") ) before accepting and
putting any _QueuedMerge items; keep the existing idempotent cleanup (clearing
_worker/_queue) but ensure the closing flag is set early so no new enqueues can
be appended behind the sentinel.
- Around line 194-207: The except block currently wraps every failure into a new
WorkspacePushError and loses backend-specific push error types; instead, keep
the logging but if the caught exception is already a push-domain error (e.g.,
WorkspacePushError or any existing backend push exception base/class in your
codebase), set that original exception on item.future
(item.future.set_exception(exc)); only wrap truly unexpected exceptions by
creating a new WorkspacePushError and setting that on item.future. Ensure you
reference WORKSPACE_PUSH_QUEUE_FAILED, safe_error_description, item.future, and
WorkspacePushError when updating the except branch.
In `@src/synthorg/persistence/postgres/backend.py`:
- Line 291: The _project_workspaces repository field is initialized but not
reset during teardown; update the backend's _clear_state() method to set
self._project_workspaces = None (same as other repository fields) so callers
won't observe a stale repository after disconnect/reset, and ensure any other
repository attributes follow the same pattern in _clear_state().
In
`@src/synthorg/persistence/postgres/revisions/20260519000001_project_workspaces.sql`:
- Around line 11-21: Add a uniqueness constraint so no two project_workspaces
can share the same workspace_path: modify the CREATE TABLE for
project_workspaces to include a UNIQUE constraint on workspace_path (or create a
unique index on workspace_path) while keeping the existing PRIMARY KEY on
project_id and the FOREIGN KEY to projects; reference the project_workspaces
table and the workspace_path and project_id columns when making this change.
In `@src/synthorg/persistence/postgres/schema.sql`:
- Around line 454-464: The schema allows duplicate workspace_path values which
permits multiple projects to share the same workspace; modify the
project_workspaces definition to enforce uniqueness on the workspace_path column
(e.g. add a UNIQUE constraint or unique index named something like
project_workspaces_workspace_path_key) so that workspace_path cannot be reused
across projects — update the CREATE TABLE for project_workspaces to include the
UNIQUE(workspace_path) constraint (or add an ALTER TABLE ... ADD CONSTRAINT ...
UNIQUE on workspace_path) and ensure any tests or migrations account for the new
constraint.
In `@src/synthorg/persistence/sqlite/backend.py`:
- Line 208: The _project_workspaces repository field is left set after teardown;
update the backend's _clear_state() method to null out self._project_workspaces
(and call any repository cleanup method first if the repository exposes one,
e.g., close()/dispose()) so the stale SQLiteProjectWorkspaceRepository instance
is released after disconnect/failure; ensure you modify the class where
_project_workspaces is defined so subsequent reconnects recreate a fresh
repository.
In `@src/synthorg/persistence/sqlite/project_workspace_repo.py`:
- Around line 75-85: The rollback helper _safe_rollback currently always logs
PERSISTENCE_PROJECT_WORKSPACE_SAVE_FAILED which mislabels failures from delete
flows; modify _safe_rollback to accept an event parameter (e.g., failure_event)
or an enum and log that instead, update callers (the save and delete methods
that call _safe_rollback) to pass PERSISTENCE_PROJECT_WORKSPACE_SAVE_FAILED for
save paths and PERSISTENCE_PROJECT_WORKSPACE_DELETE_FAILED for delete paths, and
keep the same structured fields (error_type, error, rollback_failed) when
emitting the chosen event.
In
`@src/synthorg/persistence/sqlite/revisions/20260519000001_project_workspaces.sql`:
- Around line 11-21: Add a uniqueness constraint on the workspace_path column in
the project_workspaces table so multiple projects cannot reference the same
directory; modify the CREATE TABLE for project_workspaces to declare
workspace_path as UNIQUE (or add a UNIQUE (workspace_path) table constraint)
while keeping existing columns (project_id primary key, git_backend_kind CHECK,
remote_ref, default_branch, created_at, updated_at, and the FOREIGN KEY to
projects) intact.
In `@src/synthorg/persistence/sqlite/schema.sql`:
- Around line 440-450: The project_workspaces table allows duplicate
workspace_path values; update the CREATE TABLE for project_workspaces to enforce
uniqueness on workspace_path (e.g., add a UNIQUE constraint on workspace_path or
declare workspace_path TEXT NOT NULL UNIQUE) so each workspace path maps to only
one project; modify the schema definition around project_workspaces to include
this constraint and run the corresponding migration to apply the change.
In `@tests/integration/engine/workspace/test_project_workspace_acceptance.py`:
- Around line 199-205: The test currently only asserts len(push_order) == 2
which misses concurrent overlap; update the test to record push start and end
markers (or timestamps) into push_order for each merge so you can assert
serialization by verifying the first push's end <= second push's start and that
the two pushes correspond to r1 and r2 respectively (use the existing push_order
list and the merge results r1 and r2 to correlate events); ensure you assert
both the count and non-overlap (e.g., push_order contains two items with
start/end timestamps and assert end_of_first <= start_of_second).
In `@tests/unit/engine/workspace/git_backend/test_embedded_backend.py`:
- Around line 95-99: The test calls git commit via the helper _git in
test_embedded_backend.py without configuring git user identity, causing failures
on CI machines without global git config; before invoking _git(..., "commit",
...) set repository-local git config for user.name and user.email (e.g. via
_git(ws, "config", "user.name", "...") and _git(ws, "config", "user.email",
"...")) so the commit succeeds deterministically; add these _git config calls
just after creating the feature branch (after _git(ws, "checkout", "-b",
"feature") and before adding/committing files).
In `@tests/unit/integrations/test_connection_types.py`:
- Around line 68-76: Extend the test_missing_token_rejected coverage to assert
that Gitea and Forgejo authenticators also reject empty and whitespace-only
tokens: after obtaining auth via get_authenticator(ct) for ConnectionType.GITEA
and ConnectionType.FORGEJO, call auth.validate_credentials with {"token": ""}
and with {"token": " "} and expect InvalidConnectionAuthError with a message
containing "token" for each case, ensuring the same validation path exercised by
the existing auth.validate_credentials({}) check.
In `@tests/unit/persistence/test_protocol.py`:
- Around line 1021-1024: Add an explicit protocol conformance assertion for the
new property: when returning _FakeProjectWorkspaceRepository from the
project_workspaces property, add an isinstance check that ensures the returned
object conforms to ProjectWorkspaceRepository (use ProjectWorkspaceRepository as
the protocol to check against) so the test suite will catch protocol drift for
project_workspaces.
---
Outside diff comments:
In `@src/synthorg/engine/coordination/factory.py`:
- Around line 119-162: The function _build_workspace_service currently allows
git_backend to be provided alone and returns None, silently disabling workspace
isolation; update _build_workspace_service to treat the combination where
git_backend is not None while both workspace_strategy and workspace_config are
None as an invalid configuration: log a suitable COORDINATION_FACTORY_BUILT
warning (similar to the existing mismatched deps log) and raise a ValueError
with a clear message indicating git_backend cannot be supplied without
workspace_strategy and workspace_config; apply the same validation and error
behavior to the other similar factory blocks that accept git_backend (the other
two code paths referenced in the review) so any git_backend-only wiring fails
fast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 54bbd55a-ef8c-4493-9a71-35d0a4ced99f
📒 Files selected for processing (64)
docs/design/coordination.mddocs/design/integrations.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.mdscripts/_ghost_wiring_manifest.txtscripts/schema_drift_baseline.txtsrc/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/core/enums.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/core/project_workspace.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/errors.pysrc/synthorg/engine/workspace/_git_subprocess.pysrc/synthorg/engine/workspace/git_backend/__init__.pysrc/synthorg/engine/workspace/git_backend/_git_ops.pysrc/synthorg/engine/workspace/git_backend/config.pysrc/synthorg/engine/workspace/git_backend/embedded.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/engine/workspace/git_backend/factory.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/engine/workspace/git_backend/protocol.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/engine/workspace/push_queue.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/integrations/connections/models.pysrc/synthorg/integrations/connections/types/__init__.pysrc/synthorg/integrations/connections/types/gitea.pysrc/synthorg/integrations/connections/types/gitlab.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/persistence/postgres/project_workspace_repo.pysrc/synthorg/persistence/postgres/revisions/20260519000001_project_workspaces.sqlsrc/synthorg/persistence/postgres/schema.sqlsrc/synthorg/persistence/project_workspace_protocol.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/_backend_accessors.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/persistence/sqlite/revisions/20260519000001_project_workspaces.sqlsrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/workers/execution_service.pysrc/synthorg/workers/runtime_builder.pytests/conformance/persistence/test_project_workspace_repository.pytests/integration/engine/workspace/conftest.pytests/integration/engine/workspace/test_project_workspace_acceptance.pytests/unit/api/fakes.pytests/unit/api/fakes_backend.pytests/unit/core/test_project_workspace_model.pytests/unit/engine/workspace/git_backend/conftest.pytests/unit/engine/workspace/git_backend/test_config.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/unit/engine/workspace/git_backend/test_external_remote_backend.pytests/unit/engine/workspace/git_backend/test_factory.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pytests/unit/engine/workspace/test_project_workspace_service.pytests/unit/engine/workspace/test_push_queue.pytests/unit/engine/workspace/test_service_push.pytests/unit/integrations/test_connection_types.pytests/unit/persistence/test_protocol.pyweb/src/api/types/enum-values.gen.tsweb/src/api/types/error-codes.gen.tsweb/src/api/types/openapi.gen.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Dashboard Test
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test Unit
- GitHub Check: Test E2E
- GitHub Check: Test Integration
- GitHub Check: Lighthouse Site
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (14)
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Onlysrc/synthorg/persistence/may import sqlite/psycopg or emit raw SQL
Use DB > env > code default via SettingsService/ConfigResolver (Cat-1) or env > code default with read_only_post_init (Cat-2); Cat-3 bootstrap secrets are pure env at boot site. YAML is company-template ingestion format only, not a precedence tier. No os.environ.get outside startup; pre-init Cat-2 reads use settings.bootstrap_resolver.resolve_init_value
Use from synthorg.observability import get_logger; variable always named logger. Never import logging or use print() in app code. Event names from observability.events. constants; structured kwargs (logger.info(EVENT, key=value))
Error paths log WARNING/ERROR with context before raising; state transitions log INFO via *_STATUS_TRANSITIONED after persistence write
Never log error=str(exc) or interpolate {exc}; use error_type=type(exc).name + error=safe_error_description(exc). Never use exc_info=True. OTel: forbidden span.record_exception(exc); use span.set_attribute("exception.message", safe_error_description(exc)) + record_exception=False, set_status_on_exception=False. Enforced by check_logger_exception_str_exc.py
Files:
src/synthorg/engine/workspace/git_backend/__init__.pysrc/synthorg/integrations/connections/types/gitlab.pysrc/synthorg/core/enums.pysrc/synthorg/engine/workspace/git_backend/_git_ops.pysrc/synthorg/persistence/project_workspace_protocol.pysrc/synthorg/api/app.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/engine/workspace/git_backend/embedded.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/integrations/connections/models.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/state.pysrc/synthorg/engine/workspace/git_backend/config.pysrc/synthorg/persistence/sqlite/_backend_accessors.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/errors.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/git_backend/factory.pysrc/synthorg/engine/workspace/git_backend/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/integrations/connections/types/gitea.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/_git_subprocess.pysrc/synthorg/integrations/connections/types/__init__.pysrc/synthorg/persistence/postgres/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.pysrc/synthorg/workers/execution_service.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/core/project_workspace.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Numerics live insettings/definitions/; allowlist 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants (NAME: int|float|Final|Final[int]|Final[float] = literal). Enforced by scripts/check_no_magic_numbers.py
Comments should explain WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py
Do not usefrom __future__ import annotations(Python 3.14 has PEP 649). Use PEP 758 except:except A, B:requires parens when binding
Type hints required on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines
Define errors as<Domain><Condition>Errorinheriting fromDomainError; never inherit from Exception/RuntimeError/etc directly. Enforced by check_domain_error_hierarchy.py
Pydantic v2: use frozen + extra="forbid" on every frozen model project-wide (gate check_frozen_model_extra_forbid.py;@computed_fieldauto-exempt, per-line # lint-allow: frozen-extra-forbid -- for extra="allow"/"ignore" boundaries). Use@computed_fieldfor derived fields. Use NotBlankStr for identifiers
Args models at every system boundary; parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py
Use immutability patterns: model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries
Use asyncio.TaskGroup for fan-out/fan-in async patterns; helpers must catch Exception (re-raise MemoryError/RecursionError)
Clock seam: includeclock: Clock | None = Noneparameter; tests inject FakeClock. Lifecycle: services own_lifecycle_lock; timed-out stops mark unrestartable
For untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML
Never use real vendor names in project code/tests; use example-provider, test-provider, example-{large,medium,small}-001. Allowed in .claude/, third-party imports, providers/presets.py, we...
Files:
src/synthorg/engine/workspace/git_backend/__init__.pysrc/synthorg/integrations/connections/types/gitlab.pysrc/synthorg/core/enums.pysrc/synthorg/engine/workspace/git_backend/_git_ops.pysrc/synthorg/persistence/project_workspace_protocol.pysrc/synthorg/api/app.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/engine/workspace/git_backend/embedded.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/integrations/connections/models.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/state.pysrc/synthorg/engine/workspace/git_backend/config.pysrc/synthorg/persistence/sqlite/_backend_accessors.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/errors.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/git_backend/factory.pysrc/synthorg/engine/workspace/git_backend/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/integrations/connections/types/gitea.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/_git_subprocess.pysrc/synthorg/integrations/connections/types/__init__.pysrc/synthorg/persistence/postgres/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.pysrc/synthorg/workers/execution_service.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/core/project_workspace.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/workspace/git_backend/__init__.pysrc/synthorg/integrations/connections/types/gitlab.pysrc/synthorg/core/enums.pysrc/synthorg/engine/workspace/git_backend/_git_ops.pysrc/synthorg/persistence/project_workspace_protocol.pysrc/synthorg/api/app.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/engine/workspace/git_backend/embedded.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/integrations/connections/models.pysrc/synthorg/persistence/protocol.pysrc/synthorg/api/state.pysrc/synthorg/engine/workspace/git_backend/config.pysrc/synthorg/persistence/sqlite/_backend_accessors.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/engine/errors.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/git_backend/factory.pysrc/synthorg/engine/workspace/git_backend/protocol.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/integrations/connections/types/gitea.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/_git_subprocess.pysrc/synthorg/integrations/connections/types/__init__.pysrc/synthorg/persistence/postgres/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.pysrc/synthorg/workers/execution_service.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/core/project_workspace.py
{README.md,docs/**/*.md,data/**/*.md}
📄 CodeRabbit inference engine (CLAUDE.md)
Numerics in README and public docs must be sourced from data/runtime_stats.yaml via markers
Files:
docs/design/integrations.mddocs/design/coordination.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.md
docs/**/*.{md,d2}
📄 CodeRabbit inference engine (CLAUDE.md)
Use d2 for architecture/nested containers, mermaid for flowcharts/sequence/pipelines. Markdown tables for tabular data. D2 theme 200 (Dark Mauve), D2 CLI pinned to v0.7.1 in CI
Files:
docs/design/integrations.mddocs/design/coordination.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.md
src/synthorg/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/persistence/**/*.py: New repository protocols must inherit from one or more generic categories inpersistence/_generics.py(SingletonRepository, IdKeyedRepository, FilteredQueryRepository, AppendOnlyRepository, StatefulRepository, MVCCRepository); bespoke methods allowed only under ADR-0001 D7 with real performance optimisation or domain invariant justification
Keep repository protocols as@runtime_checkable
Repository CRUD methods: save(entity), get(id), delete(id) -> bool, list_items(...), query(...) returning tuples
Datetime in persistence: use parse_iso_utc / format_iso_utc from persistence._shared (reject naive); use normalize_utc for already-typed datetimes
Files:
src/synthorg/persistence/project_workspace_protocol.pysrc/synthorg/persistence/protocol.pysrc/synthorg/persistence/sqlite/_backend_accessors.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/persistence/postgres/project_workspace_repo.pysrc/synthorg/persistence/sqlite/project_workspace_repo.py
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/api/types/error-codes.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/src/api/types/**/*.gen.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Generated DTO types (MANDATORY): NEVER hand-edit
web/src/api/types/*.gen.ts. Regenerate withuv run python scripts/generate_dto_types_ts.py. Import DTOs via the barrel (import type { AgentConfig } from '@/api/types').
Files:
web/src/api/types/error-codes.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/api/types/error-codes.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse
web/src/components/ui/design tokens only in the Web Dashboard; seeweb/CLAUDE.mdfor details
Files:
web/src/api/types/error-codes.gen.tsweb/src/api/types/enum-values.gen.tsweb/src/api/types/openapi.gen.ts
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
API startup lifecycle: construction phase wires synchronous services; on_startup wires services needing connected persistence. Construction-phase invariants: agent_registry before auto_wire_meetings; tunnel_provider wired unconditionally. On-startup invariants: SettingsService auto-wire before WorkflowExecutionObserver registration; OntologyService wires after persistence.connect() via _wire_ontology_service
Files:
src/synthorg/api/app.pysrc/synthorg/api/state.py
src/synthorg/{workers,api}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Runtime services: synthorg.workers.runtime_builder.build_runtime_services selects behind ONE provider-present switch, returning RuntimeServices (AgentEngineExecutionService + coordinator or NoProviderExecutionService + None). _install_runtime_services appends FIRST after persistence/SettingsService hooks; swap_worker_execution_service / swap_coordinator / swap_provider_registry hold a lock. Empty-company rejects task creation (AgentRuntimeNotConfiguredError 4014) and /coordinate 503s
Files:
src/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/workers/runtime_builder.pysrc/synthorg/workers/execution_service.py
src/synthorg/observability/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Telemetry: opt-in, off by default. Every event property must be in _ALLOWED_PROPERTIES. See telemetry.md
Files:
src/synthorg/observability/events/persistence.pysrc/synthorg/observability/events/workspace.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.{unit,integration,e2e,slow}. Async auto. Timeout 30s global. Coverage 80% min
Windows: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race). Subprocess tests override back
Test doubles: use FakeClock for Clock seam, mock_ofT for typed-boundary substitutions, SimpleNamespace for attribute-bags. Never use bare MagicMock at typed boundary (constructor/fn arg/annotated local/typed fixture return); blocked by scripts/check_mock_spec.py (zero-tolerance, no baseline). Import FakeClock and mock_of from tests._shared
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add@example(...))
Flaky tests: NEVER skip/xfail; fix fundamentally. Use asyncio.Event().wait() not sleep(large)
Files:
tests/integration/engine/workspace/conftest.pytests/unit/engine/workspace/git_backend/conftest.pytests/unit/api/fakes.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_external_remote_backend.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/unit/core/test_project_workspace_model.pytests/unit/api/fakes_backend.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pytests/unit/engine/workspace/git_backend/test_factory.pytests/conformance/persistence/test_project_workspace_repository.pytests/unit/integrations/test_connection_types.pytests/unit/engine/workspace/git_backend/test_config.pytests/unit/engine/workspace/test_push_queue.pytests/unit/engine/workspace/test_project_workspace_service.pytests/unit/engine/workspace/test_service_push.pytests/integration/engine/workspace/test_project_workspace_acceptance.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/integration/engine/workspace/conftest.pytests/unit/engine/workspace/git_backend/conftest.pytests/unit/api/fakes.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_external_remote_backend.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/unit/core/test_project_workspace_model.pytests/unit/api/fakes_backend.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pytests/unit/engine/workspace/git_backend/test_factory.pytests/conformance/persistence/test_project_workspace_repository.pytests/unit/integrations/test_connection_types.pytests/unit/engine/workspace/git_backend/test_config.pytests/unit/engine/workspace/test_push_queue.pytests/unit/engine/workspace/test_project_workspace_service.pytests/unit/engine/workspace/test_service_push.pytests/integration/engine/workspace/test_project_workspace_acceptance.py
tests/conformance/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Dual-backend conformance: tests/conformance/persistence/ must consume backend fixture (SQLite + Postgres). Enforced by check_dual_backend_test_parity.py
Files:
tests/conformance/persistence/test_project_workspace_repository.py
🧠 Learnings (5)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/engine/workspace/git_backend/__init__.pysrc/synthorg/integrations/connections/types/gitlab.pysrc/synthorg/core/enums.pysrc/synthorg/engine/workspace/git_backend/_git_ops.pysrc/synthorg/persistence/project_workspace_protocol.pysrc/synthorg/api/app.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/engine/workspace/git_backend/embedded.pysrc/synthorg/observability/events/persistence.pytests/integration/engine/workspace/conftest.pytests/unit/engine/workspace/git_backend/conftest.pysrc/synthorg/integrations/connections/models.pysrc/synthorg/persistence/protocol.pytests/unit/api/fakes.pytests/unit/persistence/test_protocol.pysrc/synthorg/api/state.pysrc/synthorg/engine/workspace/git_backend/config.pytests/unit/engine/workspace/git_backend/test_external_remote_backend.pysrc/synthorg/persistence/sqlite/_backend_accessors.pysrc/synthorg/observability/events/workspace.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/unit/core/test_project_workspace_model.pysrc/synthorg/engine/errors.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/engine/workspace/project_workspace_service.pytests/unit/api/fakes_backend.pysrc/synthorg/workers/runtime_builder.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pysrc/synthorg/engine/coordination/factory.pytests/unit/engine/workspace/git_backend/test_factory.pysrc/synthorg/engine/workspace/git_backend/factory.pysrc/synthorg/engine/workspace/git_backend/protocol.pysrc/synthorg/persistence/sqlite/backend.pytests/conformance/persistence/test_project_workspace_repository.pytests/unit/integrations/test_connection_types.pytests/unit/engine/workspace/git_backend/test_config.pysrc/synthorg/engine/workspace/git_backend/external_remote.pytests/unit/engine/workspace/test_push_queue.pysrc/synthorg/integrations/connections/types/gitea.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/_git_subprocess.pysrc/synthorg/integrations/connections/types/__init__.pysrc/synthorg/persistence/postgres/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.pysrc/synthorg/workers/execution_service.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pytests/unit/engine/workspace/test_project_workspace_service.pysrc/synthorg/core/project_workspace.pytests/unit/engine/workspace/test_service_push.pytests/integration/engine/workspace/test_project_workspace_acceptance.py
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/design/integrations.mddocs/design/coordination.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/design/integrations.mddocs/design/coordination.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/design/integrations.mddocs/design/coordination.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/design/integrations.mddocs/design/coordination.mddocs/reference/errors.mddocs/reference/pluggable-subsystems.md
🔇 Additional comments (55)
src/synthorg/integrations/connections/models.py (1)
39-41: LGTM!src/synthorg/integrations/connections/types/gitlab.py (1)
1-44: LGTM!src/synthorg/integrations/connections/types/gitea.py (1)
1-76: LGTM!src/synthorg/integrations/connections/types/__init__.py (1)
22-27: LGTM!Also applies to: 44-46
docs/design/coordination.md (1)
397-430: LGTM!docs/design/integrations.md (1)
34-36: LGTM!docs/reference/errors.md (1)
101-102: LGTM!docs/reference/pluggable-subsystems.md (1)
170-178: LGTM!web/src/api/types/enum-values.gen.ts (1)
217-219: LGTM!web/src/api/types/error-codes.gen.ts (1)
58-58: LGTM!web/src/api/types/openapi.gen.ts (3)
6253-6253: LGTM!
6658-6658: LGTM!
7544-7544: LGTM!tests/unit/engine/workspace/test_push_queue.py (1)
22-170: LGTM!tests/unit/engine/workspace/test_service_push.py (1)
16-109: LGTM!tests/unit/integrations/test_connection_types.py (1)
33-54: LGTM!Also applies to: 77-83
tests/unit/api/fakes.py (1)
606-630: LGTM!tests/unit/api/fakes_backend.py (1)
42-43: LGTM!Also applies to: 598-599, 713-716
tests/unit/persistence/test_protocol.py (1)
583-603: LGTM!src/synthorg/core/enums.py (1)
401-414: LGTM!src/synthorg/core/project_workspace.py (4)
19-19: LGTM!
22-54: LGTM!
56-59: LGTM!
55-55: ⚡ Quick winNo action needed—the NotBlankStr instantiation pattern is correct.
The pattern
default=NotBlankStr(_DEFAULT_BRANCH)is the established and correct idiom throughout the synthorg codebase for NotBlankStr defaults in Pydantic v2 Field definitions. This same pattern is consistently used across multiple files (config.py, bridge_configs.py, etc.) without issue. Pydantic v2 Field defaults handle Annotated[str, StringConstraints(...)] type instantiation correctly in this form.> Likely an incorrect or invalid review comment.src/synthorg/core/error_taxonomy.py (1)
110-110: LGTM!src/synthorg/engine/errors.py (3)
169-179: LGTM!
181-204: LGTM!
206-242: LGTM!src/synthorg/observability/events/workspace.py (1)
42-60: LGTM!src/synthorg/observability/events/persistence.py (1)
300-321: LGTM!src/synthorg/engine/workspace/_git_subprocess.py (1)
12-13: LGTM!Also applies to: 19-19, 26-63, 96-96, 106-106, 122-122, 132-132
src/synthorg/api/state.py (1)
148-150: LGTM!Also applies to: 269-270, 457-460, 976-995
src/synthorg/workers/execution_service.py (1)
74-76: LGTM!Also applies to: 291-292, 306-307, 322-325, 353-374
src/synthorg/workers/runtime_builder.py (1)
381-390: LGTM!Also applies to: 399-399, 546-546
scripts/_ghost_wiring_manifest.txt (1)
45-50: LGTM!src/synthorg/engine/workspace/git_backend/protocol.py (1)
19-44: LGTM!Also applies to: 46-104
src/synthorg/engine/workspace/git_backend/config.py (1)
30-68: LGTM!Also applies to: 70-88
src/synthorg/engine/workspace/git_backend/_git_ops.py (1)
28-71: LGTM!Also applies to: 74-85
src/synthorg/engine/workspace/git_backend/embedded.py (1)
124-221: LGTM!Also applies to: 223-279
src/synthorg/engine/workspace/git_backend/factory.py (1)
31-82: LGTM!Also applies to: 94-112
src/synthorg/engine/workspace/git_backend/__init__.py (1)
8-40: LGTM!tests/conformance/persistence/test_project_workspace_repository.py (1)
13-131: LGTM!tests/unit/engine/workspace/git_backend/conftest.py (1)
10-26: LGTM!tests/unit/engine/workspace/git_backend/test_config.py (1)
9-52: LGTM!tests/unit/engine/workspace/git_backend/test_external_remote_backend.py (1)
22-85: LGTM!tests/unit/engine/workspace/git_backend/test_factory.py (1)
17-60: LGTM!tests/unit/engine/workspace/git_backend/test_local_path_backend.py (1)
12-81: LGTM!tests/unit/core/test_project_workspace_model.py (1)
12-66: LGTM!tests/unit/engine/workspace/test_project_workspace_service.py (1)
1-137: LGTM!tests/integration/engine/workspace/conftest.py (1)
1-21: LGTM!tests/integration/engine/workspace/test_project_workspace_acceptance.py (1)
1-193: LGTM!Also applies to: 206-327
src/synthorg/persistence/project_workspace_protocol.py (1)
13-17: LGTM!Also applies to: 28-87
src/synthorg/persistence/postgres/project_workspace_repo.py (1)
35-207: LGTM!src/synthorg/persistence/sqlite/_backend_accessors.py (1)
85-87: LGTM!Also applies to: 146-147, 307-310
src/synthorg/persistence/protocol.py (1)
107-109: LGTM!Also applies to: 412-415
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2021 +/- ##
==========================================
- Coverage 85.02% 84.99% -0.03%
==========================================
Files 1931 1947 +16
Lines 114950 115795 +845
Branches 9782 9839 +57
==========================================
+ Hits 97733 98420 +687
- Misses 14808 14946 +138
- Partials 2409 2429 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CI (all collapsed onto one fix in connection-type-fields.ts): - Lighthouse Dashboard / Pass, Dashboard Type Check / Build, Build Web Assets (melange), CI Pass: TS2739 missing gitlab / gitea / forgejo entries in CONNECTION_TYPE_FIELDS CodeRabbit (21): - schema_drift_baseline.txt:108-109 placeholder text replaced with the standard SQLite TIMESTAMPTZ justification - api/app.py:1078 set_project_workspace_service guarded against partial-startup retry (build_runtime_services is fallible) - external_remote.py:81 credentials lookup guards against None - local_path.py:53 LocalPathGitBackend now derives per-project repos under [local_repo_path]/[project_id]; multi-project isolation no longer requires the embedded backend - local_path.py:60 _non_empty_non_repo_dir handles file paths (no NotADirectoryError leak) - project_workspace_service.py:97 traversal rejection logs WORKSPACE_PATH_TRAVERSAL_REJECTED before raising - project_workspace_service.py:149 reuses prior.workspace_path on re-provision (no path relocation across re-provisions) - push_queue.py:139 _closing flag refuses enqueue once stop starts (no items behind the sentinel) - push_queue.py:207 preserves WorkspacePushError subtypes via the extracted _push_default_branch helper - service.py:288 _shutting_down flag refuses queue creation once shutdown begins - postgres/backend.py:291 _clear_state nulls _project_workspaces - sqlite/backend.py:208 same - postgres + sqlite migrations and schemas: UNIQUE workspace_path prevents cross-project path aliasing - sqlite/project_workspace_repo.py:85 _safe_rollback takes an event parameter; delete-path rollback failures no longer log SAVE_FAILED - test_project_workspace_acceptance.py:205 asserts max_concurrent_pushes == 1 (hard serialisation invariant) and verifies the .git wipe + new-backend init on kind switch - test_embedded_backend.py:99 sets repo-local user.name / user.email before git commit (no env dependency on CI) - test_connection_types.py:76 empty / whitespace-only token rejection parametrise for Gitea + Forgejo - test_protocol.py:1024 ProjectWorkspaceRepository conformance via backend.project_workspaces AND the fake directly - coordination/factory.py:119-162 (outside diff range): _build_workspace_service fails fast when git_backend is supplied without workspace_strategy AND workspace_config Gemini (3): - external_remote.py:93 percent-encodes the token with quote and uses split.hostname + split.port to avoid double-userinfo - push_queue.py:167 worker loop wraps in try/finally; pending items fail with WorkspaceError on BaseException exit instead of hanging forever - project_workspace_service.py:138 clears prior .git on backend-kind switch so the new backend is_git_repo short-circuit no longer leaves the old layout in place
…ct-workspace # Conflicts: # scripts/_ghost_wiring_manifest.txt # src/synthorg/core/error_taxonomy.py # web/src/api/types/error-codes.gen.ts # web/src/api/types/openapi.gen.ts
Issues caught by the pre-push hooks on the first push attempt: - SQL: SQLite rejected the table-constraint `UNIQUE (workspace_path)` placed BETWEEN column definitions (`workspace_path` and `git_backend_kind`) -- column-then-constraint-then-column ordering is a syntax error in SQLite. Moved to inline `UNIQUE` on the column itself in all four files (the two backend schemas + the two migration files) so Postgres and SQLite parse identically. - mypy `external_remote.py:81`: the `credentials is None` guard CodeRabbit suggested is unreachable -- `get_credentials` returns `dict[str, str]` per protocol, never None. The empty-dict and missing-`token`-key cases are both already covered by the existing `if not token` check directly below; comment now spells out why the None-guard is unnecessary. - mypy `test_project_workspace_acceptance.py:207-208`: replaced the `asyncio.create_task(asyncio.gather(...))` shape (gather returns a Future, not a coroutine, so create_task rejects it) with two separate `asyncio.create_task` calls awaited in order. Same serialisation semantics; mypy clean. - mypy `test_protocol.py:587-599`: `_FakeProjectWorkspaceRepository` declared its entity type as `object`, which made the `isinstance(..., ProjectWorkspaceRepository)` runtime check statically unreachable. Tightened the type to `ProjectWorkspace`, matching every other Fake* in this file.
…ct-workspace # Conflicts: # scripts/_ghost_wiring_manifest.txt
Pre-push isolation gate caught a real timing regression: 7 SQLite repo tests exceed the 8.0s per-unit-test wall-clock budget by 0.4s because the migration chain grew with both this PR's project_workspaces migration and main's conversational_intake migration, and the prior `_get_template_db` only cached the migrated template per xdist worker. Under `-n 8` every worker re-ran yoyo migrations on its first persistence test (8x cost), and the chain length tipped past the budget. The fix mirrors the cross-worker pattern already used by `tests/conformance/persistence/conftest.py` for the Postgres testcontainer: a `FileLock` in `tmp_path_factory.getbasetemp().parent` (the xdist run-wide base) serialises template generation across workers. The FIRST worker to reach the fixture generates the template under the lock; the rest wait for the lock then find `template.db` already on disk and skip the migrate_apply call. `asyncio.to_thread` wraps the sync FileLock.acquire/release so the event loop stays responsive while waiting. Net cost moves from 8x migrate_apply (Windows: ~64s wall-clock spread across worker starts) to 1x migrate_apply (~8s on the generator, ~0 on every other worker). `filelock` is already in the dep tree transitively; no new deps.
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)
src/synthorg/core/enums.py (1)
407-408:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace real forge vendor names with vendor-neutral wording.
Line 407–408 uses real vendor names in project code docstrings, which violates the repo’s vendor-agnostic rule. Please switch this to neutral wording (for example, “external forge remote resolved via connection catalog”).
As per coding guidelines: “Vendor-agnostic: NEVER use real vendor names in project code/tests.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/synthorg/core/enums.py` around lines 407 - 408, The docstring in src/synthorg/core/enums.py that currently reads "delegates to a GitHub/GitLab/Gitea/Forgejo remote resolved via the connection catalog" must be made vendor-neutral; update that docstring (the enum/module-level docstring containing that phrase) to use neutral wording such as "delegates to an external forge remote resolved via the connection catalog" (or similar) so no real vendor names appear; ensure only the docstring text is changed and run quick grep/search to update any identical occurrences in the same module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/errors.md`:
- Around line 102-103: Add two new rows to the error reference table to document
the newly introduced error codes: add an entry for 3017 with the code name
CONVERSATION_NOT_FOUND and a short description like "Conversation not found" and
an entry for 7010 with the code name CONVERSATIONAL_PROPOSE_RESPONSE_INVALID and
a short description like "Proposed conversational response is invalid"; ensure
the rows follow the same pipe-delimited table format as the existing entries
(e.g., mirror the style used for 4015/4016 with code, backticked constant, and
brief description) and place them appropriately in the taxonomy table so the
client-facing docs remain canonical.
---
Outside diff comments:
In `@src/synthorg/core/enums.py`:
- Around line 407-408: The docstring in src/synthorg/core/enums.py that
currently reads "delegates to a GitHub/GitLab/Gitea/Forgejo remote resolved via
the connection catalog" must be made vendor-neutral; update that docstring (the
enum/module-level docstring containing that phrase) to use neutral wording such
as "delegates to an external forge remote resolved via the connection catalog"
(or similar) so no real vendor names appear; ensure only the docstring text is
changed and run quick grep/search to update any identical occurrences in the
same module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: cead8a93-819e-4fcf-881a-c78be78099b2
📒 Files selected for processing (32)
docs/reference/errors.mdscripts/_ghost_wiring_manifest.txtscripts/schema_drift_baseline.txtsrc/synthorg/api/app.pysrc/synthorg/api/state.pysrc/synthorg/core/enums.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/engine/workspace/push_queue.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/persistence/postgres/revisions/20260519000001_project_workspaces.sqlsrc/synthorg/persistence/postgres/schema.sqlsrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/persistence/sqlite/revisions/20260519000001_project_workspaces.sqlsrc/synthorg/persistence/sqlite/schema.sqltests/conftest.pytests/integration/engine/workspace/test_project_workspace_acceptance.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pytests/unit/integrations/test_connection_types.pytests/unit/persistence/test_protocol.pyweb/src/api/types/enum-values.gen.tsweb/src/api/types/error-codes.gen.tsweb/src/api/types/openapi.gen.tsweb/src/pages/connections/connection-type-fields.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Build Backend
- GitHub Check: Dashboard Test
- GitHub Check: Test E2E
- GitHub Check: Test Unit
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test Integration
- GitHub Check: Build Web Assets (melange)
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Lighthouse Site
- GitHub Check: Build Preview
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (12)
web/src/**/*.{js,jsx,ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{js,jsx,ts,tsx,mts}: Always usecreateLoggerfrom@/lib/logger; never bareconsole.warn/console.error/console.debugin application code. Variable name must always belog. Onlylogger.tsitself may use bare console methods. Uselog.debug()(DEV-only, stripped in production),log.warn(),log.error().
Pass dynamic/untrusted values as separate args to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Attacker-controlled fields inside structured objects must be wrapped insanitizeForLog()before embedding in log calls
Error-code constants (MANDATORY): importErrorCodeandErrorCategoryfrom@/api/types/errors(re-exported from the generatedweb/src/api/types/error-codes.gen.ts). Discriminate onErrorCode.<NAME>, never on raw integer literals.
Use@eslint-react/web-api-no-leaked-fetchto detectfetch()in effects withoutAbortControllercleanup
Files:
web/src/api/types/error-codes.gen.tsweb/src/pages/connections/connection-type-fields.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/enum-values.gen.ts
web/src/api/types/**/*.gen.ts
📄 CodeRabbit inference engine (web/CLAUDE.md)
Generated DTO types (MANDATORY): NEVER hand-edit
web/src/api/types/*.gen.ts. Regenerate withuv run python scripts/generate_dto_types_ts.py. Import DTOs via the barrel (import type { AgentConfig } from '@/api/types').
Files:
web/src/api/types/error-codes.gen.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/enum-values.gen.ts
web/src/**/*.{ts,tsx,mts}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,mts}: Use@typescript-eslint/no-floating-promisesto forbid unawaited promises so async work cannot survive the test that scheduled it and trip the active-handle gate
Use@typescript-eslint/no-misused-promises(withchecksVoidReturn: { attributes: false }) to forbid passing async functions where the callsite ignores the returned promise. React 19asyncevent handlers stay allowed via theattributes: falseexemption.
Files:
web/src/api/types/error-codes.gen.tsweb/src/pages/connections/connection-type-fields.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/enum-values.gen.ts
web/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse design tokens and components from
web/src/components/ui/; follow Web Dashboard Design System defined inweb/CLAUDE.md
Files:
web/src/api/types/error-codes.gen.tsweb/src/pages/connections/connection-type-fields.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/enum-values.gen.ts
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
React 19 web dashboard in
web/directory; seeweb/CLAUDE.mdfor detailed conventions
Files:
web/src/api/types/error-codes.gen.tsweb/src/pages/connections/connection-type-fields.tsweb/src/api/types/openapi.gen.tsweb/src/api/types/enum-values.gen.ts
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Apply no regional/currency/locale privilege; use metric units; use British English for all text
Every convention PR must ship its enforcement gate viascripts/check_*.pyor.pre-commit-config.yaml
Apply Configuration Precedence: DB > env > code default via SettingsService/ConfigResolver (Cat-1) or env > code default (Cat-2, read_only_post_init); Cat-3 bootstrap secrets are pure env at boot site. Noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value. YAML is company-template ingestion format only, not a precedence tier.
No hardcoded numeric values; numerics live insettings/definitions/. Allowlist exceptions: 0, 1, -1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants of formNAME: int|float|Final|Final[int]|Final[float] = literal. Enforced byscripts/check_no_magic_numbers.py.
Include comments explaining WHY only; no reviewer citations, issue back-references, or migration framing. Enforced bycheck_no_review_origin_in_code.py+check_no_migration_framing.py.
Do not usefrom __future__ import annotations(Python 3.14 has PEP 649). Apply PEP 758 exception: except A, B syntax without parens unless binding.
Type hints required on public functions; mypy strict mode. Use Google-style docstrings. Line length 88; functions under 50 lines; files under 800 lines.
Define errors as<Domain><Condition>Errorinheriting fromDomainError, never directly fromException/RuntimeError/etc. Enforced bycheck_domain_error_hierarchy.py.
Pydantic v2: use frozen +extra="forbid"on every frozen model project-wide (gatecheck_frozen_model_extra_forbid.py).@computed_fieldis auto-exempt; use# lint-allow: frozen-extra-forbid -- <reason>per-line forextra="allow"/"ignore"boundaries. Use@computed_fieldfor derived fields.
UseNotBlankStrfor identifier fields in Pydantic models
Args models required at every system boundary; useparse_typed()for every ...
Files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/core/error_taxonomy.pytests/unit/integrations/test_connection_types.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/api/state.pytests/conftest.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/api/app.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/service.pytests/integration/engine/workspace/test_project_workspace_acceptance.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.py
**/*.{py,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
d2for architecture and nested container diagrams; usemermaidfor flowcharts, sequence diagrams, and pipelines; use Markdown tables for tabular data
Files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/core/error_taxonomy.pytests/unit/integrations/test_connection_types.pydocs/reference/errors.mdsrc/synthorg/persistence/sqlite/backend.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/api/state.pytests/conftest.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/api/app.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/service.pytests/integration/engine/workspace/test_project_workspace_acceptance.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/core/error_taxonomy.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/api/state.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/api/app.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/service.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Timeout/slow failures indicate source-code regression; never edittests/baselines/unit_timing.jsonor anyscripts/*_baseline.{txt,json}/scripts/_*_baseline.py. Both families are PreToolUse-blocked. Per-invocation bypass for gate baselines requiresALLOW_BASELINE_GROWTH=1 git commit ...with explicit user approval.
Test markers:@pytest.mark.{unit,integration,e2e,slow}. Async tests useauto. Global timeout 30s. Minimum coverage 80%. xdist-n 8 --dist=loadfileauto-applied via pyproject addopts (loadfile prevents 3.14+Windows ProactorEventLoop leak).
Windows: unit tests useWindowsSelectorEventLoopPolicy(3.14 IOCP teardown race). Subprocess tests override back.
Use test-double ladder (see conventions.md section 12.1):FakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags. BareMagicMockat typed boundaries is blocked byscripts/check_mock_spec.py(zero-tolerance).
ImportFakeClockandmock_offromtests._shared; inject viaclock=parameter and helper's spec subscript
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)). Never skip/xfail for flaky tests; fix fundamentally. Useasyncio.Event().wait()notsleep(large).
Files:
tests/unit/integrations/test_connection_types.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/conftest.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pytests/integration/engine/workspace/test_project_workspace_acceptance.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/integrations/test_connection_types.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pytests/conftest.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pytests/integration/engine/workspace/test_project_workspace_acceptance.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.md: Read design specification fromdocs/design/page before implementing; deviations require approval per DESIGN_SPEC.md
Numeric claims in README and public documentation must be sourced fromdata/runtime_stats.yamlvia<!--RS:NAME-->markers; seedata/README.md
Files:
docs/reference/errors.md
src/synthorg/persistence/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/persistence/**/*.py: Onlysrc/synthorg/persistence/may import sqlite/psycopg or emit raw SQL. New repository protocols must inherit from one or more generic categories inpersistence/_generics.py(SingletonRepository, IdKeyedRepository, FilteredQueryRepository, AppendOnlyRepository, StatefulRepository, MVCCRepository). Bespoke methods permitted only under ADR-0001 D7 (real performance optimization or domain invariant enforcement). Maintain@runtime_checkableon protocols.
Repository CRUD methods:save(entity),get(id),delete(id) -> bool,list_items(...),query(...)returning tuples
For datetime in persistence: useparse_iso_utc/format_iso_utcfrompersistence._shared(reject naive datetimes); usenormalize_utcfor already-typed datetimes
Files:
src/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/persistence/sqlite/project_workspace_repo.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
API startup has two phases: construction (
create_appbody) wires synchronous services; on_startup (_build_lifecycle.on_startup) wires services needing connected persistence backend. Construction ordering:agent_registrybeforeauto_wire_meetings;tunnel_providerunconditionally. On-startup ordering:SettingsServicebeforeWorkflowExecutionObserver(max_subworkflow_depth),OntologyServiceafterpersistence.connect().
Files:
src/synthorg/api/state.pysrc/synthorg/api/app.py
🧠 Learnings (5)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/core/enums.pysrc/synthorg/observability/events/persistence.pysrc/synthorg/core/error_taxonomy.pytests/unit/integrations/test_connection_types.pysrc/synthorg/persistence/sqlite/backend.pytests/unit/engine/workspace/git_backend/test_embedded_backend.pysrc/synthorg/observability/events/workspace.pysrc/synthorg/api/state.pytests/conftest.pytests/unit/persistence/test_protocol.pytests/unit/engine/workspace/git_backend/test_local_path_backend.pysrc/synthorg/persistence/postgres/backend.pysrc/synthorg/engine/workspace/git_backend/local_path.pysrc/synthorg/api/app.pysrc/synthorg/engine/workspace/git_backend/external_remote.pysrc/synthorg/engine/workspace/project_workspace_service.pysrc/synthorg/engine/coordination/factory.pysrc/synthorg/engine/workspace/service.pytests/integration/engine/workspace/test_project_workspace_acceptance.pysrc/synthorg/persistence/sqlite/project_workspace_repo.pysrc/synthorg/engine/workspace/push_queue.py
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/reference/errors.md
🔇 Additional comments (41)
scripts/_ghost_wiring_manifest.txt (1)
45-46: LGTM!scripts/schema_drift_baseline.txt (1)
50-53: LGTM!Also applies to: 112-113
src/synthorg/api/app.py (1)
11-12: LGTM!Also applies to: 29-30, 93-94, 261-262, 1045-1061, 1123-1134, 1213-1273
src/synthorg/api/state.py (1)
13-14: LGTM!Also applies to: 207-215, 260-262, 348-350, 408-410, 984-1001, 1426-1431, 1447-1447, 1465-1465, 1499-1499, 1526-1638
src/synthorg/core/error_taxonomy.py (1)
93-94: LGTM!Also applies to: 111-113, 137-138
src/synthorg/engine/coordination/factory.py (1)
128-134: LGTM!Also applies to: 167-183, 196-197, 231-235, 286-288
src/synthorg/engine/workspace/git_backend/external_remote.py (2)
79-90: Past review comment addressed via protocol contract documentation.The comment at lines 79–82 clarifies that
get_credentials()returnsdict[str, str]per its protocol (neverNone), so the subsequent.get("token")is safe. The missing-token path already raisesGitBackendConfigError, which is the same outcome the prior review suggested. This is acceptable given the documented contract.
66-68: LGTM!Also applies to: 112-156, 158-188, 190-214
src/synthorg/engine/workspace/git_backend/local_path.py (2)
67-69: Past critical issues addressed: per-project isolation and non-directory handling.The
_repo_path_for_projectmethod (lines 67–69) now derives<local_repo_path>/<project_id>for each project, fixing the shared-repo issue. The_non_empty_non_repo_dirmethod (lines 71–83) now checksis_dir()before callingiterdir(), handling the non-directory case gracefully.Also applies to: 71-83
1-11: LGTM!Also applies to: 44-65, 85-118, 120-204, 206-241
src/synthorg/engine/workspace/project_workspace_service.py (4)
100-112: Past review comment addressed: path traversal rejections now logged.The logging call at lines 102–107 emits
WORKSPACE_PATH_TRAVERSAL_REJECTEDwith relevant context before raising, satisfying the audit requirement.
203-211: Past review comment addressed: re-provisioning reuses persisted workspace path.Lines 207–211 now check
if prior is not Noneand reusePath(prior.workspace_path)instead of recomputing, preventing unexpected relocation of the project tree on backend-kind switches.
115-153: LGTM!Also applies to: 213-222
1-113: LGTM!Also applies to: 155-201, 223-257
src/synthorg/engine/workspace/push_queue.py (3)
92-96: Past review comment addressed: closing flag prevents enqueues behind sentinel.The
_closingflag (lines 92–96) is set toTrueinstop()(line 113) before the sentinel is enqueued.enqueue_merge_push()checks this flag (lines 139–141) and refuses new work, preventing callers from waiting forever on items queued behind the sentinel.Also applies to: 110-113, 139-141
217-230: Past review comment addressed: WorkspacePushError subtypes preserved.Lines 217–230 now catch
WorkspacePushErrorseparately and pass the original exception toset_exception(exc), preserving forge-specific error subtypes. GenericExceptionat lines 231–244 still wraps unexpected errors in a newWorkspacePushError.
1-91: LGTM!Also applies to: 98-138, 142-216, 231-277
src/synthorg/engine/workspace/service.py (2)
87-93: Past review comment addressed: shutdown flag prevents queue resurrection.The
_shutting_downflag (lines 87–93) is flipped under_push_queues_lockinshutdown()(lines 292–295). The_get_or_create_queue()method checks this flag under the same lock (lines 232–240) and raisesWorkspaceCleanupError, preventing coordinators from being created after shutdown begins.Also applies to: 232-240, 292-295
1-86: LGTM!Also applies to: 94-231, 241-366
src/synthorg/persistence/postgres/backend.py (2)
354-354: Past review comment addressed: _project_workspaces cleared in _clear_state().Line 354 now resets
self._project_workspaces = Noneduring teardown, matching the pattern for all other repository fields.
291-291: LGTM!Also applies to: 416-416, 674-677
src/synthorg/observability/events/persistence.py (1)
300-321: LGTM!Also applies to: 634-667
src/synthorg/observability/events/workspace.py (2)
55-58: LGTM!
1-54: LGTM!Also applies to: 59-65
src/synthorg/persistence/postgres/revisions/20260519000001_project_workspaces.sql (1)
13-13: LGTM!src/synthorg/persistence/postgres/schema.sql (1)
456-456: LGTM!Also applies to: 1169-1169, 1214-1254
src/synthorg/persistence/sqlite/backend.py (1)
208-208: LGTM!Also applies to: 277-277
src/synthorg/persistence/sqlite/project_workspace_repo.py (1)
75-91: LGTM!Also applies to: 114-116, 216-218
src/synthorg/persistence/sqlite/revisions/20260519000001_project_workspaces.sql (1)
13-13: LGTM!src/synthorg/persistence/sqlite/schema.sql (1)
442-442: LGTM!Also applies to: 1091-1095, 1143-1197
tests/conftest.py (1)
3-3: LGTM!Also applies to: 546-546, 552-595
tests/integration/engine/workspace/test_project_workspace_acceptance.py (1)
158-161: LGTM!Also applies to: 168-189, 207-227, 237-240, 252-257, 277-288
tests/unit/engine/workspace/git_backend/test_embedded_backend.py (1)
98-102: LGTM!tests/unit/engine/workspace/git_backend/test_local_path_backend.py (1)
15-19: LGTM!Also applies to: 26-27, 34-35, 40-42, 55-57, 59-59, 69-72, 78-78, 85-103
tests/unit/integrations/test_connection_types.py (1)
77-99: LGTM!tests/unit/persistence/test_protocol.py (1)
41-43: LGTM!Also applies to: 75-75, 588-588, 591-591, 600-600, 1308-1319
web/src/api/types/enum-values.gen.ts (1)
43-43: LGTM!Also applies to: 624-624
web/src/api/types/error-codes.gen.ts (1)
43-43: LGTM!Also applies to: 59-60, 79-79
web/src/api/types/openapi.gen.ts (2)
2120-17517: AI summary mentions ConnectionType changes not present in the provided code.The AI-generated summary claims that
required_connection_typeandConnectionTypewere expanded to include"gitlab" | "gitea" | "forgejo", but these changes are not visible in any of the provided hunks. The visible changes all relate to the conversational propose feature (new/api/v1/meta/chat/proposeendpoint,ProposeResult,ProposedApprovalSummary, expandedApprovalSourceandSettingNamespaceenums).
2120-17517: LGTM!web/src/pages/connections/connection-type-fields.ts (1)
48-117: LGTM!
CodeRabbit (2): - docs/reference/errors.md: add CONVERSATION_NOT_FOUND (3017) and CONVERSATIONAL_PROPOSE_RESPONSE_INVALID (7010) rows so the client-facing reference matches the taxonomy. - src/synthorg/core/enums.py:407-408 (outside diff range): GitBackendType docstring referenced real forge vendor names (GitHub/GitLab/Gitea/Forgejo); the project's vendor-agnostic rule forbids that in src/ code. Replaced with "an external forge remote resolved via the connection catalog". Defensive: - tests/conftest.py _get_template_db FileLock now uses timeout=180 (matches tests/conformance/persistence/conftest.py). Without a timeout a worker that dies mid-acquire would block every peer forever; 180s bounds the wait while still covering the yoyo migration chain on cold caches with generous headroom. CI on the prior head hung at 99% in the unit job for 11 minutes before being cancelled by the workflow-level timeout. One observation without a deterministic repro is a flake; this push will give CI a second run to confirm.
…hain The pre-push isolation gate caught 3 SQLite tests that intrinsically run a full migration chain (``test_migrate_creates_tables`` calls ``backend.migrate()``, ``test_multi_tenancy_separate_databases`` calls ``migrate_apply`` directly, and the worker that owns the cross-worker template generation pays the same cost in ``test_save_and_get``). On Windows under xdist contention the chain now takes ~8.5-9s, because two new yoyo migrations landed since the 8s ceiling was calibrated (mine: ``project_workspaces``; main: ``conversational_intake``). The cross-worker FileLock in ``_get_template_db`` already collapsed the cost from 7-worker x ~8s to 1-worker x ~8s, but a single worker still pays the full chain on cold caches and there is no clever fixture trick that makes ``backend.migrate()`` itself faster. 12s covers the current chain with headroom for normal xdist contention; tests that ran in <8s still run in <8s, so the gate's regression-catching purpose is preserved (it catches *new* leaks, not the moving migration-chain floor).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/synthorg/core/enums.py`:
- Around line 407-408: Update the docstring that contains the sentence
"delegates to an external forge remote resolved via the connection catalog." to
use British English by replacing "catalog" with "catalogue" (i.e., change the
phrase to "connection catalogue"). Locate the docstring containing that exact
sentence and make this one-word spelling change so repository British-English
conventions are preserved.
In `@tests/conftest.py`:
- Line 593: Hoist the inline literal 180 used when constructing FileLock into a
module-level annotated constant (e.g., FILE_LOCK_TIMEOUT: int = 180) and replace
the inline literal in the FileLock call (where lock = FileLock(str(lock_path),
timeout=180)) with that constant; ensure the constant is declared at top of the
module so timeout tuning follows the project rule and the value is typed for
linters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4d9001e-334d-42db-890e-6045d9edd840
📒 Files selected for processing (3)
docs/reference/errors.mdsrc/synthorg/core/enums.pytests/conftest.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Deploy Preview
- GitHub Check: Build Backend
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Test Integration
- GitHub Check: Test E2E
- GitHub Check: Dashboard Test
- GitHub Check: Test Conformance (SQLite)
- GitHub Check: Test Unit
- GitHub Check: CodSpeed Web benchmarks
- GitHub Check: CodSpeed Python benchmarks
- GitHub Check: Lighthouse Site
- GitHub Check: Lighthouse Dashboard
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.md: Read design specification fromdocs/design/page before implementing; deviations require approval per DESIGN_SPEC.md
Numeric claims in README and public documentation must be sourced fromdata/runtime_stats.yamlvia<!--RS:NAME-->markers; seedata/README.md
Files:
docs/reference/errors.md
**/*.{py,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
d2for architecture and nested container diagrams; usemermaidfor flowcharts, sequence diagrams, and pipelines; use Markdown tables for tabular data
Files:
docs/reference/errors.mdsrc/synthorg/core/enums.pytests/conftest.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Apply no regional/currency/locale privilege; use metric units; use British English for all text
Every convention PR must ship its enforcement gate viascripts/check_*.pyor.pre-commit-config.yaml
Apply Configuration Precedence: DB > env > code default via SettingsService/ConfigResolver (Cat-1) or env > code default (Cat-2, read_only_post_init); Cat-3 bootstrap secrets are pure env at boot site. Noos.environ.getoutside startup; pre-init Cat-2 reads usesettings.bootstrap_resolver.resolve_init_value. YAML is company-template ingestion format only, not a precedence tier.
No hardcoded numeric values; numerics live insettings/definitions/. Allowlist exceptions: 0, 1, -1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants of formNAME: int|float|Final|Final[int]|Final[float] = literal. Enforced byscripts/check_no_magic_numbers.py.
Include comments explaining WHY only; no reviewer citations, issue back-references, or migration framing. Enforced bycheck_no_review_origin_in_code.py+check_no_migration_framing.py.
Do not usefrom __future__ import annotations(Python 3.14 has PEP 649). Apply PEP 758 exception: except A, B syntax without parens unless binding.
Type hints required on public functions; mypy strict mode. Use Google-style docstrings. Line length 88; functions under 50 lines; files under 800 lines.
Define errors as<Domain><Condition>Errorinheriting fromDomainError, never directly fromException/RuntimeError/etc. Enforced bycheck_domain_error_hierarchy.py.
Pydantic v2: use frozen +extra="forbid"on every frozen model project-wide (gatecheck_frozen_model_extra_forbid.py).@computed_fieldis auto-exempt; use# lint-allow: frozen-extra-forbid -- <reason>per-line forextra="allow"/"ignore"boundaries. Use@computed_fieldfor derived fields.
UseNotBlankStrfor identifier fields in Pydantic models
Args models required at every system boundary; useparse_typed()for every ...
Files:
src/synthorg/core/enums.pytests/conftest.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/core/enums.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Timeout/slow failures indicate source-code regression; never edittests/baselines/unit_timing.jsonor anyscripts/*_baseline.{txt,json}/scripts/_*_baseline.py. Both families are PreToolUse-blocked. Per-invocation bypass for gate baselines requiresALLOW_BASELINE_GROWTH=1 git commit ...with explicit user approval.
Test markers:@pytest.mark.{unit,integration,e2e,slow}. Async tests useauto. Global timeout 30s. Minimum coverage 80%. xdist-n 8 --dist=loadfileauto-applied via pyproject addopts (loadfile prevents 3.14+Windows ProactorEventLoop leak).
Windows: unit tests useWindowsSelectorEventLoopPolicy(3.14 IOCP teardown race). Subprocess tests override back.
Use test-double ladder (see conventions.md section 12.1):FakeClockfor Clock seam,mock_of[T](**overrides)for typed-boundary substitutions,SimpleNamespacefor attribute-bags. BareMagicMockat typed boundaries is blocked byscripts/check_mock_spec.py(zero-tolerance).
ImportFakeClockandmock_offromtests._shared; inject viaclock=parameter and helper's spec subscript
Hypothesis: 10 deterministic CI examples; failures are real bugs (fix + add@example(...)). Never skip/xfail for flaky tests; fix fundamentally. Useasyncio.Event().wait()notsleep(large).
Files:
tests/conftest.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/conftest.py
🧠 Learnings (5)
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/reference/errors.md
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.
Applied to files:
src/synthorg/core/enums.pytests/conftest.py
🔇 Additional comments (2)
docs/reference/errors.md (1)
80-80: LGTM!Also applies to: 138-138
tests/conftest.py (1)
205-213: LGTM!
CodeRabbit (2): - src/synthorg/core/enums.py:408: British English convention - "catalog" -> "catalogue" in the GitBackendType docstring (the rest of the project uses "connection catalogue" consistently). - tests/conftest.py: hoist the inline `timeout=180` FileLock literal to a module-level annotated constant `_FILE_LOCK_TIMEOUT_SECONDS: Final[int] = 180` per the project's no-bare-numeric-literals convention. Documents the 180s rationale in one place; the FileLock call site now reads as named-constant tuning. CI (pip-audit): - markdown 3.10.2 / PYSEC-2026-89 (CVE-2025-69534): HTMLParser AssertionError on malformed Markdown. markdown is a mkdocs transitive used only at docs-build time on our own trusted documentation; the framework runtime never processes attacker-controlled Markdown, so the exposure surface is nil. 3.10.2 is the latest released version (no upstream fix). - pyjwt 2.12.1 / PYSEC-2025-183 (CVE-2025-45768): "weak encryption" advisory DISPUTED by the supplier because the key length is chosen by the caller. We provide strong keys at every JWT site, so the advisory is inapplicable here. 2.12.1 is the latest released version (no upstream fix). Both vulns added to `--ignore-vuln` in `ci.yml` and `python-audit.yml` with documented justifications matching the existing pattern for GHSA-xqmj-j6mv-4862 and CVE-2026-3219.
…ct-workspace # Conflicts: # .github/workflows/ci.yml # .github/workflows/python-audit.yml
… + per-project push queue (#2021) Closes #1974. Foundational substrate for EPIC #1973 (autonomous product studio). Each project gets a 1:1 persistent git-backed working tree that survives across agents, tasks, and sessions; two agents merging concurrently are serialised through a per-project push queue; the git backend is config-only swappable across `embedded` / `local_path` / `external_remote` (with GitHub / GitLab / Gitea / Forgejo connection identities). ## Locked acceptance criteria 1. **1:1 project to persistent git-backed workspace surviving across sessions.** `ProjectWorkspaceService.get_or_provision(project_id)` materialises `<base>/projects/<project_id>/` once; the `ProjectWorkspace` row plus the on-disk working tree are the durable contract. A backend-kind change in `GitBackendConfig.kind` re-provisions under the new backend (keeping the workspace path) with a `WORKSPACE_BACKEND_KIND_CHANGED` warning event. Conformance-tested under SQLite + Postgres. 2. **Two agents merge concurrently without branch collision via a per-project serial push queue.** `WorkspaceIsolationService.merge_workspace_with_push` routes through a lazily-built `PushQueueCoordinator` per project. FIFO; sentinel-based stop; `WorkspacePushError` distinguishes a forge-rejection from a local `WorkspaceMergeError` (conflicted merge does not push). The queue sits in front of the existing `WorkspaceIsolationStrategy` so a future virtual-branch strategy slots in without architectural change. 3. **Backend is config-only swappable.** `GitBackendType` is `embedded` / `external_remote` / `local_path`; `build_git_backend(config, deps)` resolves a `GitBackend` via `StrategyRegistry`. `external_remote` resolves a saved connection from the catalogue (GitHub, GitLab, Gitea, Forgejo) and injects the token as a URL-userinfo. Deep OAuth refresh + rate-limit hardening is tracked in the follow-up issue. 4. **Validated under the simulation harness.** Acceptance tests in `tests/integration/engine/workspace/test_project_workspace_acceptance.py` cover persistence across "session restarts", concurrent merge-without-collision, config-only backend swap, and cross-project sandbox isolation. ## What ships - `ProjectWorkspace` model + `ProjectWorkspaceRepository` (sqlite + postgres) + paired `20260519000001_project_workspaces` migrations + `schema.sql` parity. FK to `projects(id) ON DELETE CASCADE`. - `engine/workspace/git_backend/`: `GitBackend` protocol, `GitBackendConfig` / `GitBackendDeps`, `EmbeddedGitBackend` (default) / `LocalPathGitBackend` / `ExternalRemoteGitBackend`, `StrategyRegistry`-driven factory with fail-fast missing-deps validation. - `engine/workspace/project_workspace_service.py` with per-project async lock, config-authoritative backend-kind precedence, path-traversal defence. - `engine/workspace/push_queue.py` (`PushQueueCoordinator`) integrated into `WorkspaceIsolationService` (per-project dict, guard lock, double-checked locking). - Connection-catalogue expansion: `GITLAB`, `GITEA`, `FORGEJO` connection types alongside `GITHUB`. Gitea + Forgejo share a `_GiteaFamilyAuthenticator` base today but stay distinct connection identities so a future API divergence does not invalidate saved connections. - Boot wiring in `_install_runtime_services` (gated by `app_state.has_persistence`); `AgentEngineExecutionService.execute_once` now `get_or_provision`s the workspace per project. New `AppState.project_workspace_service` once-only seam. - Ghost-wiring manifest: 6 `ENFORCED` entries for the new runtime symbols (`ProjectWorkspaceService`, `build_git_backend`, `EmbeddedGitBackend`, `LocalPathGitBackend`, `ExternalRemoteGitBackend`, `PushQueueCoordinator`). - Error taxonomy: `ProjectWorkspaceNotProvisionedError` (4015, CONFLICT), `GitBackend*Error` family, `WorkspacePushError`. - Docs: `docs/design/coordination.md` (persistent workspace + push-queue section), `docs/design/integrations.md` (connection types), `docs/reference/errors.md` (4014 / 4015), `docs/reference/pluggable-subsystems.md` (git backend strategy). ## Pre-PR review (this push) Ran the core agent set on the substrate; applied every valid finding before opening the PR. Highlights: - **SEC-1 redaction**: `_redact_args` strips embedded URL credentials at every `run_git_subprocess` log site (no token-in-URL ever lands in structured logs). - **`GIT_*` env sanitisation in `run_git_subprocess`**: when this code runs from inside a git pre-push hook (or any caller whose own cwd is a git working tree), the hook process exports `GIT_DIR` / `GIT_WORK_TREE` and any child git subprocess inherits them, making `is_git_repo()` report True for a freshly-mkdir'd tmp dir because git resolves against the parent repo. The fix strips `GIT_*` env vars in `run_git_subprocess` so backend strategies resolve purely via the per-call `cwd`. (Without this, the embedded backend's `git commit --allow-empty` walked up to the host's `.git` during the pre-push run; the rebased branch you're looking at has those stray commits removed and the gate now reproduces clean.) - **Defensive nested-worktree guard** on `EmbeddedGitBackend` and `LocalPathGitBackend` (compares resolved `--show-toplevel` against the requested path; only refuses when toplevel is a strict parent). - Path-traversal defence in `ProjectWorkspaceService._workspace_path` rejects `/`, `\\`, `..`. - Double-checked locking TOCTOU closed in `ProjectWorkspaceService._lock_for` and `WorkspaceIsolationService._get_or_create_queue` (dict reads moved inside the guard lock). - Push-queue logging surfaces `WORKSPACE_PUSH_QUEUE_ENQUEUED` / `_MERGED` / `_FAILED` / `_WORKER_FAILED`. ## Out of scope / follow-up Filed as #2020 (referenced, not silently deferred): - External-remote OAuth refresh + rate-limit hardening (protocol + thin glue ship now; backend-switch-is-config-only already holds). - Per-execution Docker per-project mount migration into the sandbox lifecycle owner. - `dispatch` callers of `merge_workspace` migrated onto `merge_workspace_with_push` (currently only the direct execution path uses the queue). - `PlannerWorktreeStrategy` per-project `repo_root` resolution at execute-time. - `tests/unit/api/conftest.py::test_client` baseline reset for the known `project_shared_app_once_only_install_leak`. ## Verification - `uv run python -m pytest tests/ -m unit` clean. - Targeted dual-backend conformance: `tests/conformance/persistence/test_project_workspace_repository.py` (18 tests, SQLite + Postgres). - Acceptance tests: `tests/integration/engine/workspace/test_project_workspace_acceptance.py`. - Pre-push gates: ruff, mypy strict, schema-drift (both backends), no-ghost-wiring, no-magic-numbers, isolation `--count=2` gate.
- gate.py: drop 'once the knowledge substrate ships' migration framing from module + class docstrings; extract _ELLIPSIS/_ELLIPSIS_OVERHEAD constants for excerpt truncation (no bare -3) - builder.py: log RED_TEAM_GATE_SKIPPED with seed context before raising RedTeamRuntimeSeedIncompleteError - report_repo.py: drop 'deferred to a follow-up (see plan doc)' migration framing - submit_report.py: wrap RedTeamReport(...) construction in try/except ValidationError -> RedTeamReportValidationError (no uncaught escape) - test_review_gate_red_team.py: replace 'flagged by the pre-PR review' reviewer-origin docstring with WHY-only intent Note: Test Unit CI job was CANCELLED on the prior head (exceeded 15min cap after rebase pulled in #2021 workspace tests); this push re-triggers a fresh CI run.
- gate.py: drop 'once the knowledge substrate ships' migration framing from module + class docstrings; extract _ELLIPSIS/_ELLIPSIS_OVERHEAD constants for excerpt truncation (no bare -3) - builder.py: log RED_TEAM_GATE_SKIPPED with seed context before raising RedTeamRuntimeSeedIncompleteError - report_repo.py: drop 'deferred to a follow-up (see plan doc)' migration framing - submit_report.py: wrap RedTeamReport(...) construction in try/except ValidationError -> RedTeamReportValidationError (no uncaught escape) - test_review_gate_red_team.py: replace 'flagged by the pre-PR review' reviewer-origin docstring with WHY-only intent Note: Test Unit CI job was CANCELLED on the prior head (exceeded 15min cap after rebase pulled in #2021 workspace tests); this push re-triggers a fresh CI run.
<!-- HIGHLIGHTS_START --> ## Highlights > _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub Models). Commit-based changelog below._ ### What you'll notice - Introduced conversational interface for direct clarify and propose interactions. - Cost management now includes forecast gates, hard ceilings, and Pareto considerations. - Added living documentation engine combining wiki and retrieval-augmented generation features. - Real intake engine is now operational for live data processing. - Virtual desktop tool with vision verification gate available for enhanced workspace control. ### What's new - Per-project reproducible environments for consistent setups. - Headless browser testing tool integrated for automated UI validation. - Governed external API and data access tool introduced. - Hardened external-remote git backend with sandbox mounts and push-queue dispatching. - Adversarial red-team gate subsystem for enhanced security testing. - Self-extending toolkit to dynamically expand capabilities. - Stakes-aware model routing enables prioritized processing. - Task-board entry adapter connects live runtime with project management. - Persistent project workspace with pluggable git backend and per-project push queues implemented. - Knowledge and provenance substrate added to track data lineage. - Scoring and data contract framework for golden-company benchmark evaluations. ### Under the hood - Desktop Dockerfile pinned by digest to improve build stability and documented publishing gap fixed. <!-- HIGHLIGHTS_END --> :robot: I have created a release *beep* *boop* --- ## [0.8.7](v0.8.6...v0.8.7) (2026-05-22) ### Features * conversational interface v1 - 1:1 clarify + propose ([#2019](#2019)) ([216ef94](216ef94)), closes [#1968](#1968) * cost as a first-class dial (forecast gate, hard ceiling, Pareto) ([#2029](#2029)) ([700a59e](700a59e)), closes [#1982](#1982) * **env:** reproducible per-project environments ([#2039](#2039)) ([d2c0ef9](d2c0ef9)), closes [#1994](#1994) * **evals:** [#1980](#1980) spine -- scoring + data contract for golden-company benchmark ([#2025](#2025)) ([53108e8](53108e8)) * goal/objective entry adapter ([#1964](#1964)) ([#2022](#2022)) ([cb15c3c](cb15c3c)) * governed external API/data access tool ([#1991](#1991)) ([#2032](#2032)) ([e08b451](e08b451)) * harden external-remote git backend + per-project sandbox mount + push-queue dispatch ([#2020](#2020)) ([#2030](#2030)) ([2fa2e1e](2fa2e1e)) * headless browser testing tool ([#1992](#1992)) ([#2024](#2024)) ([277b52a](277b52a)) * knowledge + provenance substrate ([#2036](#2036)) ([48c897b](48c897b)) * living documentation engine (dual-purpose wiki + RAG namespace) ([#2028](#2028)) ([3d10da9](3d10da9)), closes [#1976](#1976) * real intake engine online ([#2017](#2017)) ([9d8eb34](9d8eb34)) * **redteam:** adversarial red-team gate subsystem ([#1986](#1986)) ([#2026](#2026)) ([d2207e9](d2207e9)) * self-extending toolkit ([#1995](#1995)) ([#2035](#2035)) ([5ffc545](5ffc545)) * stakes-aware model routing ([#1998](#1998)) ([#2038](#2038)) ([9b98312](9b98312)) * task-board entry adapter to live runtime ([#1963](#1963)) ([#2023](#2023)) ([a8f1eea](a8f1eea)) * virtual desktop tool and vision verifier gate ([#2031](#2031)) ([dfe8b42](dfe8b42)), closes [#1993](#1993) * **workspace:** persistent project workspace + pluggable git backend + per-project push queue ([#2021](#2021)) ([ee58ee7](ee58ee7)) ### Bug Fixes * pin desktop Dockerfile by digest (Scorecard [#309](#309)) + document publish gap ([#2034](#2034)) ([8fda188](8fda188)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Closes #1974.
Foundational substrate for EPIC #1973 (autonomous product studio). Each project gets a 1:1 persistent git-backed working tree that survives across agents, tasks, and sessions; two agents merging concurrently are serialised through a per-project push queue; the git backend is config-only swappable across
embedded/local_path/external_remote(with GitHub / GitLab / Gitea / Forgejo connection identities).Locked acceptance criteria
ProjectWorkspaceService.get_or_provision(project_id)materialises<base>/projects/<project_id>/once; theProjectWorkspacerow plus the on-disk working tree are the durable contract. A backend-kind change inGitBackendConfig.kindre-provisions under the new backend (keeping the workspace path) with aWORKSPACE_BACKEND_KIND_CHANGEDwarning event. Conformance-tested under SQLite + Postgres.WorkspaceIsolationService.merge_workspace_with_pushroutes through a lazily-builtPushQueueCoordinatorper project. FIFO; sentinel-based stop;WorkspacePushErrordistinguishes a forge-rejection from a localWorkspaceMergeError(conflicted merge does not push). The queue sits in front of the existingWorkspaceIsolationStrategyso a future virtual-branch strategy slots in without architectural change.GitBackendTypeisembedded/external_remote/local_path;build_git_backend(config, deps)resolves aGitBackendviaStrategyRegistry.external_remoteresolves a saved connection from the catalogue (GitHub, GitLab, Gitea, Forgejo) and injects the token as a URL-userinfo. Deep OAuth refresh + rate-limit hardening is tracked in the follow-up issue.tests/integration/engine/workspace/test_project_workspace_acceptance.pycover persistence across "session restarts", concurrent merge-without-collision, config-only backend swap, and cross-project sandbox isolation.What ships
ProjectWorkspacemodel +ProjectWorkspaceRepository(sqlite + postgres) + paired20260519000001_project_workspacesmigrations +schema.sqlparity. FK toprojects(id) ON DELETE CASCADE.engine/workspace/git_backend/:GitBackendprotocol,GitBackendConfig/GitBackendDeps,EmbeddedGitBackend(default) /LocalPathGitBackend/ExternalRemoteGitBackend,StrategyRegistry-driven factory with fail-fast missing-deps validation.engine/workspace/project_workspace_service.pywith per-project async lock, config-authoritative backend-kind precedence, path-traversal defence.engine/workspace/push_queue.py(PushQueueCoordinator) integrated intoWorkspaceIsolationService(per-project dict, guard lock, double-checked locking).GITLAB,GITEA,FORGEJOconnection types alongsideGITHUB. Gitea + Forgejo share a_GiteaFamilyAuthenticatorbase today but stay distinct connection identities so a future API divergence does not invalidate saved connections._install_runtime_services(gated byapp_state.has_persistence);AgentEngineExecutionService.execute_oncenowget_or_provisions the workspace per project. NewAppState.project_workspace_serviceonce-only seam.ENFORCEDentries for the new runtime symbols (ProjectWorkspaceService,build_git_backend,EmbeddedGitBackend,LocalPathGitBackend,ExternalRemoteGitBackend,PushQueueCoordinator).ProjectWorkspaceNotProvisionedError(4015, CONFLICT),GitBackend*Errorfamily,WorkspacePushError.docs/design/coordination.md(persistent workspace + push-queue section),docs/design/integrations.md(connection types),docs/reference/errors.md(4014 / 4015),docs/reference/pluggable-subsystems.md(git backend strategy).Pre-PR review (this push)
Ran the core agent set on the substrate; applied every valid finding before opening the PR. Highlights:
_redact_argsstrips embedded URL credentials at everyrun_git_subprocesslog site (no token-in-URL ever lands in structured logs).GIT_*env sanitisation inrun_git_subprocess: when this code runs from inside a git pre-push hook (or any caller whose own cwd is a git working tree), the hook process exportsGIT_DIR/GIT_WORK_TREEand any child git subprocess inherits them, makingis_git_repo()report True for a freshly-mkdir'd tmp dir because git resolves against the parent repo. The fix stripsGIT_*env vars inrun_git_subprocessso backend strategies resolve purely via the per-callcwd. (Without this, the embedded backend'sgit commit --allow-emptywalked up to the host's.gitduring the pre-push run; the rebased branch you're looking at has those stray commits removed and the gate now reproduces clean.)EmbeddedGitBackendandLocalPathGitBackend(compares resolved--show-toplevelagainst the requested path; only refuses when toplevel is a strict parent).ProjectWorkspaceService._workspace_pathrejects/,\\,...ProjectWorkspaceService._lock_forandWorkspaceIsolationService._get_or_create_queue(dict reads moved inside the guard lock).WORKSPACE_PUSH_QUEUE_ENQUEUED/_MERGED/_FAILED/_WORKER_FAILED.Out of scope / follow-up
Filed as #2020 (referenced, not silently deferred):
dispatchcallers ofmerge_workspacemigrated ontomerge_workspace_with_push(currently only the direct execution path uses the queue).PlannerWorktreeStrategyper-projectrepo_rootresolution at execute-time.tests/unit/api/conftest.py::test_clientbaseline reset for the knownproject_shared_app_once_only_install_leak.Verification
uv run python -m pytest tests/ -m unitclean.tests/conformance/persistence/test_project_workspace_repository.py(18 tests, SQLite + Postgres).tests/integration/engine/workspace/test_project_workspace_acceptance.py.--count=2gate.