Skip to content

feat(server): namespace scoping and control bindings#203

Open
abhinav-galileo wants to merge 34 commits intomainfrom
abhi/data-model-v1
Open

feat(server): namespace scoping and control bindings#203
abhinav-galileo wants to merge 34 commits intomainfrom
abhi/data-model-v1

Conversation

@abhinav-galileo
Copy link
Copy Markdown
Collaborator

@abhinav-galileo abhinav-galileo commented Apr 27, 2026

Summary

Adds the namespace-scoping data model and a single merged effective-controls contract that initAgent, GET /agents/{name}/controls, and POST /evaluation all share.

  • namespace_key VARCHAR(255) NOT NULL DEFAULT 'default' on agents, controls, policies, agent_controls, agent_policies, policy_controls, control_bindings.
  • Single-column uniqueness replaced by namespace-scoped composite uniqueness; association-table foreign keys are composite same-namespace foreign keys (Postgres-enforced).
  • New control_bindings table for attaching controls to opaque external targets. One binding shape: each row attaches one control to one target inside a namespace, uniqueness on (namespace_key, target_type, target_id, control_id). The enabled flag is a soft toggle - disabled bindings are preserved but excluded from the effective set.
  • Single merged resolver. ControlService.list_controls_for_agent (and its runtime cousin) returns the de-duplicated union of the agent's direct controls, policy-derived controls, and (when target context is supplied) controls attached to that target via enabled bindings in the same namespace. initAgent, GET /agents/{name}/controls?target_type=...&target_id=..., and POST /evaluation all call into this resolver and return the same set for the same inputs.
  • Management API (/control-bindings): full CRUD plus idempotent natural-key upsert/delete (PUT /control-bindings/by-key, POST /control-bindings/by-key:delete). Cursor-based pagination on list. Natural key is (target_type, target_id, control_id).
  • initAgent accepts optional top-level target_type / target_id. Bindings can pre-exist the agent row, so a newly created agent registering with target context picks up pre-existing bindings on its first response (no second round-trip).
  • Python SDK target context is fixed per session: init(target_type=..., target_id=...) stores it on state and forwards it on the registration call and on every subsequent /agents/{name}/controls poll. The single existing policy refresh loop carries the merged set; there is no separate target controls cache or refresh worker.
  • check_evaluation, check_evaluation_with_local, and evaluate_controls default per-call target context from state and reject mismatches with a clear ValueError. The cached controls were fetched for the session target; accepting a mismatched override would drive stale local-first evaluation.

Namespace scoping

Every effective-controls query filters every joined table on namespace_key explicitly. Composite FKs prevent cross-namespace writes; explicit query scoping prevents reads from spanning namespaces in the presence of namespace-collision attacks or compromised callers. Both layers are required.

V1 ships namespace plumbing at the schema level. Endpoints route through a single get_namespace_key dependency that always returns the default namespace; overriding it is not supported in V1 because controls/agents/policies endpoints still write under the default namespace, and an override here would create rows the existing endpoints cannot find. A follow-up will thread the resolver through every write path together.

OSS / single-namespace deployments are preserved by the 'default' server default. Plain ix_agents_name, ix_policies_name, and ix_controls_name (partial on deleted_at IS NULL) indexes preserve name-only lookup performance during the rollout window.

The migration is reversible. downgrade() aborts with a clear error if cross-namespace duplicate names exist on agents, policies, or live controls, since restoring global single-column uniqueness would conflict. Soft-deleted control duplicates do not block downgrade.

V1 contract

SDK init target is fixed per session.
initAgent and GET /agents/{name}/controls return the effective controls for that session context.
Runtime /evaluation uses the same merged resolution.
Dynamic per-request target switching, inheritance, DAGs, and target-agent overrides remain out of scope.

The SDK supports one target per process. CrewAI-style multi-agent crews already use one init() per process and route per-tool policy via step_name; the same model carries over to target context.

