chore: refactor no more channels - #5625
Merged
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
The user who starts a session has always been known - the repo spends it on the dedicated channel's owner and participant rows - but the session itself never stored it, so "who started this?" could only be answered by reading a channel-ownership fact as a proxy for an attribution fact. Adds `initiator_user_id` to `agent_session`, backfilled from the channel owner (exact: `create` sets both from the same value, and `channel_id` is NOT NULL UNIQUE). Renames `CreateAgentSessionParams::owner_id` to match - "owner" reads as a channel concept and collides with `AccessLevel::Owner` in the permissions work this precedes. The column is deliberately absent from the repo's `UPDATE`, alongside `channel_id`: the initiator is immutable for a session's life. The PUT body carries it as an echo field, like `channel_id` and `createdAt`, so a changed value is ignored rather than applied. `ChannelSession` and `AgentSessionEventDecision` gain `allow(clippy::large_enum_variant)` rather than boxing, since both are reshaped when sessions stop owning channels.
A session's live transport is in-memory state owned by whichever process
opened it, so `document_storage_service` - which builds its own
`AgentSessionServiceImpl` with no attached actors - could never reach one.
Mutating routes move to `agent_harness_service`, which owns them; reads
stay in storage, where every other read is.
The router splits accordingly: `agent_session_read_router` (get, logs) and
`agent_session_control_router` (delete, control). The harness service grows
an axum server on 8101 alongside its Kafka loop, with its own OpenAPI bin
and generated web client.
`PUT /agent-sessions/{id}` is gone. It had no callers, and full-resource
replace was the wrong shape for "change the model" - you had to echo back
`channelId` and `createdAt` to touch one field. `POST /{id}/control`
replaces it with a named operation: prompt, change_model, or stop. The
acting user comes from the caller's credentials, never the body, so a
caller cannot attribute an operation to someone else.
Stop is a real interrupt: `AgentAction::supersedes_queued` lets the machine
drop what it has queued rather than sending it and then cancelling it, and
those dropped actions complete `Ok` - a `Disconnected` there would have the
harness resume and resend the very prompt the stop killed.
`HarnessCommand` becomes Open / Deliver / Delete. Forward and Control were
the same operation wearing different clothes: the differences that seemed
to justify separate variants - whether to reconnect a dead session, whether
to announce - are properties of the action and its origin, so they moved
onto `AgentAction::must_reach_agent` and `DeliverAction::announce`.
The comms placeholder mirror is deleted whole: the `Comms` port, both write
paths, `sync_placeholders` and its binary, and `append_event`. The log is
the only source of truth now. `GET /agent-sessions/{id}/log` serves it by
session rather than by channel, and each entry carries `createdAt` - the
column always existed and is in the ordering index, but was never on the
wire, so a reader had nothing to order by.
Agent messages no longer appear in channel message lists. That is intended;
the frontend catches up separately. `crates/channels` keeps its joins
against the identifier table, which simply stop matching new rows, and no
migration drops anything yet.
A session no longer owns, creates, or is resolved through a comms
channel. `agent_session.channel_id` is dropped - the column, the field,
the unique constraint, and the foreign key - and `create` is a single
row insert with the initiator persisted on the session itself. `delete`
deletes the session; it used to delete the *channel* and ride the
cascade, which stopped meaning anything the moment there was no channel.
The channel-keyed log endpoint is gone. `GET /agent-sessions/{id}/log`
is the only way to read a log, and it now names the session's bot -
the one thing a reader cannot derive from the frames - alongside the
entries, as the channel response used to.
`ChannelSession` collapses to `None | CreatedFromThread`: with no
dedicated channel there is nothing else a channel context can resolve
to, so `find_for_channel` matches only the originating thread and bot.
The trigger's decision function now takes a `PotentialTriggerEvent`,
whose second variant - `AgentSessionMessage`, a message sent straight
to a session - is the session-native replacement for the dedicated
channel: declared, decided, and translated by the harness, but produced
by nothing until its topic exists.
Realtime frames are addressed by session and fan out to the session's
viewers (today: the initiator, which is exactly who the dedicated
channel's one participant row used to resolve to). The announcement
chip carries only the session id; old chips still deserialize, their
channel just no longer means anything.
The web app keeps compiling but the agent channel view goes dark: it
resolved sessions through the channel relation this removes. It comes
back keyed by session id with the frontend pass.
`EntityType::AgentSession`, with grants written when the session is: the initiator as owner, and - when a mention opened the session - the channel the mention was posted in as editor. The channel grant is derived by reading the originating message's channel rather than trusted from the caller, so a caller cannot claim a channel the mention did not happen in. Channel membership is not copied; it arrives through `source_ids`, so adding someone to that channel gives them the session on their next request. The session row and its grants are written in one transaction, so a crash cannot leave a session nobody - not even its initiator - could open. Deletion is transactional for the mirrored reason: `entity_access.entity_id` is polymorphic and carries no foreign key, so nothing else would reap the rows. `get_agent_session_access` answers the access question through the same port, repo, and optimized-access paths every other entity uses. There is no public-sharing arm: a session carries no `SharePermission`, so a caller with no source ids has no way to reach one. Enforcement: `AgentSessionAccessLevelExtractor`, modeled on the call extractor, gates every session route in both processes - reads at `View` (get, log) in storage, mutations at `Owner` (control, delete) in the harness. Control inherits the `Owner` bar from the replace endpoint it superseded. Both router states carry the entity access service; the harness service now constructs one from its pool. `GraphqlEntityType` gains a schema variant rather than silently mapping sessions onto an existing type. Originally written against the pre-detach route shapes; adapted here to the split read/control routers and the channel-less session row.
…tract The grants written by `create` - initiator owner, mention's channel editor, atomically with the row - were implemented in the Postgres adapter but stated nowhere, which made them look like the adapter's invention rather than the port's contract. Now the port says so. The control body's flattened tag gets a decode test, since it is the one wire shape here that nothing round-tripped.
404Wolf
force-pushed
the
wolf/move-the-crud
branch
from
August 13, 2026 00:13
fa38532 to
e1c32f8
Compare
ACP's `plan` session update carries the agent's complete todo list on every change, and the fold previously logged it as an unmodelled frame. Fold it as a new `MessagePart::Plan`: the turn's first non-empty update pushes the part onto the agent message, and every later update replaces it wholesale, so a message always shows the list as it last stood. A re-emit identical to what the part already holds reports `Unchanged`, since the harness resends each state verbatim between real changes. Covered by a real recording (`plan_todo.jsonl`) registered alongside the other fixtures, plus tests pinning replace-in-place and re-emit dedup. The wire grows a `plan` part kind (types.ts regenerated), and the MagicChip presentation renders it as todo progress.
404Wolf
marked this pull request as ready for review
August 13, 2026 20:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.