feat(space): enforce per-agent autonomyLevel as a ceiling - #2276
Conversation
SpaceLongHorizonAgent.autonomyLevel was stored and templated (suggestedAutonomyLevel) but never read in the daemon — all gating used the space-level autonomy, so per-agent trust tiers had no effect. For long-term agent sessions (resolved via promptProvenance.agentId), treat the agent's autonomyLevel as a ceiling on its own space-agent-tools calls: effective = min(spaceLevel, agentLevel), applied to requireSessionWriteAutonomy, approve_gate (autonomy path), and approve_task. Ceiling-blocked denials are audit-logged. Coordinators and ad-hoc members remain uncapped (only buildLongTermAgentMcpServer passes myAgentId); agents without a level and worker-agent-backed sessions are unchanged, preserving existing error messages and behavior. Tests cover the min() ceiling, the unaffected higher-level case, the no-level fallback (space-level message preserved), and audit logging.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 533bb30ec7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address PR #2276 review test-coverage gaps (no behavior change): - writers-allowlist override: a level-1 agent named in a gate's approved field writers list is approved (ceiling only binds the autonomy path) - cross-space myAgentId guard: an LH agent in another space contributes no ceiling; the space-level message governs - session-write boundary at agentLevel === 4 is permitted (inclusive <) - ceiling-blocked approve_task denial is audit-logged (task-attributed) Also attribute the approve_task ceiling audit entry with taskId so it is reachable via listByTask.
Address review findings (chatgpt-codex-connector) that the new ceiling was evadable. No change to the happy path. - send_session_message: the coordinator exemption now requires no calling agent id (myAgentId), so a long-term agent renamed to "coordinator"/ "space-agent" via update_agent can't bypass the ceiling. The real space:chat coordinator (no myAgentId) stays exempt. - getCallingAgentAutonomyLevel: fail closed at level 1 when myAgentId is set but resolves to no long-horizon record AND no worker record — i.e. a long-horizon agent deleted while its session is still live. Worker- agent long-term sessions (which legitimately have no LH record) remain uncapped, and foreign-space agents defer to the space level. - create_agent / create_agent_from_template: the created child inherits the caller's ceiling (autonomyLevel), so a restricted agent cannot spawn an uncapped child to perform privileged operations on its behalf. Uncapped callers still spawn uncapped (null) children. Tests cover all three: coordinator-name bypass blocked, deleted-record fail-closed, level-1 child inheritance, and uncapped-child preservation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fae25a073
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A restricted caller must not manufacture an uncapped child to bypass | ||
| // its own ceiling; the child inherits the caller's ceiling (null when | ||
| // the caller itself is uncapped). | ||
| autonomyLevel: getCallingAgentAutonomyLevel(), |
There was a problem hiding this comment.
Prevent restricted agents from commandeering trusted peers
Although this revision caps newly created children, a level-1 caller can still bypass the ceiling when the space already contains an uncapped or level-5 agent: update_agent allows it to replace that agent's instructions, and send_message_to_task can route a message to the agent and activate it, after which the target's higher ceiling permits the requested approval or session write. Restrict delegation and mutation of higher-autonomy agents based on the caller's ceiling; inheriting the ceiling only in the two creation paths does not close this bypass.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Investigated — the autonomy-escalation half of this is not reachable, and the instruction-rewriting half is real but pre-existing/out-of-scope for this PR.
Autonomy escalation (not reachable): the update_agent MCP tool schema (space-agent-tools.ts:4448-4477) exposes no autonomy_level field — only name/status/description/model/thinking_level/provider/custom_prompt/tools/setting_sources. The ceiling is keyed to agent.autonomyLevel (getCallingAgentAutonomyLevel reads repo.getById(myAgentId).autonomyLevel), and the only writer of that column is the human-facing spaceLongHorizonAgent.update RPC handler (rpc-handlers/space-long-horizon-agent-handlers.ts), not an agent MCP tool. So a level-1 agent cannot raise a peer's ceiling.
Instruction rewriting (real, but out of scope): true that update_agent lets a caller rewrite a peer's instructions with no ceiling check. But (a) that predates this PR — update_agent always allowed instruction edits; (b) the per-agent ceiling's stated scope is the agent's direct privileged calls (requireSessionWriteAutonomy/approve_gate/approve_task), not an indirect-influence/capability model; (c) the puppeted peer acts under its own identity/task context, not a forced privileged call on the caller's behalf. Worth a separate follow-up issue (capability model bounding which peers a low-trust agent can mutate), but not a regression introduced or left open here.
| agent.displayName, | ||
| sessionId, | ||
| null, | ||
| agentId, |
There was a problem hiding this comment.
Attach the ceiling before publishing new agent sessions
When ensureLongHorizonAgentSession creates a session, SessionLifecycle.create publishes session.created before this method writes promptProvenance.agentId; the event subscriber therefore classifies the new session as an ad-hoc member and starts attaching a generic, uncapped space-agent-tools server. This explicit capped attachment then races that asynchronous generic attachment, and both merge the same server key, so the generic server commonly finishes last and overwrites the capped one. On a restricted agent's first activation in a permissive space, its approval and session-write calls can consequently run without the new ceiling; persist the provenance before session.created is emitted or otherwise prevent the generic attachment from starting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Investigated — no race, because internalEventBus.publish awaits every subscriber before resolving.
internal-event-bus.ts:185 (publish) ends with await Promise.all(handlers) (line :222) — the doc comment states it "awaits every local internal handler." So in ensureLongHorizonAgentSession, the await sessionManager.createSession(...) at space-runtime-service.ts:591 does not return until the session.created subscriber (the generic attachSpaceToolsToMemberSession path) has fully resolved — including its internal getSessionAsync/getSpace awaits. Only then do lines :610 (provenance write) and :621 (attachLongTermAgentMcpServers, the capped server) run, and mergeRuntimeMcpServers is last-writer-wins for the space-agent-tools key, so the capped variant is the final writer. The session isn't active for tool calls until ensureLongHorizonAgentSession returns, so there is no window in which the uncapped server is live. No change needed.
🤖 Review by glm-5.1 (NeoKai) — Round 2
Recommendation: APPROVE
Round-1 gaps and the three Codex bypass findings are all addressed correctly with solid coverage. 240 tests pass; typecheck/lint clean.
Verified correct
security-auditorin the gatewriterslist approving at space 5. Deliberate delegation intact.myAgentIdguard (my P2) — a foreign-space LH agent is uncapped and the space-level message applies. Confirmed theagent.spaceId !== spaceIdbranch.effective 4 >= 4permitted; inclusive<locked.approve_taskaudit entry (my P3) —listByTaskfinds theagent_autonomy_ceilingentry withtaskIdas a first-class column.outboundSenderLevel === 'space-agent'is reachable by renaming an LH agent, and(… || myAgentId)re-gates it; the real coordinator (nomyAgentId) stays exempt.spaceAgentManageris a required config field, so the new.getById()can't NPE; worker agents resolve uncapped, a deleted LH agent's live session fails closed at level 1.create_agent/create_agent_from_templatetake noautonomy_levelarg, so no operator input is overridden; restricted callers can't spawn an uncapped child, uncapped callers spawnnull-level children.Non-blocking suggestions (not gating)
create_agent_from_templateinheritance would also lock thetemplate.suggestedAutonomyLevelfallback interaction in the uncapped-caller case (thecreate_agentpath is already covered).spaceAgentManager.getById → null) is correct by inspection but untested.Neither rises to a finding — the behavior is deliberate and verifiable from the code. Approving; ready for QA.