Notes on control_bindings

  • ON DELETE CASCADE on the parent control fires only on hard deletes. Soft-deleted controls (deleted_at IS NOT NULL) keep their bindings; the resolver excludes soft-deleted controls.
  • updated_at refreshes on every UPDATE via SQLAlchemy onupdate.
  • A (namespace_key, control_id) index covers the cascade path and list_bindings(control_id=...) filtering.
  • idx_controls_namespace_name_active is recognized as a name-conflict constraint, so concurrent duplicate-name races surface as 409, not 500.
  • Per-agent overrides and exemptions within a target are intentionally out of scope at this stage. Two forward paths are documented in code (migration comment + ControlBinding docstring) and in the design doc:
    • re-introduce an agent_name column with a partial-index pair and an enabled-aware most-specific-wins resolver. Supports both per-agent additions and per-agent exemptions.
    • or merge target-bearing resolution with the existing agent_controls table at runtime. Supports per-agent additions only; exemptions still need a schema change because agent_controls has no enabled flag.

Out of scope (follow-up PRs)

  • Threading get_namespace_key through controls/agents/policies endpoints and services so per-request namespace resolution is honored end-to-end. V1 honors it only on read paths.
  • Auth-derived get_namespace_key resolution.
  • Namespace scoping for control_versions and control_execution_events.
  • Per-agent overrides and exemptions within a target (see forward paths above).

Test plan

  • Migration: upgrade applies columns/constraints/indexes; control_bindings table created; downgrade restores originals; upgrade/downgrade round-trip; downgrade rejects cross-namespace duplicates on agents, policies, and live controls; allows soft-deleted duplicates.
  • Isolation: same name across namespaces allowed; same name within a namespace rejected; soft-deleted control names reusable within a namespace; cross-namespace foreign keys rejected on every association table.
  • control_bindings table: same control bindable to different targets; duplicate (namespace, target_type, target_id, control_id) rejected; cross-namespace control_id rejected; ON DELETE CASCADE on hard delete; bindings survive parent soft delete.
  • Merged resolver: target bindings union with direct + policy controls; de-duplication when a control is attached through both paths; disabled binding excluded; soft-deleted controls excluded; namespace isolation; absence of target context omits bindings.
  • initAgent: target params merge into the returned controls; newly created agent with target context picks up pre-existing bindings; partial target pair rejected (422).
  • GET /agents/{name}/controls?target_type=...&target_id=... returns the same merged set as initAgent; partial target pair rejected (400).
  • /evaluation: target context flows through the same merged resolver (target + agent + policy); 404 when the agent is not registered; partial target pair rejected (422).
  • Management API: create / get / list (cursor pagination) / patch / delete; non-admin write rejection; natural-key upsert (idempotent, updates enabled and updated_at, handles concurrent insert race); natural-key delete (idempotent).
  • Control-name conflict mapping: both legacy and new partial-unique index names trigger CONTROL_NAME_CONFLICT (409) instead of 500.
  • SDK: init(target_type=..., target_id=...) stores session target and forwards it on registration and on every refresh; partial target pair rejected; per-call target overrides default from state and reject mismatches with the session target.
  • Full server test suite: 621 tests pass.
  • SDK test suite: 539 tests pass.
  • Models tests: 64 pass.
  • TS SDK tests: 8 pass.
  • make lint clean.
  • make typecheck clean.
  • make sdk-ts-generate-check clean.

Adds a namespace_key column to agents, controls, policies, and the three
association tables. Replaces single-column uniqueness with
namespace-scoped composite uniqueness, and converts association-table
foreign keys to composite same-namespace foreign keys.

Adds a control_bindings table for attaching controls to opaque external
targets, with an optional agent_name selector for narrower overrides
inside a target. Two binding shapes are supported via partial unique
indexes: target-default (agent_name IS NULL) and target-agent.

OSS and single-namespace deployments are preserved by the 'default'
server default on every namespace_key column. Existing endpoint and
service code is unchanged; default-namespace behavior is fully
backward compatible.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

… indexes

- control_bindings.id: change migration column type from BigInteger to
  Integer to match the ORM model and the convention of every other id
  column in the schema.
