Repository navigation
fix(api,frontend): let people write to channel chats in /m and keep private agents in channels - #7822
Conversation
📘 Docs preview
Pages changed in this pull request This comment updates in place on every push. |
|
🤖 The AI agent says: Live QA evidence for this PR (local stack, OSS and EE, mock channel, mock harness; the self-managed case uses the runner's own ChatGPT login through a subscription sidecar with no key).
The /m answer of a turn sent in a channel chat is also posted to the channel thread by the existing outbox (unchanged behavior). Recordings (MP4) are kept on the dev box; the API cannot attach them to a comment. Files: |
|
Caution Review failedThe pull request is closed. 📝 Summary
Pre-merge checks:
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.74% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 136 functions across 52 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes both primary changes: allowing users to write to channel chats in /m and keeping private agents active in channels. Description check The description directly explains the channel-chat permission changes, private-agent binding behavior, UI updates, documentation changes, known limits, and test results. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Website previewPreview URL: https://pr-7822-agenta-website-preview.mahmoud-637.workers.dev Built from |
Railway Preview Environment
Dependent tests finished. Post /preview for manual review. Environment absence verified; retained storage requires separate provider accounting. Post |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
spec.md:11
openspec/changes/agent-session-sharing/specs/agent-session-sharing/spec.md:11
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTwo requirements contradict the updated channel scenario.
Line 11 says humans only read a channel session in Agenta. Line 18 says a Slack session keeps Viewer access for people. The scenario at lines 33-34 now says a member can read a project-visible Slack or Telegram session and send to it.
The
channel-session-accessspec and_project_members_sendboth follow the new rule. Update lines 11 and 18 to match. Remove the "only read" and "keep Viewer access" wording.Also applies to: 18-18
🧹 Nitpick comments: 1
api/oss/tests/pytest/integration/sharing/test_sharing_query_counts_postgres.py: 1
681-761:Trivial · Maintainability & Code Quality · Low value Remove the duplicated
_rawhelper.
RedisCommands.touchingalready defines a local_rawwith identical logic. This change adds a second copy at module level, after its first use in_create. Move the helper aboveRedisCommandsand use it in both places.
-
-
- Source mode: Auto
- Repository: Agenta-AI/agenta
- Branch: fix/channel-private-agents
- File: .coderabbit.yaml
- Commit: bc3b5b5
- Layers: Repository YAML (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
25c7af85-4f91-40c3-80bd-bdb54d8f52c3
-
api/entrypoints/access.pyapi/oss/src/apis/fastapi/agent_templates/router.pyapi/oss/src/apis/fastapi/channels/router.pyapi/oss/src/core/channels/interfaces.pyapi/oss/src/core/channels/service.pyapi/oss/src/core/sessions/starts/service.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/streams/service.pyapi/oss/src/core/sessions/streams/types.pyapi/oss/src/core/sharing/admission.pyapi/oss/src/core/sharing/dtos.pyapi/oss/src/core/sharing/service.pyapi/oss/src/dbs/postgres/channels/dao.pyapi/oss/src/dbs/postgres/sessions/streams/dao.pyapi/oss/src/dbs/postgres/sharing/dao.pyapi/oss/tests/pytest/integration/sharing/test_list_scope_sql.pyapi/oss/tests/pytest/integration/sharing/test_runner_write_counts_postgres.pyapi/oss/tests/pytest/integration/sharing/test_server_run_ownerless_sessions.pyapi/oss/tests/pytest/integration/sharing/test_sharing_dao_locks.pyapi/oss/tests/pytest/integration/sharing/test_sharing_query_counts_postgres.pyapi/oss/tests/pytest/unit/channels/test_channels_outbox_worker.pyapi/oss/tests/pytest/unit/sessions/test_session_start_service.pyapi/oss/tests/pytest/unit/sessions/test_watch_lifecycle_publish.pyapi/oss/tests/pytest/unit/sharing/test_channel_session_policy.pyapi/oss/tests/pytest/unit/sharing/test_composition_roots.pyapi/oss/tests/pytest/unit/sharing/test_initial_policy.pyapi/oss/tests/pytest/unit/sharing/test_resource_policy.pyapi/oss/tests/pytest/unit/sharing/test_sharing_writes.pyapi/oss/tests/pytest/utils/sharing.pydocs/design/sharing-program/CONTRACT.mddocs/design/sharing-program/interfaces/m3/entry-points.mddocs/design/sharing-program/interfaces/m3/interfaces.mddocs/design/sharing-program/interfaces/m3/technical-design.mddocs/docs/guides/12-connect-slack.mdxdocs/docs/guides/14-share-agents-and-chats.mdxopenspec/changes/agent-session-sharing/design.mdopenspec/changes/agent-session-sharing/specs/agent-session-sharing/spec.mdopenspec/changes/agent-session-sharing/specs/channel-session-access/spec.mdweb/mobile/src/features/agents/AgentOverviewScreen.tsxweb/mobile/src/features/agents/useAgentOverviewAccess.tsweb/mobile/src/features/agents/useAgentPublishPanel.tsxweb/mobile/src/features/chat/AgentPrivacyDialogHost.tsxweb/mobile/src/features/chat/SessionSharingControl.tsxweb/mobile/src/features/chat/ShareMenu.tsxweb/mobile/src/features/chat/sessionReadOnly.tsweb/mobile/src/features/chat/useAgentPrivacyProps.tsweb/mobile/src/features/settings/ChannelsTab.tsxweb/mobile/src/lib/sharingCopy.tsweb/mobile/tests/unit/agentChannelSharing.render.test.tsxweb/mobile/tests/unit/agentOverviewAccess.render.test.tsxweb/mobile/tests/unit/sessionReadOnly.render.test.tsxweb/mobile/tests/unit/sessionSharingControl.render.test.tsxweb/mobile/tests/unit/shareMenu.render.test.tsxweb/packages/agenta-entity-ui/src/sharing/AgentPrivacyDialog.tsxweb/packages/agenta-entity-ui/src/sharing/copy.tsweb/packages/agenta-entity-ui/src/sharing/index.tsweb/packages/agenta-entity-ui/src/sharing/useAgentSharingWrites.tsweb/packages/agenta-sessions-ui/src/sharing/SessionAccessChip.tsxweb/packages/agenta-sessions-ui/src/sharing/copy.tsweb/packages/agenta-sessions-ui/tests/unit/SessionAccessChipProjectSend.render.test.tsxweb/packages/agenta-settings-ui/src/channels/ChannelAgentPicker.stories.tsxweb/packages/agenta-settings-ui/src/channels/ChannelAgentPicker.tsxweb/packages/agenta-settings-ui/src/channels/ChannelManagePanel.stories.tsxweb/packages/agenta-settings-ui/src/channels/ChannelManagePanel.tsxweb/packages/agenta-settings-ui/src/channels/ChannelsHubView.tsxweb/packages/agenta-settings-ui/src/channels/ChannelsSettingsPage.tsxweb/packages/agenta-settings-ui/src/channels/actions.tsweb/packages/agenta-settings-ui/src/channels/index.tsweb/packages/agenta-settings-ui/src/channels/sharingCopy.tsweb/packages/agenta-settings-ui/src/channels/types.tsweb/packages/agenta-settings-ui/src/channels/useChannelPanel.tsxweb/packages/agenta-settings-ui/src/index.tsweb/packages/agenta-settings-ui/tests/unit/channelSharing.test.tsxweb/storybook/stories/entity-ui/SharingAgentPrivacy.stories.tsx
-
- api/oss/tests/pytest/integration/sharing/test_list_scope_sql.py
- api/oss/tests/pytest/integration/sharing/test_runner_write_counts_postgres.py
- web/mobile/src/features/chat/AgentPrivacyDialogHost.tsx
- web/packages/agenta-settings-ui/src/channels/index.ts
- api/entrypoints/access.py
- web/packages/agenta-entity-ui/src/sharing/copy.ts
- api/oss/tests/pytest/unit/sharing/test_initial_policy.py
- web/mobile/src/features/agents/useAgentPublishPanel.tsx
- web/mobile/tests/unit/shareMenu.render.test.tsx
- web/packages/agenta-settings-ui/src/channels/ChannelsSettingsPage.tsx
- web/mobile/src/features/settings/ChannelsTab.tsx
- web/storybook/stories/entity-ui/SharingAgentPrivacy.stories.tsx
- web/mobile/src/features/chat/sessionReadOnly.ts
- web/packages/agenta-settings-ui/src/index.ts
- web/mobile/src/features/agents/useAgentOverviewAccess.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.
-
| await self.channels_dao.clear_agent_default( | ||
| project_id=project_id, | ||
| user_id=user_id, | ||
| agent_id=current_default.id, |
There was a problem hiding this comment.
🔎 Supported by static analysis
-
Script executed:
sed -n '1280,1330p' api/oss/src/core/channels/service.py sed -n '1425,1475p' api/oss/src/core/channels/service.py sed -n '455,540p' api/oss/src/dbs/postgres/channels/dao.py
Repository: Agenta-AI/agenta
Length of output: 6128
Script executed:
rg -n -F -e '_activation_guard' -e 'guard_agent_activation' -e 'SharedItemNotFound' -e 'set_channel_agent_default' -e 'clear_agent_default' -e 'is_default' -- api/oss/src api/ee/src 2>/dev/null || test "$?" -eq 1 echo '---' sed -n '1,1' /dev/null ast-grep outline api/oss/src/core/channels/service.py --view expanded 2>/dev/null | rg -n -F -e '_activation_guard' -e 'set_agent_default' -e '_edit_agent_row' || test "$?" -eq 1
Repository: Agenta-AI/agenta
Length of output: 14300
Script executed:
rg -n -F -e 'def guard_agent_activation' -e 'def _activation_guard' -e 'is_default' -e 'partial' -e 'unique' -- api/oss/src api/ee/src api/oss/databases 2>/dev/null | head -n 80 || test "$?" -eq 1
Repository: Agenta-AI/agenta
Length of output: 8529
Clear the old default only after the promote's guard passes.
set_agent_default clears the old default in its own committed session, then calls _edit_agent_row. For an already-active binding, _edit_agent_row passes is_active=True to _activation_guard, and edit_agent runs guard_agent_activation before it writes. If the caller does not own the private agent, the guard raises SharedItemNotFound. The old default stays cleared, and the promoted agent is not set as default.
The partial unique index on is_default is why the clear must run before the set. Use one transaction, or restore the old default when the promote fails.
Without a default, sigil-less messages do not go unanswered. They fall back to the first active binding, so routing changes to an agent that the user did not choose.
| - The sharing state reads (`fetch_*_sharing` and every write) read the policy, `is_channel` and `references_agent` from the DAO, uncached. `references` is filled once by a later heartbeat, so a cached `shareable` would be stale. Only `evaluate` and the read scopes use the caches. | ||
| - Disclosure is not a `SharingService` method. NEW pure `oss/src/core/sharing/disclosure.py::build_share_disclosure(*, parameters: Optional[dict]) -> ShareDisclosure` parses the revision's `parameters["tools"]`. The sharing router reads the agent's head revision through `WorkflowsService` and calls it. | ||
| - After a make-private commits, the service calls `PrincipalsDAOInterface.invalidate_principal(principal=PrincipalRef(kind=INSTALLATION, id=<channel agent id>))` through its injected `principals_dao` (core may not import the Postgres DAO module; `bootstrap_access()` builds one `PrincipalsDAO` for `SharingService` and `AccessService`) for each id in `AgentPolicyWriteResult.deactivated_channel_agent_ids`, so a warm `fetch_principal` entry cannot keep a deactivated installation active. A failed invalidation is logged and does not fail the write. | ||
| - A make-private changes no channel agent (changed 2026-10-26: a private agent may answer in channels), so it clears no installation principal. |
There was a problem hiding this comment.
Remove the stale principals_dao dependency claim.
The dependency section still says that SharingService needs principals_dao for installation-principal cache invalidation after make-private. This change states that make-private clears no installation-principal cache entries. Update the dependency description, or document the remaining use of principals_dao.
| | P4 heartbeat | `SessionStreamsService.heartbeat(*, project_id, request, runner_verified=False, envelope)` (M2) and `_heartbeat_locked` | new required kwargs `access: AccessPolicy` (used only in the `prior_stream is None` create), `run_admission: RunAdmission`, `admission_envelope: RunEnvelope`, next to M2's `envelope` (which is `None` for Bearer and ApiKey). `_heartbeat_locked` reads `prior_stream` once, at the top, before the supersession check (its branch reuses the row). Then it authorizes: row absent → `admit_turn(session_id=None)`, then insert the row (initial flags, `turn_id`, `access`) before any liveness lock; on `SessionStreamAlreadyExists` read the winning row and authorize it as below before any lock, update or response; row present and verified continuity (`request.is_running`, `request.turn_id == prior_stream.turn_id`, and `get_turn_admission(...)` equals `admission_envelope.authority.principal`; `get_turn_admission` is one `EVAL` of `CURRENT_TURN_ADMISSION_LUA`, which returns and renews the record only while the session's `running` key equals the turn; a Redis error counts as no record) → no check; otherwise → `admit_turn(session_id=request.session_id)` for a running beat, `admit(session_id, agent_artifact_id=None)` for any other beat. After a successful check of a running beat with a `turn_id`, for the row actually used, `record_turn_admission(...)` (a write error is logged; the next beat is checked again) (Redis, NEW `record_turn_admission` and `get_turn_admission` in `dbs/redis/sessions/locks.py` next to `bind_turn`; key per project, session and turn; value the admitted `PrincipalRef`; same TTL as the binding; written only after create-or-conflict is resolved and the used row passed). NEW `forget_turn_admissions(engine, *, project_id, session_id, turn_ids)` deletes the records on the end beat, on displacement (cancel, steer, kill), on handover and on stop; a refused end beat (not running, with a `turn_id`) whose record names the same principal deletes that record too, so a replay of an ended turn is fully checked. A refusal raises before any lock, row write or response. A heartbeat with no envelope answers the canonical 404 before any row read. Router `heartbeat_session_stream` computes `access` and `admission_envelope = request_run_envelope(request)`; it reads no session row. | | ||
|
|
||
| Channel threads first (release QA, 2026-10-25). The channel inbox writes a `channel_threads` row before its session's first heartbeat, so any first touch can insert that session's row. `SessionStreamsDAO.create` asks, in its one insert statement (index `ix_channel_threads_project_id_session_id`, added in `oss000000048`), whether a thread names the session id. If one does, it writes the channel policy (project-visible, ownerless) instead of the requested one and returns `has_channel_thread`; the created row's cache entry records a channel session. A creation that is not the channel's own run (origin `installation`) is then checked against that row before anything else: P1 runs the route's recheck before any liveness read, lock or displacement (or `_recheck_row` when the row is created after the turn's locks); P4 runs a session-only `admit` (the beat's agent target was checked before the insert); a template start runs its existing `admit` before any claim. A refused P1 or P4 keeps the row (it is the channel session's own row), clears the refused request's flags and turn id from it (`release_refused_turn`, guarded by row id and the turn id the insert wrote), publishes it to the project's lists and deletes nothing. P2 keeps the ownerless edit rule. P3 does not apply the rule yet (its session ids are fresh UUIDs; follow-up). | ||
| Channel threads first (release QA, 2026-10-25; changed 2026-10-26). The channel inbox writes a `channel_threads` row before its session's first heartbeat. `SessionStreamsDAO.create` asks, in its one insert statement (index `ix_channel_threads_project_id_session_id`, added in `oss000000048`), whether a thread names the session id, and returns `has_channel_thread`. Only the channel's own run (`SessionStreamCreate.by_channel_run`, origin `installation`) inserts such a row, with the channel policy it computed (`channel_session_policy`: private to the agent's owner for a private agent, else project-visible and ownerless); the created row's cache entry records a channel session. Any other creator inserts nothing: the id is reserved like a tombstone's (`SessionIdTombstoned`, answered as a hidden session), and a turn that took its locks hands them back. Before 2026-10-26 the insert wrote the ownerless project policy for any creator and kept a refused creator's row (`release_refused_turn`, now removed). |
There was a problem hiding this comment.
🔎 Supported by static analysis
-
Script executed:
rg -n 'commands.py|OWNERLESS_PROJECT_POLICY|P1' docs/design/sharing-program/interfaces/m3/interfaces.md rg -n 'OWNERLESS_PROJECT_POLICY|access=' api/oss/src/core/channels/commands.py
Repository: Agenta-AI/agenta
Length of output: 2432
Script executed:
set -o pipefail printf '%s\n' '--- channel command definitions and relevant calls ---' rg -n -F --glob '*.py' -e 'def command' -e '_start_turn' -e 'OWNERLESS_PROJECT_POLICY' -e 'by_channel_run' api/oss/src/core/channels api/oss/src/core/sessions api/oss/src/tasks || true printf '%s\n' '--- channel command source around the policy call ---' nl -ba api/oss/src/core/channels/commands.py | sed -n '1,230p' printf '%s\n' '--- session stream service definitions and session-row creation references ---' rg -n -F --glob '*.py' -e 'class SessionStreamsService' -e 'def _start_turn' -e 'SessionStreamCreate' -e 'channel_thread' -e 'has_channel_thread' api/oss/src/core/sessions api/oss/src/core/channels || true
Repository: Agenta-AI/agenta
Length of output: 13974
Script executed:
set -o pipefail printf '%s\n' '--- SessionStreamsService.command ---' nl -ba api/oss/src/core/sessions/streams/service.py | sed -n '320,465p' printf '%s\n' '--- SessionStreamsService._start_turn ---' nl -ba api/oss/src/core/sessions/streams/service.py | sed -n '1500,1715p' printf '%s\n' '--- SessionStreamCommandRequest and SessionStreamCreate definitions ---' nl -ba api/oss/src/core/sessions/streams/dtos.py | sed -n '85,145p' printf '%s\n' '--- session start service creation path ---' nl -ba api/oss/src/core/sessions/starts/service.py | sed -n '175,230p' nl -ba api/oss/src/core/sessions/starts/service.py | sed -n '330,405p'
Repository: Agenta-AI/agenta
Length of output: 27246
Correct the stale P1 caller description.
core/channels/commands.py passes OWNERLESS_PROJECT_POLICY only to the !stop path. That call does not reach the row-creation arm because command(...) creates a row only when recheck is present and the mode is send or steer. State in the P1 row that this caller cannot create a channel session.
| 1. `decide(EDIT_WORKFLOWS or MANAGE_AGENT_OWNERSHIP, resource=agent)`. The service write takes `organization_id`, `project_id` and `access: RequestAccess` (the audit event needs the organization, the credential kind and the acting user). | ||
| 2. `SharingService` reads `current`, checks the rule (active owner, caller is owner, or no active owner for take-ownership), computes `new`. A non-owner write on a readable agent raises `AccessDenied` (`RESOURCE_DENIED`, 403); an unreadable agent raises `SharedItemNotFound` (404). A write that changes nothing commits nothing and publishes no event. | ||
| 3. `SharingDAO.set_agent_policy(expected=current, policy=new)` in one transaction: lock `workflow_artifacts (project_id, id) FOR UPDATE` for an artifact with an `is_agent` revision (else `SharedItemNotFound`); compare `agent_access` with `expected` (mismatch: `SharingConflict`, rollback); upsert; for `private`, `UPDATE channel_agents SET flags = COALESCE(flags, '{}'::jsonb) || '{"is_active": false}'` for channel agents whose references name the artifact (interfaces.md 3.8); commit. It returns `AgentPolicyWriteResult(policy, deactivated_channel_agent_ids)`. | ||
| 3. `SharingDAO.set_agent_policy(expected=current, policy=new)` in one transaction: lock `workflow_artifacts (project_id, id) FOR UPDATE` for an artifact with an `is_agent` revision (else `SharedItemNotFound`); compare `agent_access` with `expected` (mismatch: `SharingConflict`, rollback); upsert (channel agents are not touched; 2026-10-26: a private agent may answer in channels); commit. It returns `AgentPolicyWriteResult(policy, deactivated_channel_agent_ids)`. |
There was a problem hiding this comment.
Fix the stale return type in step 3.
The step now says channel agents are not touched. It still says the call returns AgentPolicyWriteResult(policy, deactivated_channel_agent_ids). The DTO and SharingDAO.set_agent_policy now return only policy. Change the text to AgentPolicyWriteResult(policy).
| When the owner makes a connected agent private, Agenta turns its Slack connection off. Messages in Slack then get no answer. The connection stays off when the agent is shared again: open the agent's connection in **Channels** and click **Turn on**. **Turn on** is not available while the agent is private. | ||
| When the owner makes a connected agent private, the agent keeps answering in Slack through the connections the owner made. A connection that another member made stops answering; the owner can connect the agent again. New Slack chats with it are private to the owner. Chats that already exist keep who can see them. | ||
|
|
||
| If a connection is off, open it in **Channels** and click **Turn on**. |
There was a problem hiding this comment.
Distinguish an off binding from a binding that cannot answer.
If another member created the binding before the agent became private, Turn on cannot make that binding eligible to answer. The preceding paragraph identifies this case, but this instruction offers only Turn on as recovery. Tell the owner to create an owner-created connection or binding for that case; reserve Turn on for a binding that is actually off. Based on the PR objectives, a private agent runs only when the binding creator is its owner.
| panelPrivate: (owner: string) => `Private: only people ${owner} added can open it.`, | ||
| panelChannel: | ||
| "This chat comes from Slack. Everyone in the project can read it here. Reply in Slack to continue it.", | ||
| "This chat comes from a channel. Everyone in the project can read it and send to it.", |
There was a problem hiding this comment.
Qualify who can send to a channel chat.
This sentence says everyone in the project can send. The channel-chip test also covers a project viewer with can_send: false. That viewer sees this sentence and a view-only explanation in the same popover. Say that project members with send access can reply, and align the same claim in docs/docs/guides/14-share-agents-and-chats.mdx Line 114.
| connection.agentOff = answering | ||
| ? asRecord(answering.flags).is_active === false | ||
| : false |
There was a problem hiding this comment.
Represent bindings that are active but cannot answer.
When another member created a binding and its agent becomes private, the PR’s run rule stops that binding from answering without turning its is_active flag off. This mapper still sets agentOff to false. ChannelManagePanel then labels the connection “Live.” Selecting the same agent again does not repair it because pointConnectionAt returns when the reference already matches. Expose the binding’s run eligibility or its creator to the connection UI, and provide an owner-created replacement path. Based on the PR objectives, non-owner-created bindings remain active but no longer answer after the agent becomes private.
| privateAgentRow: (platform: string) => | ||
| `Private. Share it with the project to use it in ${platform}.`, | ||
| share: "Share", | ||
| `For a shared agent, everyone in ${project} can read its ${platform} conversations in Agenta, including direct messages. For a private agent, only its owner can read them.`, |
There was a problem hiding this comment.
Limit the private-conversation privacy claim.
This notice says only the owner can read a private agent’s channel conversations in Agenta. Project members with channel-view permission can still read that connection’s message logs through the channel API. Say that only the owner can open the resulting private chats, and disclose the separate log access. Align the quoted notice in docs/docs/guides/12-connect-slack.mdx Line 25. Based on the PR objectives, channel observability routes still expose raw message logs to members with VIEW_CHANNELS.
…rivate agents in channels A chat that came from Slack or Telegram was read-only for every person: the sharing rules made channel sessions view only. Members now send to project-visible channel chats like any project chat. A private agent may now answer in a channel. Only its owner binds it, a make-private no longer turns its bindings off, and the channel chats it creates are private to its owner. Only the owner's bindings run the private agent and act in those chats, and only the channel's own run creates a channel thread's session.
…ts follow the owner's ruling
bc3b5b5 to
24c4468
Compare
|
🤖 The AI agent says: Acceptance results, and one code change. The acceptance suites
Both lanes in this clone are rebased on
Head: 24c4468. |
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: Agenta-AI/agenta
- Branch: fix/channel-private-agents
- File: .coderabbit.yaml
- Commit: 24c4468
- Layers: Repository YAML (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a605f465-6188-459b-ab63-2c4679289b6a
-
api/oss/src/core/sharing/service.pyapi/oss/tests/pytest/acceptance/sharing/test_channel_sessions.pyapi/oss/tests/pytest/acceptance/sharing/test_channel_thread_first_touch.pyapi/oss/tests/pytest/unit/sharing/test_resource_policy.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.
-
| action == Permission.RUN_WORKFLOWS | ||
| and _is_channel_turn(principal, context) | ||
| and _binding_creator(principal) == policy.owner_user_id |
There was a problem hiding this comment.
🔎 Supported by static analysis
-
Script executed:
set -eu printf '%s\n' '--- changed diff ---' git diff --unified=30 ca29ff147815e7d7b49866c1abc2983a154d0413 24c4468d5aa70c6c64af57897aa53dc7ee6c8908 -- api/oss/src/core/sharing/service.py printf '%s\n' '--- sharing definitions and callers ---' rg -n -F --glob '*.py' -e '_is_channel_turn' -e 'class AccessService' -e 'def decide' -e 'origin_id' api/oss/src api/ee/src 2>/dev/null || test "$?" -eq 1 printf '%s\n' '--- relevant file outlines ---' ast-grep outline api/oss/src/core/sharing/service.py --view expanded || true printf '%s\n' '--- access and run-context excerpts ---' rg -n -F --glob '*.py' -e 'AccessService' -e 'RunContext' -e 'origin_id' api/oss/src/core/access api/oss/src/core api/oss/src/apis api/oss/tests 2>/dev/null | head -240 printf '%s\n' '--- changed tests mentioning channel origin or mismatch ---' rg -n -F --glob '*.py' -e 'INSTALLATION' -e 'origin_id' -e 'private agent' -e 'private_agent' api/oss/tests api/ee/tests 2>/dev/null | head -240
Repository: Agenta-AI/agenta
Length of output: 42320
Script executed:
set -eu printf '%s\n' '--- policy helpers and tests ---' rg -n -F -- '_is_channel_turn' api/oss/src/core/sharing/service.py rg -n -F -- '_binding_creator' api/oss/src/core/sharing/service.py sed -n '125,185p' api/oss/tests/pytest/unit/sharing/test_resource_policy.py sed -n '1060,1160p' api/oss/src/core/sharing/service.py printf '%s\n' '--- access-service declarations and decision flow ---' rg -n -F --glob '*.py' -e 'class AccessService' -e 'async def decide' -e 'def decide' -e 'RunContext' api/oss/src/core/access api/oss/src | head -220 printf '%s\n' '--- exact access files ---' find api/oss/src/core/access -maxdepth 3 -type f -name '*.py' -print printf '%s\n' '--- channel principal and admission construction ---' rg -n -F --glob '*.py' -e 'context.origin_id' -e 'origin_id=str' -e 'origin=RunOrigin.INSTALLATION' -e 'authority.principal' api/oss/src/core api/oss/src/apis api/oss/src/entrypoints api/oss/tests/pytest/unit/channels api/oss/tests/pytest/acceptance/channels | head -260 printf '%s\n' '--- resumed and durable admission references ---' rg -n -F --glob '*.py' -e 'admit_turn' -e 'resumed_envelope' -e 'stored_work_envelope' -e 'request_run_envelope' -e 'RunAdmission' api/oss/src | head -260
Repository: Agenta-AI/agenta
Length of output: 20215
Script executed:
set -eu printf '%s\n' '--- AccessService decision ---' sed -n '80,190p' api/oss/src/core/access/service.py printf '%s\n' '--- AccessService run-admission helpers ---' sed -n '640,745p' api/oss/src/core/access/service.py printf '%s\n' '--- RunAdmission ---' sed -n '1,235p' api/oss/src/core/sharing/admission.py printf '%s\n' '--- principal envelope constructors ---' sed -n '70,220p' api/oss/src/core/access/principals.py printf '%s\n' '--- channel inbox envelope creation ---' sed -n '350,410p' api/oss/src/tasks/asyncio/channels/inbox.py printf '%s\n' '--- channel admission tests and run-context mapping ---' sed -n '1,190p' api/oss/tests/pytest/unit/channels/test_channels_inbox_principal.py sed -n '130,210p' api/oss/src/dbs/postgres/sessions/turns/mappings.py
Repository: Agenta-AI/agenta
Length of output: 32276
Script executed:
set -eu printf '%s\n' '--- AccessService resource-policy path ---' sed -n '145,285p' api/oss/src/core/access/service.py printf '%s\n' '--- exact policy helper lines ---' nl -ba api/oss/src/core/sharing/service.py | sed -n '1088,1135p' printf '%s\n' '--- exact admission and resume lines ---' nl -ba api/oss/src/core/sharing/admission.py | sed -n '88,145p' nl -ba api/oss/src/core/access/principals.py | sed -n '150,175p' printf '%s\n' '--- relevant channel envelope lines ---' nl -ba api/oss/src/tasks/asyncio/channels/inbox.py | sed -n '370,398p'
Repository: Agenta-AI/agenta
Length of output: 12249
Require the channel origin ID to match the installation binding.
_is_channel_turn accepts any installation context with origin=INSTALLATION. AccessService forwards the context without validating origin_id, including for resumed runs. A mismatched origin can therefore pass the private-agent RUN_WORKFLOWS exception. Require context.origin_id == str(principal.id) and add a mismatched-origin test.
🐛 Suggested fix
-
principal.kind == PrincipalKind.INSTALLATION and context is not None and context.origin == RunOrigin.INSTALLATION + and context.origin_id == str(principal.id)
People could not write to their own agents from the app. A chat that started in Telegram or Slack showed a "View only" pill in /m, the composer was replaced by "This chat comes from Slack. Reply in the Slack thread to continue it.", and the popover named Slack even for a Telegram chat. Separately, a private agent could not stay in a channel: binding it was refused, and making a bound agent private turned its channels off.
Why it happened
SharingServiceexcluded every session that achannel_threadsrow names from "project visibility gives members send" (_project_members_send), socan_sendwas false for every person in any chat created from a channel message (Slack or Telegram). /m only rendered that answer, with Slack-only copy. It did not treat every chat of a bound agent as a channel chat.set_agent_policyturned every binding of the agent off in the make-private transaction, and rule A2 refused the installation principal on a private agent. Channel sessions were always written ownerless and project-visible.What changes
Channel chats are ordinary chats now:
Private agents can stay in channels (owner ruling, 2026-10-26):
RUN_WORKFLOWSonly; it never reads, lists or edits it) a private agent only on its own channel turn (run origininstallation, not a child run) and only when the binding's creator is the agent's owner. S3 now means: the owner's binding reads, runs and edits the private channel chats its creator owns. The binding creator comes from the principal record that the same decision already read (request memo, no I/O).SessionIdTombstoned), so nobody can make a private agent's channel chat project-visible by touching its id first. This removes the old "write the channel policy and keep the refused row" path (release_refused_turn).UPDATEofflags.is_default(clear_agent_default), so it never rewrites or turns off another person's binding.UI: the private-agent blocks in the channel setup (picker rows, Publish note, manage panel note, refusal error) and the make-private "connections turn off" warning are removed.
The run principal of a channel turn is unchanged: the installation (binding) principal, with the linked user's role when the sender is linked, else the binding creator's role.
Copy changes
composerChannel(/m),privateAgentRow,publishPrivate,refusedPrivate,manageOffPrivate,shareWithProject,share(settings-ui),channelOne,channelMany,channelRestore,toastConnectionOff(entity-ui).panelChannelis now "This chat comes from a channel. Everyone in the project can read it and send to it."Known limits (owner decisions, not fixed here)
/channels/inbox/query,/channels/outbox/query, thread and agenta conversation reads) check onlyVIEW_CHANNELS, so project members can read the raw message log of a private agent's channel. Same kind of gap as the existing trace limit; documented in the sharing guide.Tests
test_a_channel_run_creates_its_session_at_the_same_costpins a private agent's channel-session create at the same statement count as a project agent's (HB_CREATE + 1) and exactly one Redis read of the agent entry (the admission's), so zero added statements and round trips. A member's send to a channel chat costs exactly SEND (test_channel_thread_creation_paths). A make-private is now 4 statements instead of 5.pnpm lint-fixclean.Codex review: two rounds; every must-fix is fixed (binding scoping, reserved channel thread ids, atomic default clear, a story import).