feat: add Source bindings and governed Agent Control - #45
Conversation
…urceBinding + pinned revisions
Implements ADR-0041 vertical slice: Workspace-owned Source/SourceRevision
and Project-owned SourceBinding/SourceBindingRevision, distinct from the
legacy DataSource and FeedProvider concepts (untouched). Bindings pin an
exact SourceRevision and only drift on an explicit re-pin. New RBAC-gated
endpoints under /workspaces/{id}/sources and
/workspaces/{id}/projects/{id}/source-bindings. Single new Alembic head
f3g4h5i6j7k8 on top of 1901f6da7138.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
✅ Health: 8.7 📋 At a glance 📌 Before you merge
🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (2 with dependents)"]
f_backend_api_v1___init___py[".../v1/__init__.py"]:::changed
f_backend_models___init___py["backend/models/__init__.py"]:::changed
end
f_backend_main_py["backend/main.py"]
f_backend_api_v1___init___py --> f_backend_main_py
f_backend_api_v1_workers_py[".../v1/workers.py"]
f_backend_models___init___py --> f_backend_api_v1_workers_py
f_backend_channels_opencli_channel_py["backend/channels/opencli_channel.py"]
f_backend_models___init___py --> f_backend_channels_opencli_channel_py
f_backend_config_py["backend/config.py"]
f_backend_models___init___py --> f_backend_config_py
f_backend_models___init___py --> f_backend_main_py
more(["+1 more dependent"])
PR --> more
t_tests_conftest_py(["✅ tests/conftest.py"]):::guard
t_tests_conftest_py -.-> f_backend_models___init___py
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (5 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🚨 Change risk: 9.7/10 (high)
📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-26 12:22 UTC |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds revisioned workspace sources and project bindings, exposes their APIs, and introduces centralized Agent Control governance for chat write actions with authenticated proposals, confirmations, version checks, and execution evidence. ChangesSource Management
Agent Governance
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ChatClient
participant ChatAPI
participant AgentControlService
participant OperationsWorkItem
participant ActionExecutor
ChatClient->>ChatAPI: Submit write tool request
ChatAPI->>AgentControlService: Create authenticated proposal
AgentControlService->>OperationsWorkItem: Store proposal evidence
ChatClient->>ChatAPI: Confirm proposal
ChatAPI->>AgentControlService: Execute confirmed proposal
AgentControlService->>ActionExecutor: Apply governed action
ActionExecutor-->>AgentControlService: Return execution result
AgentControlService->>OperationsWorkItem: Record confirmation and status
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
docs/verification/2026-07-26-source-agent-ere-report.md (1)
28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the verification record independently reproducible.
The report gives result counts but not the exact commands, toolchain versions, or full immutable worker revision for Codex (
2ab91e1is abbreviated). Add those details so the readiness decision can be independently rerun and audited.Also applies to: 61-72
🤖 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 `@docs/verification/2026-07-26-source-agent-ere-report.md` around lines 28 - 31, Update the verification records for both the Source/Binding V1 and Global Agent Control V1 rows to include the exact reproducible test and validation commands, relevant toolchain versions, and complete immutable worker commit hashes; replace Codex’s abbreviated 2ab91e1 with its full revision. Ensure the added details support independently rerunning and auditing the readiness decision.Source: Coding guidelines
tests/integration/test_chat_api.py (2)
34-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_authorize_chatleaves the dependency override installed.Cleanup only happens because every current caller also requests the
clientfixture, which clears overrides on teardown (tests/conftest.py Lines 73-82). A futuredb_session-only caller would silently leak the identity override into unrelated tests. Consider converting this into a fixture with explicit teardown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_chat_api.py` around lines 34 - 58, Update _authorize_chat to provide explicit teardown for the app.dependency_overrides[get_request_identity] override, preferably by converting it into a fixture that yields the authorization data and removes the override afterward. Preserve the existing identity, user, workspace setup and ensure cleanup occurs independently of the client fixture.
141-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test covers the governed metadata happy path.
This test (and the trigger_task one) posts a legacy-shape proposal, so it exercises
chat.confirm.compat. The metadata-bearing tests (Lines 264-333) both assert rejection, leaving "recorded proposal → successful confirm → RESOLVED" uncovered — the primary flow of this PR. Want me to draft that test?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_chat_api.py` around lines 141 - 172, Add a governed metadata happy-path test alongside test_confirm_update_provider that submits a recorded proposal with the required metadata rather than the legacy proposal shape, then asserts successful confirmation, provider updates, and a RESOLVED OperationsWorkItem with matching proposal and approval-grant metadata. Keep the existing legacy compatibility test unchanged and cover the corresponding successful trigger_task flow if applicable.backend/api/v1/chat.py (1)
437-453: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winLegacy-shape confirmations get no stale-target protection.
Because the proposal is created here against current state,
execute_confirmed'starget_resource_versioncheck always passes for legacy clients — they can apply a change computed from state the user never saw. The governed path (Lines 300-341) is the one that actually guards this. Consider logging thesechat.confirm.compatconfirmations at warning level and setting a removal milestone so the gap doesn't become permanent.🤖 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 `@backend/api/v1/chat.py` around lines 437 - 453, The legacy compatibility branch in the chat confirmation flow creates a proposal from current state, bypassing stale-target protection in execute_confirmed. Update the work-item_id is None path around create_proposal to log these chat.confirm.compat confirmations at warning level and add an explicit removal milestone for the legacy compatibility behavior, without changing the governed confirmation path.backend/control/agent_control.py (1)
611-625: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winGeneric-exception rollback can't undo an action that already committed.
_execute_trigger_taskcommits mid-action (Line 228). If it (or a future action doing the same) raises anything other thanCommittedActionErrorafter that commit, theexcept Exceptionbranch rolls back nothing meaningful, and the work item silently reverts toOPENwhile the side effect persists — a re-confirm would duplicate it. Consider having actions that commit declare that fact (e.g. a flag onRegisteredAction) and routing their post-commit failures through thefailed_after_commitpath unconditionally.🤖 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 `@backend/control/agent_control.py` around lines 611 - 625, Update the action execution flow around RegisteredAction and _execute_trigger_task so actions that commit mid-execution explicitly declare that behavior, and route any subsequent exception through the failed_after_commit evidence path rather than the generic rollback path. Ensure the work item is persisted as failed after commit for all declared committing actions, while retaining normal rollback handling for actions that do not commit.
🤖 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 `@backend/api/v1/chat.py`:
- Around line 157-160: Update _require_write_identity so its 401 HTTPException
includes the WWW-Authenticate header with the Bearer challenge, matching the
behavior of get_request_identity while preserving the existing detail message
and successful identity return.
In `@backend/api/v1/project_source_bindings.py`:
- Around line 77-78: Update the revision allocation flow around the for_update
branch to be safe on SQLite: use a transaction/retry-based allocation pattern
that prevents concurrent update_sequence + 1 races, or gate the single-lock path
to dialects where row locking is supported. Preserve unique revision allocation
and extend the existing tests to cover concurrent SQLite behavior.
In `@backend/api/v1/workspace_sources.py`:
- Around line 134-157: The create_source_revision flow must assign revision
numbers atomically without relying on _get_source(..., for_update=True), which
is ineffective on SQLite. Update the logic around source.current_revision_number
and SourceRevision creation to use a database-backed atomic allocation with
uniqueness/conflict protection for (source_id, revision_number), while
preserving the returned revision and source update behavior.
- Around line 54-83: Update create_source to handle IntegrityError from the
Source flush when the workspace already contains the requested slug, translating
that duplicate-slug conflict into the API’s standard 409 response instead of
allowing a raw database error; preserve normal source and revision creation
behavior for non-conflicting requests.
In `@backend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.py`:
- Around line 65-105: The source binding schema currently permits
cross-workspace bindings and revisions pinned to unrelated sources. In
backend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.py:65-105,
add database-enforced composite ownership and pinned-revision ancestry
constraints, using denormalized fields or triggers as appropriate. In
backend/models/source_binding.py:80-115, mirror those fields and relationships
so ORM writes enforce the same invariants.
In `@docs/verification/2026-07-26-source-agent-ere-report.md`:
- Around line 75-81: Revise the baseline evidence paragraph to replace “proving”
with qualified wording that only establishes the failure also occurs on the
unmodified main worktree. Add the exact RSS test name and relevant log output,
and remove the unsupported claims that production SSRF behavior was proven
unaffected or that the network assumption is valid.
- Around line 33-35: Update the committed report’s environment details at the
referenced evidence sections, including the lines around the isolated worktree
statement and line 73, to remove the private WebSocket endpoint and Windows
filesystem/process state. Replace them with a redacted ORCA/workstation
identifier and retain only the reproducible command and its result.
---
Nitpick comments:
In `@backend/api/v1/chat.py`:
- Around line 437-453: The legacy compatibility branch in the chat confirmation
flow creates a proposal from current state, bypassing stale-target protection in
execute_confirmed. Update the work-item_id is None path around create_proposal
to log these chat.confirm.compat confirmations at warning level and add an
explicit removal milestone for the legacy compatibility behavior, without
changing the governed confirmation path.
In `@backend/control/agent_control.py`:
- Around line 611-625: Update the action execution flow around RegisteredAction
and _execute_trigger_task so actions that commit mid-execution explicitly
declare that behavior, and route any subsequent exception through the
failed_after_commit evidence path rather than the generic rollback path. Ensure
the work item is persisted as failed after commit for all declared committing
actions, while retaining normal rollback handling for actions that do not
commit.
In `@docs/verification/2026-07-26-source-agent-ere-report.md`:
- Around line 28-31: Update the verification records for both the Source/Binding
V1 and Global Agent Control V1 rows to include the exact reproducible test and
validation commands, relevant toolchain versions, and complete immutable worker
commit hashes; replace Codex’s abbreviated 2ab91e1 with its full revision.
Ensure the added details support independently rerunning and auditing the
readiness decision.
In `@tests/integration/test_chat_api.py`:
- Around line 34-58: Update _authorize_chat to provide explicit teardown for the
app.dependency_overrides[get_request_identity] override, preferably by
converting it into a fixture that yields the authorization data and removes the
override afterward. Preserve the existing identity, user, workspace setup and
ensure cleanup occurs independently of the client fixture.
- Around line 141-172: Add a governed metadata happy-path test alongside
test_confirm_update_provider that submits a recorded proposal with the required
metadata rather than the legacy proposal shape, then asserts successful
confirmation, provider updates, and a RESOLVED OperationsWorkItem with matching
proposal and approval-grant metadata. Keep the existing legacy compatibility
test unchanged and cover the corresponding successful trigger_task flow if
applicable.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cf72f910-b259-4fde-846a-ebf5e560bd0d
📒 Files selected for processing (18)
backend/api/v1/__init__.pybackend/api/v1/chat.pybackend/api/v1/project_source_bindings.pybackend/api/v1/workspace_sources.pybackend/control/agent_control.pybackend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.pybackend/models/__init__.pybackend/models/source_binding.pybackend/schemas/source_binding.pydocs/backend-architecture-consolidated.mddocs/backend-capability-exposure-matrix.yamldocs/verification/2026-07-26-source-agent-ere-report.mdtests/integration/test_chat_api.pytests/integration/test_legacy_native_intelligence_migration.pytests/integration/test_legacy_plugin_migration.pytests/unit/api/test_source_binding.pytests/unit/control/test_agent_control.pytests/unit/test_migration_heads.py
| def _require_write_identity(identity: RequestIdentity | None) -> RequestIdentity: | ||
| if identity is None: | ||
| raise HTTPException(status_code=401, detail="Bearer token required for write proposals") | ||
| return identity |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
401 should carry WWW-Authenticate: Bearer.
get_request_identity sets it (backend/security/identity.py Lines 107-113); this parallel 401 doesn't, so clients get an inconsistent challenge on write proposals.
🔧 Proposed fix
- raise HTTPException(status_code=401, detail="Bearer token required for write proposals")
+ raise HTTPException(
+ status_code=401,
+ detail="Bearer token required for write proposals",
+ headers={"WWW-Authenticate": "Bearer"},
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _require_write_identity(identity: RequestIdentity | None) -> RequestIdentity: | |
| if identity is None: | |
| raise HTTPException(status_code=401, detail="Bearer token required for write proposals") | |
| return identity | |
| def _require_write_identity(identity: RequestIdentity | None) -> RequestIdentity: | |
| if identity is None: | |
| raise HTTPException( | |
| status_code=401, | |
| detail="Bearer token required for write proposals", | |
| headers={"WWW-Authenticate": "Bearer"}, | |
| ) | |
| return identity |
🤖 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 `@backend/api/v1/chat.py` around lines 157 - 160, Update
_require_write_identity so its 401 HTTPException includes the WWW-Authenticate
header with the Bearer challenge, matching the behavior of get_request_identity
while preserving the existing detail message and successful identity return.
| if for_update: | ||
| query = query.with_for_update() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python -m pip install --quiet "SQLAlchemy==2.0.0"
python - <<'PY'
from sqlalchemy import column, select, table
from sqlalchemy.dialects import postgresql, sqlite
stmt = select(table("source_bindings", column("id"))).with_for_update()
sqlite_sql = str(stmt.compile(dialect=sqlite.dialect()))
postgres_sql = str(stmt.compile(dialect=postgresql.dialect()))
print(f"SQLite: {sqlite_sql}")
print(f"PostgreSQL: {postgres_sql}")
assert "FOR UPDATE" not in sqlite_sql.upper()
assert "FOR UPDATE" in postgres_sql.upper()
PYRepository: 2233admin/opencli-admin
Length of output: 292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching project_source_bindings.py:\n'
fd -a 'project_source_bindings\.py$' . | sed 's#^\./##'
printf '\nFile excerpt:\n'
file="$(fd 'project_source_bindings\.py$' . | head -n1)"
if [ -n "$file" ]; then
nl -ba "$file" | sed -n '1,260p'
fi
printf '\nSearch for supported DB docs / sqlite references:\n'
rg -n "sqlite|SQLite|PostgreSQL|postgres|FOR UPDATE|with_for_update|revision" -S . \
-g '!*.lock' -g '!**/*.pyc' -g '!__pycache__' -g '!node_modules' -g '!dist' -g '!build' | head -n 200
printf '\nSQLAlchemy dependency/version references:\n'
rg -n "sqlalchemy|SQLAlchemy|dependency|DATABASE|DATABASE_URL" -S . \
-g 'pyproject.toml' -g 'requirements*.txt' -g 'poetry.lock' -g 'Pipfile*' -g 'environment.yml' -g '!node_modules' -g '!dist' -g '!build' | head -n 200Repository: 2233admin/opencli-admin
Length of output: 332
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'File excerpt:\n'
file="$(fd 'project_source_bindings\.py$' . | head -n1)"
python3 - <<'PY' "$file"
import sys
path = sys.argv[1]
for i, line in enumerate(open(path, encoding="utf-8"), 1):
if 1 <= i <= 260:
print(f"{i:4d}\t{line}", end="")
PY
printf '\nSQLite references and relevant docs/code:\n'
rg -n "sqlite|SQLite|DATABASE_URL|DATABASE|PostgreSQL|postgres|FOR UPDATE|with_for_update|update_sequence|revision" -S . \
-g '!*.lock' -g '!**/*.pyc' -g '!__pycache__' -g '!node_modules' -g '!dist' -g '!build' | head -n 240
printf '\nDependency references:\n'
rg -n "sqlalchemy|SQLAlchemy|Django|FastAPI|uvicorn|dependency" -S . \
-g 'pyproject.toml' -g 'requirements*.txt' -g 'poetry.lock' -g 'Pipfile*' -g 'environment.yml' -g '!node_modules' -g '!dist' -g '!build' | head -n 200Repository: 2233admin/opencli-admin
Length of output: 40890
Make revision allocation dialect-safe for SQLite.
with_for_update() is only an exclusive row lock on PostgreSQL; on SQLite it drops to no locking, so the following update_sequence + 1 write can race and hit the unique revision constraint. Use a SQLite transaction/retry-safe allocation pattern with the existing tests, or restrict the single-lock revision path to a dialect-safe backend.
🤖 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 `@backend/api/v1/project_source_bindings.py` around lines 77 - 78, Update the
revision allocation flow around the for_update branch to be safe on SQLite: use
a transaction/retry-based allocation pattern that prevents concurrent
update_sequence + 1 races, or gate the single-lock path to dialects where row
locking is supported. Preserve unique revision allocation and extend the
existing tests to cover concurrent SQLite behavior.
| @router.post("", response_model=ApiResponse[SourceRead], status_code=201) | ||
| async def create_source( | ||
| workspace_id: str, | ||
| body: SourceCreate, | ||
| identity: RequestIdentity = Depends(get_request_identity), | ||
| db: AsyncSession = Depends(get_db), | ||
| ) -> ApiResponse: | ||
| access = await get_workspace_access(db, workspace_id, identity) | ||
| require_permission(access, WorkspacePermission.MANAGE_CONFIGURATION) | ||
| source = Source( | ||
| workspace_id=workspace_id, | ||
| name=body.name, | ||
| slug=body.slug, | ||
| adapter_type=body.adapter_type, | ||
| description=body.description, | ||
| current_revision_number=1, | ||
| created_by_user_id=access.user_id, | ||
| ) | ||
| db.add(source) | ||
| await db.flush() | ||
| db.add( | ||
| SourceRevision( | ||
| source_id=source.id, | ||
| revision_number=1, | ||
| adapter_config=body.adapter_config, | ||
| created_by_user_id=access.user_id, | ||
| ) | ||
| ) | ||
| await db.flush() | ||
| return ApiResponse.ok(SourceRead.model_validate(source)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle duplicate-slug conflicts explicitly.
If (workspace_id, slug) is unique at the DB level, a duplicate slug will bubble up as an unhandled IntegrityError from db.flush() (Line 73/82), returning a raw 500 instead of a clean 409.
🐛 Proposed fix
+from sqlalchemy.exc import IntegrityError
+
`@router.post`("", response_model=ApiResponse[SourceRead], status_code=201)
async def create_source(
workspace_id: str,
body: SourceCreate,
identity: RequestIdentity = Depends(get_request_identity),
db: AsyncSession = Depends(get_db),
) -> ApiResponse:
access = await get_workspace_access(db, workspace_id, identity)
require_permission(access, WorkspacePermission.MANAGE_CONFIGURATION)
source = Source(
workspace_id=workspace_id,
name=body.name,
slug=body.slug,
adapter_type=body.adapter_type,
description=body.description,
current_revision_number=1,
created_by_user_id=access.user_id,
)
db.add(source)
- await db.flush()
- db.add(
- SourceRevision(
- source_id=source.id,
- revision_number=1,
- adapter_config=body.adapter_config,
- created_by_user_id=access.user_id,
- )
- )
- await db.flush()
+ try:
+ await db.flush()
+ db.add(
+ SourceRevision(
+ source_id=source.id,
+ revision_number=1,
+ adapter_config=body.adapter_config,
+ created_by_user_id=access.user_id,
+ )
+ )
+ await db.flush()
+ except IntegrityError as exc:
+ await db.rollback()
+ raise HTTPException(status.HTTP_409_CONFLICT, "Source slug already exists") from exc
return ApiResponse.ok(SourceRead.model_validate(source))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @router.post("", response_model=ApiResponse[SourceRead], status_code=201) | |
| async def create_source( | |
| workspace_id: str, | |
| body: SourceCreate, | |
| identity: RequestIdentity = Depends(get_request_identity), | |
| db: AsyncSession = Depends(get_db), | |
| ) -> ApiResponse: | |
| access = await get_workspace_access(db, workspace_id, identity) | |
| require_permission(access, WorkspacePermission.MANAGE_CONFIGURATION) | |
| source = Source( | |
| workspace_id=workspace_id, | |
| name=body.name, | |
| slug=body.slug, | |
| adapter_type=body.adapter_type, | |
| description=body.description, | |
| current_revision_number=1, | |
| created_by_user_id=access.user_id, | |
| ) | |
| db.add(source) | |
| await db.flush() | |
| db.add( | |
| SourceRevision( | |
| source_id=source.id, | |
| revision_number=1, | |
| adapter_config=body.adapter_config, | |
| created_by_user_id=access.user_id, | |
| ) | |
| ) | |
| await db.flush() | |
| return ApiResponse.ok(SourceRead.model_validate(source)) | |
| from sqlalchemy.exc import IntegrityError | |
| `@router.post`("", response_model=ApiResponse[SourceRead], status_code=201) | |
| async def create_source( | |
| workspace_id: str, | |
| body: SourceCreate, | |
| identity: RequestIdentity = Depends(get_request_identity), | |
| db: AsyncSession = Depends(get_db), | |
| ) -> ApiResponse: | |
| access = await get_workspace_access(db, workspace_id, identity) | |
| require_permission(access, WorkspacePermission.MANAGE_CONFIGURATION) | |
| source = Source( | |
| workspace_id=workspace_id, | |
| name=body.name, | |
| slug=body.slug, | |
| adapter_type=body.adapter_type, | |
| description=body.description, | |
| current_revision_number=1, | |
| created_by_user_id=access.user_id, | |
| ) | |
| db.add(source) | |
| try: | |
| await db.flush() | |
| db.add( | |
| SourceRevision( | |
| source_id=source.id, | |
| revision_number=1, | |
| adapter_config=body.adapter_config, | |
| created_by_user_id=access.user_id, | |
| ) | |
| ) | |
| await db.flush() | |
| except IntegrityError as exc: | |
| await db.rollback() | |
| raise HTTPException(status.HTTP_409_CONFLICT, "Source slug already exists") from exc | |
| return ApiResponse.ok(SourceRead.model_validate(source)) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 58-58: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 59-59: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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 `@backend/api/v1/workspace_sources.py` around lines 54 - 83, Update
create_source to handle IntegrityError from the Source flush when the workspace
already contains the requested slug, translating that duplicate-slug conflict
into the API’s standard 409 response instead of allowing a raw database error;
preserve normal source and revision creation behavior for non-conflicting
requests.
| @router.post( | ||
| "/{source_id}/revisions", response_model=ApiResponse[SourceRevisionRead], status_code=201 | ||
| ) | ||
| async def create_source_revision( | ||
| workspace_id: str, | ||
| source_id: str, | ||
| body: SourceRevisionCreate, | ||
| identity: RequestIdentity = Depends(get_request_identity), | ||
| db: AsyncSession = Depends(get_db), | ||
| ) -> ApiResponse: | ||
| access = await get_workspace_access(db, workspace_id, identity) | ||
| require_permission(access, WorkspacePermission.MANAGE_CONFIGURATION) | ||
| source = await _get_source(db, workspace_id, source_id, for_update=True) | ||
| next_revision = source.current_revision_number + 1 | ||
| revision = SourceRevision( | ||
| source_id=source.id, | ||
| revision_number=next_revision, | ||
| adapter_config=body.adapter_config, | ||
| created_by_user_id=access.user_id, | ||
| ) | ||
| db.add(revision) | ||
| source.current_revision_number = next_revision | ||
| await db.flush() | ||
| return ApiResponse.ok(SourceRevisionRead.model_validate(revision)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the configured production database driver/backend.
fd database.py backend --exec cat -n {}
rg -n 'DATABASE_URL|create_async_engine|postgresql|sqlite' backend --type=py -g '!**/tests/**'Repository: 2233admin/opencli-admin
Length of output: 9381
Use a SQLite-safe revision-number assignment.
DATABASE_URL defaults to SQLite, so this endpoint reaches SQLite in the configured default and with_for_update() is a no-op there. Concurrent create_source_revision calls on the same source can read the same current_revision_number, leading to duplicate or skipped revision numbers. Use a database-backed assignment such as a ROW_NUMBER()/RETURNING sequence update, insert with an UNIQUE(source_id, revision_number) conflict guard, or another atomic mechanism that does not depend on FOR UPDATE.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 141-141: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 142-142: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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 `@backend/api/v1/workspace_sources.py` around lines 134 - 157, The
create_source_revision flow must assign revision numbers atomically without
relying on _get_source(..., for_update=True), which is ineffective on SQLite.
Update the logic around source.current_revision_number and SourceRevision
creation to use a database-backed atomic allocation with uniqueness/conflict
protection for (source_id, revision_number), while preserving the returned
revision and source update behavior.
| op.create_table( | ||
| "source_bindings", | ||
| sa.Column("project_id", sa.String(36), nullable=False), | ||
| sa.Column("source_id", sa.String(36), nullable=False), | ||
| sa.Column("name", sa.String(255), nullable=False), | ||
| sa.Column("slug", sa.String(100), nullable=False), | ||
| sa.Column( | ||
| "status", | ||
| sa.Enum("active", "disabled", "revoked", name="source_binding_lifecycle_status"), | ||
| nullable=False, | ||
| ), | ||
| sa.Column("current_revision_number", sa.Integer(), nullable=False), | ||
| sa.Column("created_by_user_id", sa.String(36), nullable=False), | ||
| sa.Column("id", sa.String(36), primary_key=True), | ||
| sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), | ||
| sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), | ||
| sa.ForeignKeyConstraint(["project_id"], ["projects.id"], ondelete="CASCADE"), | ||
| sa.ForeignKeyConstraint(["source_id"], ["sources.id"], ondelete="RESTRICT"), | ||
| sa.ForeignKeyConstraint(["created_by_user_id"], ["users.id"], ondelete="RESTRICT"), | ||
| sa.UniqueConstraint("project_id", "slug"), | ||
| ) | ||
| op.create_index("ix_source_bindings_project_id", "source_bindings", ["project_id"]) | ||
| op.create_index("ix_source_bindings_source_id", "source_bindings", ["source_id"]) | ||
|
|
||
| op.create_table( | ||
| "source_binding_revisions", | ||
| sa.Column("source_binding_id", sa.String(36), nullable=False), | ||
| sa.Column("revision_number", sa.Integer(), nullable=False), | ||
| sa.Column("pinned_source_revision_id", sa.String(36), nullable=False), | ||
| sa.Column("scope_config", sa.JSON(), nullable=False), | ||
| sa.Column("created_by_user_id", sa.String(36), nullable=False), | ||
| sa.Column("id", sa.String(36), primary_key=True), | ||
| sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), | ||
| sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), | ||
| sa.ForeignKeyConstraint( | ||
| ["source_binding_id"], ["source_bindings.id"], ondelete="CASCADE" | ||
| ), | ||
| sa.ForeignKeyConstraint( | ||
| ["pinned_source_revision_id"], ["source_revisions.id"], ondelete="RESTRICT" | ||
| ), | ||
| sa.ForeignKeyConstraint(["created_by_user_id"], ["users.id"], ondelete="RESTRICT"), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Enforce binding provenance at the database boundary.
Independent foreign keys allow direct ORM/admin/worker writes to create a binding whose Project and Source belong to different workspaces, or a binding revision pinned to a revision from a different Source. That violates the stated ownership and explicit-pin contracts even though this API path validates them.
backend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.py#L65-L105: add database-enforced workspace ownership and pinned-revision/source ancestry constraints (for example, composite keys with denormalized ownership fields or triggers).backend/models/source_binding.py#L80-L115: mirror the added constraint fields/relationships so ORM writes cannot bypass the same invariants.
📍 Affects 2 files
backend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.py#L65-L105(this comment)backend/models/source_binding.py#L80-L115
🤖 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 `@backend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.py`
around lines 65 - 105, The source binding schema currently permits
cross-workspace bindings and revisions pinned to unrelated sources. In
backend/migrations/versions/f3g4h5i6j7k8_add_source_and_source_binding.py:65-105,
add database-enforced composite ownership and pinned-revision ancestry
constraints, using denormalized fields or triggers as appropriate. In
backend/models/source_binding.py:80-115, mirror those fields and relationships
so ORM writes enforce the same invariants.
| Both lanes ran in isolated clean worktrees on the saved ORCA environment | ||
| `5090` (`ws://100.80.105.128:6768`) from the same `origin/main` baseline. The | ||
| dirty 5090 `main` worktree and the dirty local root worktree were not modified. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate and inspect target file =="
if [ -f docs/verification/2026-07-26-source-agent-ere-report.md ]; then
wc -l docs/verification/2026-07-26-source-agent-ere-report.md
sed -n '1,100p' docs/verification/2026-07-26-source-agent-ere-report.md | cat -n
else
echo "target file not found"
fi
echo
echo "== Search for potentially private endpoint/path-like details in verification docs =="
rg -n "100\.80\.105\.128|6768|ws://|5090|dirty|worktree|worktrees|source-agent" docs/verification -S || trueRepository: 2233admin/opencli-admin
Length of output: 7079
Remove workstation-specific evidence details from the committed report.
Lines 33-35 and 73 leak a private local WebSocket endpoint plus Windows filesystem path/process state; these are unstable evidence and unnecessarily disclose environment details. Replace them with a redacted ORCA/workstation identifier plus the reproducible command/result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/verification/2026-07-26-source-agent-ere-report.md` around lines 33 -
35, Update the committed report’s environment details at the referenced evidence
sections, including the lines around the isolated worktree statement and line
73, to remove the private WebSocket endpoint and Windows filesystem/process
state. Replace them with a redacted ORCA/workstation identifier and retain only
the reproducible command and its result.
| The first complete non-live run reported 2491 passed and 12 failed. Two failures | ||
| were stale assertions for the previous migration head and were fixed. The other | ||
| 10 failures came from this machine resolving `example.com` to the reserved | ||
| benchmark address `198.18.1.6`, which the production SSRF guard correctly | ||
| rejects. The same RSS success test fails identically on the unmodified main | ||
| worktree, proving this is a local test-environment dependency rather than an | ||
| integration regression. The SSRF protection was not weakened. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file context =="
sed -n '1,140p' docs/verification/2026-07-26-source-agent-ere-report.md
echo
echo "== searches for affected test names/logs in repo =="
rg -n "example\.com|198\.18\.1\.6|RSS|rss|ssrf|SSRF|source-agent|ere" . -S --glob '!node_modules' --glob '!dist' --glob '!build' 2>/dev/null | head -200Repository: 2233admin/opencli-admin
Length of output: 28471
Record the RSS baseline evidence instead of overstating SSRF safety.
The baseline reproduction shows the failure predates this branch, but reword “proving” and include the exact RSS test name/log. That comparison still does not prove the production SSRF behavior is unaffected or that the network assumption is valid.
🤖 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 `@docs/verification/2026-07-26-source-agent-ere-report.md` around lines 75 -
81, Revise the baseline evidence paragraph to replace “proving” with qualified
wording that only establishes the failure also occurs on the unmodified main
worktree. Add the exact RSS test name and relevant log output, and remove the
unsupported claims that production SSRF behavior was proven unaffected or that
the network assumption is valid.
1 participant
Integrates the ORCA/5090 Claude and Codex lanes for Workspace Source/Project SourceBinding V1 and the unified Agent Control write path. Includes capability-ledger reconciliation, migration compatibility fixes, stale-proposal coverage, and the ERE report at docs/verification/2026-07-26-source-agent-ere-report.md. Verification: 50 related tests, 6 migration tests, ruff, mypy, capability generator check, SQLite upgrade-downgrade-upgrade, and 2489 non-live tests passed after excluding three RSS files affected by the workstation DNS policy.