- control_bindings.agent_name: change column type from Text to
  String(255) and add a check constraint requiring NULL or the same
  format/length as agents.name. Bindings may still predate agent
  registration; callers must normalize before insert.
- Add plain natural-key indexes ix_agents_name and ix_policies_name to
  preserve name-only lookup performance while service code is still
  namespace-blind. The new composite primary keys and unique constraints
  lead with namespace_key, so name-only queries no longer have a
  leading-column index without these.
- Document that soft deletes on a control do not cascade to bindings
  (the runtime resolver excludes soft-deleted controls).
- Add tests for the soft-delete survival path and malformed-agent_name
  rejection.
Adds ControlBindingsService.resolve_effective_controls, which returns the
active control set for a target-bearing request. Two binding shapes are
considered: target-default (agent_name IS NULL) and target-agent. For each
control_id, the most-specific binding wins (target-agent beats
target-default); a winning binding with enabled=False excludes the control.
Soft-deleted controls are filtered out.
Adds CRUD endpoints under /control-bindings backed by ControlBindingsService.

- PUT  /control-bindings        create binding (admin)
- GET  /control-bindings        list with optional target/agent/control filters
- GET  /control-bindings/{id}   single-binding detail
- PATCH /control-bindings/{id}  toggle enabled (admin)
- DELETE /control-bindings/{id} delete binding (admin)

Adds CONTROL_BINDING_NOT_FOUND and CONTROL_BINDING_CONFLICT error codes,
the matching request/response Pydantic types, and a get_namespace_key
dependency that returns the default namespace and is overridable for
deployment-specific namespace resolution.

Service create/update/delete enforce same-namespace integrity by checking
the parent control belongs to the request's namespace; uniqueness
violations are translated into 409 conflicts.
EvaluationRequest gains optional target_type/target_id fields. When both
are supplied the evaluation endpoint resolves the effective control set
from control_bindings (no agents row required); otherwise it uses the
existing agent-attached path. The two paths do not silently merge.

Adds ControlBindingsService.resolve_runtime_controls and a shared
parse_runtime_controls helper to avoid duplicating the Control to
RuntimeControl conversion across services.
Adds idempotent attach/detach endpoints addressed by the natural key
(target_type, target_id, agent_name?, control_id):

- PUT  /control-bindings/by-key       upsert (creates or updates enabled)
- POST /control-bindings/by-key:delete delete (returns deleted=False if missing)

Useful for callers that want to attach a control without first checking
whether a binding already exists. Backed by ControlBindingsService.upsert_by_natural_key
and delete_by_natural_key.
Adds optional target_type/target_id parameters to evaluate_controls,
check_evaluation, and check_evaluation_with_local. When supplied, both
fields are included on the EvaluationRequest sent to the server, which
routes the request through the target-bearing resolution path.

Both fields must be supplied together; the server enforces this via the
EvaluationRequest model validator.
- SDK target-bearing requests now bypass cached agent-attached controls
  and call the server unconditionally. The cached controls (from
  initAgent) are agent-attachment data; target-bearing requests must
  resolve from control_bindings only, which the server enforces, but
  the SDK was previously short-circuiting against the cache when no
  applicable server controls were present.
- agent_name on control-binding requests is normalized and validated
  at the API boundary using the same rules as agents.name. Mixed-case
  or whitespace-padded values are accepted and normalized; values that
  fail the format/length rules are rejected with 422 instead of leaking
  to the database check constraint as 500/conflict.
- ControlBinding.updated_at is refreshed on UPDATE via SQLAlchemy
  onupdate. PATCH /control-bindings/{id} and idempotent natural-key
  upserts now reflect the updated timestamp on subsequent reads.
Each binding row now attaches one control to one target inside a
namespace. Per-agent overrides and exemptions within a target are out of
scope at this stage; both the migration and the ControlBinding model
docstring document the two forward paths if and when those become a
product requirement (re-add agent_name with a partial-index pair, or
merge target-bearing resolution with agent_controls).

Net simplifications:

- One unique constraint instead of a partial-index pair on
  (agent_name IS NULL / IS NOT NULL).
- No agent-name CHECK constraint or normalize_optional_agent_name
  validator on binding requests.
- Resolver returns the target-level control set directly; no most-
  specific-wins logic, no winners dict.
- Pydantic request/response models lose the agent_name field; the
  list endpoint loses the agent_name query parameter.
- SDK target-bearing path is unchanged (it never carried agent_name on
  bindings).

Schema/code/test/doc all stay aligned. Migration round-trip verified
locally; full server suite passes (601 tests), lint clean, typecheck
clean.
…Request

The class docstring still described the upsert natural key as
(target_type, target_id, agent_name, control_id). Update to match the V1
shape: (target_type, target_id, control_id).
@abhinav-galileo abhinav-galileo marked this pull request as ready for review April 28, 2026 16:00
…oints

Adds the auto-generated bindings for the new /control-bindings surface
(create, list, get, patch, delete, upsert-by-key, delete-by-key) and
refreshes the evaluation models/sdk to include the optional
target_type / target_id fields. Also adds the method-name overrides
under sdks/typescript/overlays/method-names.overlay.yaml.
Copy link
Copy Markdown
Contributor

@lan17 lan17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work here. The overall direction makes sense, but I found a couple issues that should be fixed before merge.

One P1 could not be placed inline because it points at an unchanged file:

[P1] Control deletion ignores target bindings (server/src/agent_control_server/endpoints/controls.py:921-925)

delete_control only checks policy and agent associations before soft-deleting a control. A control that is actively attached through control_bindings can be deleted with force=false, which disables protection for that target while leaving the binding row behind. The lifecycle/in-use checks should include target bindings, and force=true should have explicit binding semantics.

The rest of the findings are inline.

Comment thread server/src/agent_control_server/endpoints/evaluation.py Outdated
Comment thread server/src/agent_control_server/services/control_bindings.py
Comment thread server/src/agent_control_server/services/control_bindings.py
Adds GET /api/v1/control-bindings/effective which returns the effective
control set for a target in the same shape as InitAgentResponse.controls.
The Python SDK now fetches this list when a request is target-bearing
and runs it through the existing local-vs-server execution split, so
controls with execution='sdk' bound to a target run locally instead of
being silently dropped on the server side.

Adds the EffectiveTargetControlsResponse model, regenerates the
TypeScript client to include the new endpoint, and adds endpoint-level
tests covering the bound-controls, disabled-binding, and empty-result
cases plus an SDK test that exercises the local-eval path end-to-end.

The previous _evaluate_target_bearing short-circuit is gone; target-
bearing requests now go through check_evaluation_with_local with the
target-bound controls in place of the cached agent set.
delete_control previously only inspected agent and policy associations,
so a control attached via control_bindings could be soft-deleted with
force=false. The binding row would remain pointing at a deleted control,
silently disabling protection on the target.

The lifecycle check now also lists target bindings: force=false rejects
deletion with CONTROL_IN_USE listing the binding IDs, and force=true
removes the bindings before soft-deleting the control. The detached
binding IDs are surfaced in the response under detached_target_bindings.
upsert_by_natural_key previously did SELECT then INSERT, so two
concurrent calls for the same (namespace_key, target_type, target_id,
control_id) could both miss the existing row, then one would hit the
unique-constraint violation and surface as an unhandled IntegrityError
(500) even though the endpoint is documented as idempotent.

The loser of the race now catches IntegrityError, rolls back its
insert, re-reads the winning row, and applies its requested enabled
value as an update. Both calls return successfully; the create flag is
true only for the caller whose insert actually wrote the row.
list_bindings previously returned every binding in the namespace, which
grows linearly with attached targets. Switched to cursor-based pagination
matching the existing list_controls_page idiom: cursor + limit query
params, results ordered by ID descending (newest first), pagination
metadata returned alongside the binding list.

ListControlBindingsResponse now carries a PaginationInfo block (limit,
total, next_cursor, has_more). Default limit is 20, max 100, mirroring
the controls list endpoint. The TypeScript client is regenerated to
include the new query params.
The * marker before target_type/target_id also made the previously
positional-or-keyword arguments trace_id, span_id, and event_agent_name
keyword-only. External callers passing them positionally would break
with TypeError.

Move * to immediately before target_type so the new fields are
keyword-only while existing arguments retain their positional contract.
…gle query

Add (namespace_key, control_id) index used by list_bindings(control_id=...)
and the cascade path from controls. Collapse resolve_effective_controls
into a single JOIN against control_bindings instead of two round trips.
Match the existing String(255) convention used for agent_name and
control/policy names. Bounds index key size and prevents pathological
long values across all namespace-scoped tables.
The downgrade refuses to run when cross-namespace duplicates exist on
agents, policies, or live controls; only the agents path was tested.
Add policies and live-controls cases plus a soft-deleted positive case
that verifies the deleted_at filter still allows downgrade.
Spell out that integrators can declare any FastAPI-resolvable dependency
in their override signature, and include a JWT-claim example so the
extension point is concrete instead of abstract.
Two related gaps left by the namespace migration:

- Add idx_controls_namespace_name_active to the duplicate-name conflict
  set so concurrent name collisions surface as 409 instead of 500.
  Parametrize the IntegrityError tests across both index names.
- Restore a non-unique partial index ix_controls_name on
  controls(name) WHERE deleted_at IS NULL, mirroring the natural-key
  indexes added for agents and policies. Existing service code still
  does name-only Control lookups; without this index those go to a
  sequential scan post-migration.
Endpoints used a Depends(get_namespace_key) seam, but only the binding
and evaluation endpoints honored it; controls/agents/policies still
wrote and queried under the DB default. Overriding the seam in a
deployment broke binding creation with CONTROL_NOT_FOUND.

Drop the seam for V1: endpoints resolve to DEFAULT_NAMESPACE_KEY
directly. Schema and services remain namespace-scoped so a future
change can thread a single resolver through every write path together.
f90f8f0 dropped the seam, but a single resolver function on the read
side is still useful: every namespace-scoped endpoint funnels through
one call site, so a future change can switch every reader to a real
per-request resolver in one place. V1 returns DEFAULT_NAMESPACE_KEY
unconditionally and documents that overriding is unsupported until
controls/agents/policies endpoints are threaded.
Mirrors the agent-bound flow: a per-target LRU cache populated lazily
on first evaluation, kept fresh by a daemon thread that refetches
each cached entry on a fixed interval. Public API parallels the
agent-bound side: refresh_target_controls / refresh_target_controls_async
for explicit refresh, invalidate_target_controls_cache for explicit drop.
init() takes target_controls_refresh_interval_seconds (default 60s,
0 disables); shutdown stops the loop alongside the policy refresh loop.
A process-wide LRU cache keyed only by (target_type, target_id) cannot
distinguish entries written under different SDK sessions. Re-initing
against a different server or API key would have served controls from
the previous identity until the entry was evicted or overwritten,
including races where an in-flight refresh lands after init() proceeds.

Tie cache lifetime to the session boundary: init() and shutdown call
cache.reset(), which both clears every entry and advances an internal
epoch token. Writers capture the epoch before fetching and pass it to
put(); writes whose epoch no longer matches are rejected silently.
Both write paths (refresh worker and lazy fetch) are covered.
check_evaluation_with_local accepts an arbitrary AgentControlClient.
Reusing the global cache for a client whose base_url or api_key does
not match the active SDK session would let controls fetched against
one server serve evaluations against another. Skip the cache entirely
in that case and fetch live; only the init()-managed session populates
or reads the shared cache.
Patch coverage on the previous push fell below the repo target. The
gap was concentrated in two places, both genuinely useful to test:

- SDK target-controls polling and refresh APIs:
  - invalidate_target_controls_cache (single-key, both args, no args)
  - refresh_target_controls_async (empty cache, multi-key fetch,
    per-target failure isolation, stale-write rejection after reset)
  - refresh_target_controls (sync wrapper from sync and async contexts)
  - _start_target_controls_refresh_loop / _stop_target_controls_refresh_loop
    (round trip, zero-interval no-op)
  - _target_controls_refresh_worker (cache-empty short-circuit,
    refresh-known-keys integration, session-unset skip, exception
    isolation)

- ControlBindingsService.upsert_by_natural_key IntegrityError branch:
  the existing test exercised the SELECT-then-UPDATE fast path; the
  new test simulates the race where both transactions miss on SELECT,
  one INSERT trips the unique index, and the loser must roll back,
  re-fetch the winner, and apply the requested enabled value.
@lan17
Copy link
Copy Markdown
Contributor

lan17 commented Apr 29, 2026

I think there is still a broader contract issue here: both initAgent and GET /agents/{agent}/controls should return all controls assigned to the agent by any means, not just direct agent links and policy-derived controls. With the new target-binding mechanism, those two agent control surfaces should include controls that are effectively assigned through target bindings as well, otherwise the SDK/API can report an incomplete control set for the agent while target-bearing evaluation sees a different set.

initAgent, GET /agents/{name}/controls, and POST /evaluation now resolve
the same de-duplicated effective set: direct attachments, policy-derived
controls, and (when target context is supplied) controls bound to that
target via enabled bindings in the same namespace.

Server
- Add optional target_type/target_id to InitAgentRequest with paired-target
  validation. initAgent merges target bindings into its returned controls
  and no longer short-circuits to an empty list on agent creation, so a
  newly registered agent picks up pre-existing bindings.
- list_agent_controls accepts target_type/target_id; the GET endpoint
  enforces the paired-target rule and threads namespace_key through.
- ControlService.list_controls_for_agent / list_runtime_controls_for_agent
  / _list_db_controls_for_agent now require namespace_key and accept
  optional target params; every joined table (agent_controls,
  agent_policies, policy_controls, ControlBinding, Control) is filtered
  on namespace_key.
- /evaluation collapses to one resolver: ControlService.list_runtime_controls_for_agent
  with namespace + target. The agent row is required on every request.
- Drop GET /control-bindings/effective and the
  ControlBindingsService.resolve_effective_controls /
  resolve_runtime_controls helpers; the merged ControlService path is
  authoritative.

SDK (Python)
- init() takes target_type/target_id; both must be supplied together.
  The values flow into state and ride on the registration call and on
  every subsequent /agents/{name}/controls poll.
- Drop the per-target controls cache, refresh worker, refresh API, and
  invalidate API; one polling loop and one publish path remain.
- check_evaluation, check_evaluation_with_local, and evaluate_controls
  default target_type/target_id from state when omitted.
- _reset_state and shutdown clear target context.
- agents.register_agent / list_agent_controls forward target params.

Drops EffectiveTargetControlsResponse from the model exports and
regenerates the TypeScript SDK against the new spec.
…rget

The session control cache (state.server_controls) is fetched for the
target context fixed at init() time. A per-call target override that
disagrees with the session target would drive local-first evaluation
against the wrong cached set and could return safe without contacting
the server.

The V1 contract is one target per SDK session. Add a shared
_resolve_session_target helper that defaults missing per-call targets
from state and rejects mismatches with a clear ValueError pointing
callers at re-init. Apply at all three entry points (check_evaluation,
check_evaluation_with_local, evaluate_controls) so the contract is
uniform across the public API.

Also update EvaluationRequest's target_type/target_id descriptions: the
server now merges target bindings into the agent + policy effective
set rather than resolving from bindings alone. The TypeScript SDK
regenerates the new wording.
A session initialized via init() without target context still has a
control cache fetched for that no-target context. The previous mismatch
check only fired when state.target_type was set, so a caller could pass
target_type/target_id per call on a no-target session and have those
values accepted - then evaluate against the wrong cache, potentially
returning safe without contacting the server.

Treat (None, None) as a valid session target. Use state.current_agent
as the active-session sentinel so the rule applies inside an init()'d
session (any per-call target must equal the session target) but is
skipped outside one (lower-level direct-client flows still work).

Add a regression test covering the no-target session path and update
the existing test_per_call_target_must_match_session_target to model
an active session by patching state.current_agent.
Runnable example showing the V1 contract end-to-end:

- init(target_type='env', target_id='prod') returns the merged effective
  set (agent's direct attachments + bindings for the supplied target).
- @control() decorator runs against that merged set automatically.
- evaluate_controls(...) defaults its target context from the session.
- A per-call target that disagrees with the session target is rejected
  with a clear ValueError.

setup_controls.py provisions the agent, two controls, attaches one
directly, and binds the other to (env, prod) via the natural-key upsert
endpoint (idempotent on re-run). demo_agent.py walks through the four
phases and prints the expected outcome at each step.

Indexed in examples/README.md alongside the other framework demos.
],
)

agent = await _get_agent_or_404(agent_name, db)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope GET /agents/{name}/controls existence check by namespace

This route now advertises namespace-scoped effective controls, but it still resolves the agent with _get_agent_or_404(agent_name, db), which only filters on Agent.name. Once duplicate agent names exist across namespaces (explicitly allowed by this migration), a request in namespace default can succeed against an agent row that only exists in another namespace and return 200 with the wrong/empty control set instead of 404. That also breaks the stated contract that this endpoint and /evaluation return the same result for the same namespace-scoped inputs, because /evaluation does filter by namespace_key.


Returns the resolved ``(target_type, target_id)`` to forward.
"""
if target_type is None and target_id is None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_resolve_session_target() now rewrites or rejects the caller's target whenever state.current_agent is set, but check_evaluation() and the exported check_evaluation_with_local() helper are not inherently session-bound: they already take their own client and/or controls. After init(target_type="env", target_id="prod"), a later ad hoc evaluation for another agent, server, or target will either inherit env/prod when the caller omits target fields or raise when the caller passes a different target, even though no session cache is being consulted on these paths. That breaks legitimate multi-agent / multi-target server-side checks.

has_more = len(rows) > limit
if has_more:
rows = rows[:limit]
next_cursor = str(rows[-1].id) if has_more and rows else None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next_cursor is serialized as a string here, but the new control-bindings list request still validates cursor as an integer in the generated SDK. That means the documented flow of taking pagination.next_cursor from one page and feeding it into the next .list() call fails client-side validation in TypeScript instead of paginating. This endpoint should return the same cursor type that it expects back on the next request.

…-binding evaluator gate

Three review issues against the merged-resolver contract:

1. Cross-namespace agent lookup: GET /agents/{name}/controls now passes
   namespace_key into _get_agent_or_404. An agent that exists only in
   another namespace surfaces as 404 instead of returning a 200 with the
   wrong/empty effective set. The lookup is opt-in at the helper so other
   call sites that don't yet thread namespace through stay unchanged.

2. List-binding cursor type: server emits next_cursor as a string, so the
   GET /control-bindings cursor parameter accepts a string and parses it
   to int internally. Round-trip with PaginationInfo.next_cursor now works
   end-to-end through the generated TypeScript SDK; previously the int
   typing on cursor failed client-side validation when fed back from
   pagination.next_cursor.

3. Agent-scoped evaluators on target bindings: ControlBindingsService now
   rejects controls whose condition tree references agent-scoped
   evaluators (agent_name:evaluator) at binding creation time. Target
   bindings have no specific agent to validate against, so a binding can
   apply a control to any agent that later evaluates against the target;
   accepting agent-scoped references would surface as a runtime
   evaluation failure instead of a clear 400 at attach time. New
   ErrorCode.CONTROL_BINDING_INCOMPATIBLE.

Also:

- SDK check_evaluation / check_evaluation_with_local are no longer
  session-bound. They take their own client (and controls); session
  target enforcement lives only on evaluate_controls. The shared
  validator is split: _validate_target_pair (both-or-neither) for the
  caller-owned helpers, _resolve_session_target (default + reject
  mismatch) for the session-bound entry point. Tests for the
  session-target rules move to evaluate_controls.

- TypeScript SDK regenerated to match the new cursor type.

- One regression test in test_target_merged_contract pins the
  cross-namespace 404.
except IntegrityError:
# Concurrent insert won the natural-key race. Roll back our insert,
# re-read the winning row, and apply the requested enabled value.
await self._db.rollback()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0:
await self._db.rollback() on the race path still resets the entire session, not just the failed insert. Silent data loss if the caller flushed anything else before this call. Needs async with self._db.begin_nested() to scope the rollback.

Policy,
agent_policies,
)
from ..namespace import get_namespace_key
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
Issue: 8 _get_agent_or_404 call sites still have no namespace scope
Only list_agent_controls (line 1426) passes namespace_key. The remaining 9 calls — get_agent (925), add_agent_policy (991), set_agent_policy (989), get_agent_policies (1064), get_agent_policy (1083), remove_all_agent_policies (1165), delete_agent_policy (1199), remove_agent_control (1299), patch_agent — do not. V1 is a single namespace so there's no live exposure, but these endpoints will silently operate cross-namespace the moment multi-tenancy is wired in. The fix in this branch introduces the helper signature for it; the remaining sites should be filled in now while the pattern is in front of us.

request: InitAgentRequest,
client: RequireAPIKey,
db: AsyncSession = Depends(get_async_db),
namespace_key: str = Depends(get_namespace_key),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
Issue: init_agent strict path now returns target controls on creation (behavior change)

Need API contract document update for changed behavior. Previously returned controls=[] for newly created agents; now queries and returns target-bound controls. Undocumented API contract change for callers that treat an empty list as a freshly-created-agent signal.

try:
definition = ControlDefinitionRuntime.model_validate(control_data)
except ValidationError:
return set()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returns set() for any control whose data fails ControlDefinitionRuntime.model_validate. A corrupted control with agent-scoped evaluators passes the gate and becomes bindable. The comment documents this as intentional, but it means the evaluator gate has a blind spot on malformed controls. At minimum, the docstring should be explicit that this is a known gap.

…ace, expose controlBindings

Three review issues:

1. Savepoint scoping for the upsert race: ``upsert_by_natural_key`` now
   wraps the conflicting insert in ``begin_nested()`` so a unique-
   constraint collision rolls back the SAVEPOINT only. The previous
   ``session.rollback()`` would discard every pending change in the
   surrounding transaction once anything composed this service after a
   prior flush.

2. Namespace-scope agent endpoints end-to-end. ``_get_agent_or_404``
   now requires ``namespace_key`` and is non-disclosing across
   namespaces. The 11 callers thread ``namespace_key=Depends(get_namespace_key)``
   through every signature; agent_policies / agent_controls reads,
   inserts, and deletes filter by namespace_key. Policy lookups in the
   association routes also filter by namespace_key, and
   ``ControlService.get_active_control_or_404 / list_controls_for_policy
   / add_control_to_agent / remove_control_from_agent`` accept
   ``namespace_key`` so the service layer is no longer namespace-blind.
   A regression test pins that cross-namespace agent association calls
   surface 404, mirroring the pattern from the GET /agents/{name}/
   controls case.

3. TypeScript client wrapper exposes the new ``controlBindings`` API
   alongside ``agents``, ``controls``, etc., so consumers using the
   public ``AgentControlClient`` no longer have to reach into the
   generated internals.
…g race

Two review issues:

1. ``create_binding`` now wraps the conflicting insert in a SAVEPOINT
   via ``begin_nested()`` so a duplicate-natural-key collision rolls
   back only that insert. Mirrors the upsert path so neither service
   method discards unrelated flushed work in a caller's transaction.

2. Plain agent metadata reads — ``GET /agents/{name}``,
   ``GET /agents``, ``GET /agents/{name}/evaluators``,
   ``GET /agents/{name}/evaluators/{evaluator_name}`` — now scope by
   ``namespace_key`` so duplicate names across namespaces (allowed by
   this migration) cannot leak rows from another namespace. The list
   endpoint additionally namespace-scopes the count, the page query,
   and the cursor-row lookup so pagination cannot redirect through a
   foreign-namespace agent.

TypeScript SDK regenerated to pick up the new docstrings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants