[feat] Add Channels (ongoing and far from properly tested) - #6051
[feat] Add Channels (ongoing and far from properly tested)#6051junaway wants to merge 463 commits into
Conversation
chore(hosting): dev runner auto-reloads on save (tsx watch + polling)
fix(runner): let an environment own the configuration it installed
feat(sdk): let a builder tool call carry the agent's own note
feat(api): add the change-set engine for ordered config edits
refactor(runner): move session decisions out of server.ts
fix(api): serialize concurrent commits on one variant with a row lock
feat(api): wire ordered operations into the workflow commit endpoint
refactor(runner): extract the sandbox and workspace lifecycle units
feat(api): add read_config and harden the commit endpoint
refactor(runner): finish the lifecycle split with a typed context
feat(runner): read workspace files into a commit, under one root
feat(runner): change a warm environment instead of rebuilding it
feat(runner): bind an approval to the exact call that may execute
feat(runner): reopen a harness session without losing the sandbox
feat(runner): rotate a credential without deleting the sandbox
…-runner feat(runner): show the real change on the card, commit those bytes
feat(sdk): deliver the approval manifest to the live approval card
…-removal refactor (keep the Badge idiom from release, keep the defaultOpen/sectionOpenState feature from the lane, drop the stale antd imports)
…-web feat(frontend): show the frozen content and diff on commit approvals
…-mount fix(runner): steered sessions keep their workspace mount (pre-existing)
…migration feat(api): agent behavior leaves the public API: handler-mode ops, one error envelope, routes deleted
chore(clients): regenerate both API clients from the branch spec
…e benchmark's run identity is now derived from non-secret values only (base+project context hash replaces the API-key fingerprint; the record only needs to say which deployment context produced a run); the runner test asserts the fence shape with startsWith/endsWith instead of a regex CodeQL reads as HTML filtering; the web test parses rendered HTML with the test environment's DOM instead of regex tag-stripping, which is also more correct. No product code changes.
fix(ci): clear the CodeQL highs (test and benchmark code only)
The tab icon on agenta.ai did not match the docs (and every other Agenta surface). Browsers prefer the SVG favicon over the .ico when both are declared, so favicon.svg — not favicon.ico — is what actually renders, and it was the only asset in the set drawn from different measurements. The two .ico files are byte-identical; favicon.svg was hand-authored separately in the on-page-SEO commit and its transform was eyeballed: the symbol came out at 53.1% x 43.8% of the tile against ~71.5% x 58.6% everywhere else, on a #0A0A0B tile instead of #1E1C1D. Rescale the symbol to scale(2.14) at translate(73 105) and correct the tile fill and radius, which reproduces the bounding box of android-chrome-512x512.png to within a pixel: favicon.svg x[73..437] y[105..403] canonical x[73..438] y[106..404] Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the last C0 gap. The design stated "one function composes it, no exceptions" but never said how, and the function name in circulation was invented rather than specified. external_key becomes a UUID — uuid5 over the adapter's declared key fields for that grain — rather than a joined string. A join needs an escaping rule for platforms whose ids carry the delimiter (Teams), and an untested escaping rule is a latent collision that merges two conversations. Adapters declare identity.key_fields per grain and never compose; core composes. The field set is the fragile part either way: change it and every row re-keys, and a hash makes that invisible rather than merely quiet. A declaration is checkable, so WP2's contract suite holds adapters to it — including the distinctness case, since a too-small field set silently merges threads. Renames the wire field data.space.external_key to data.space.locator: a bridge sends its platform's fields, never a key, and the old name would have invited a raw string into a UUID column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review pass. Each of these was the same concept said twice, or a platform
word in a channel-neutral place.
- ChannelDeliveryState.PENDING -> CREATED. Past tense like the trigger
states, and it records what we did rather than asserting a queue we do
not have.
- Inbox `content`/`sender` move under `processed`, which is what the table
below them already called that pair. `raw` stays commented on both sides.
- Agent DTOs inherit the house `Slug` mixin instead of declaring `slug: str`.
- Dropped `name` from the agent and space queries.
- ChannelSpaceCandidate: no external_key (it would key an unconfigured
place), `name` -> `display_name`, and a note on why a view holds its
locator flat.
- Catalog routes lose the repeated domain: /channels/catalog/, not
/channels/catalog/channels/.
- text.format is markdown|html|plain, never a platform's dialect name;
identity.scope and conversation.default_unit documented.
- addressing.sigils.{agent,command} and protocol.versions grouped.
- Protocol starts at 0.1.0. CloudEvents' specversion keeps its own name and
its own 1.0 — three version fields at three granularities, now
distinguished, with parsers dispatching on event type not protocol.
- adapter port gains discover_spaces (seven methods); verify_signature
returns the installation id, not bool — verification and identification
are one act.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
build_setup_document was reachable only through the per-connection setup route, which 404s without a connection -- but the manifest is what an operator needs before they have anything to connect. Add a sibling route that takes a channel name alone and composes the request URL the same way, so the manifest is reachable in the order a human does the work. Leaves the per-connection route and the Slack adapter untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fern-generated from the updated OpenAPI spec; adds fetchChannelSetup and its request/response types to the channels resource client. No hand edits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A section on the Channels settings tab: the generated manifest (copyable text plus a pre-filled create-from-manifest link), the request URL and a tunnel note for local deployments, and a paste form. The paste form renders from the setup declaration's fields, not from hand-written inputs: a field's name, label, password-ness and required-ness all come from the API response, and values are routed to `data` or `credentials` by the field's own `secret` flag rather than by name -- the routing that keeps a locator field like api_app_id out of the vault. On save, a rejected verification shows the service's own error and leaves the form filled; nothing is retyped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Against a real Postgres, using the real SlackAdapter over a fake HTTP transport that answers auth.test the way Slack answers a bad bot token. Reads both the connections and secrets tables after the attempt, and pairs it with the success case so the failure assertion has a working baseline to be read against. Written but not run: this worktree carries no schema for these tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Slack setup for a customer-owned app: a per-channel setup route that needs no connection, a settings section carrying the manifest and its request URL, and a paste form rendered from the channel's own declaration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Building the first human-facing writer found it: the declaration names three connection identity fields, verification discovers two, and key composition refuses a partial locator. Two test fixtures carried the empty string, so nothing disagreed with the declaration until a form had to satisfy it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing previously exercised the kind-level grant path end to end: a signed is_im-shaped Slack event, no space row pre-created, one kind=private ALLOW grant seeded through the same DAO calls the write path uses, resolved by the real ChannelsService/ChannelsDAO after going through the signed HTTP route. Extends the existing seam fixture with the connection id and DAO/service handles the new test needs; the three existing tests are unaffected. Needs Postgres (the channels_scope fixture); unrun in this worktree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GrantFormDrawer required both agent_id and space_id and hardcoded effect: "allow", which made a kind-level grant impossible to submit and a denial impossible to author. Splits the agent detail screen's grant section into three questions instead of one space-only table: - GrantKindSection: a single allow/deny/unanswered control per kind (direct messages, group chats), writing (agent, effect, kind=...) with no space_id field. Effect/kind/space_id are immutable once written, so re-answering deletes the old row and creates a new one. Shows the accepted-cost notice next to a kind-level deny. - GrantChannelsSection: a picker over discovered channel candidates, writing space-level allow/deny grants; flags a granted channel with no confirmed Slack invite (not_in_channel). - GrantFormDrawer keeps the single space-level create/edit form used from the space detail screen, now with a real effect field instead of a hardcoded allow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cret Nothing wrote delivery_url or minted a bridge signing secret through the create path -- only an acceptance fixture inserted a row directly, so a bridge created the supported way accepted events and could never deliver a reply. create_connection now mints the secret itself when the caller supplies none, writes it under ChannelSecretKind.BRIDGE, and the create response carries a one-time document with the inbound URL and the plaintext exactly once; the GET setup route builds a secret-free document and is structurally unable to read one back. delivery_url already flows through ChannelConnectionCreate.data via the existing locator merge, so no code change was needed there beyond documenting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comparison did not exist anywhere in the tree before now, despite the wave calling this package's bridge work "re-verified" -- an earlier package was designed to build it and shipped without it. One DM, posted and edited, driven through SlackAdapter in process and through BridgeAdapter over the wire, both terminating at the same FakeSlackWorkspace object: the bridge side fronts Slack via a small ASGI app that replays deliveries through the identical SlackAdapter instance, so a divergence can only be the bridge's, never two fakes disagreeing. Covers a DM's thread being created and reused, a reply's content and its edit-in-place, and capability-driven button degradation, agreeing between both arms. fake_slack.py gains auth.test support, needed for a Slack connection to be created through the real write path here rather than seeded directly. The two-bridge test (bridge_process/test_bridge_two_bridges.py) was checked, not rebuilt: fetch_capabilities already reads connection.data per call and the xfail(strict=True) it once carried is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Client id, client secret and the app's own signing secret are one deployment's, never a project's, so they go through the shared env object like every other setting rather than the vault. Bare VAR= lines in the env example files; the self-host docs carry the explanation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
verify_connection discovered only team_id, bot_user_id and api_app_id, filtering out falsy values -- so enterprise_id, one of the three fields the connection's own declaration asks for, could never reach the locator. compose_external_key then raised on the missing field, so a connection created with exactly the fields a human is asked for failed before a row was written, and every caller had to know to backfill an empty enterprise_id by hand. Fixed at the one place that already knows the identity model: reuse _connection_discriminator against auth.test's own response, so exactly one of enterprise_id/team_id is populated, the other empty, matching the ingress path exactly. The caller-side filler in the web form is no longer needed. The adapter also gets what its install callback needs next: the signing secret resolves from the connection's own row for a customer-owned app or from this deployment's configuration for a hosted one (the secret is the app's, not the installation's), the declaration narrows for a hosted connection (no native commands, fill modes gated on what scopes were actually granted), and two lifecycle hooks (detect an app_uninstalled/tokens_revoked signal, revoke the installation on removal) default to no-ops for every other adapter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Install mints the existing signed OAuth state (a shorter age than the gateway's own use of it) and redirects to Slack's authorize URL; refuses with a reason, not a 500, when this deployment has not configured the three hosted-app settings. Callback decodes state first -- unknown, expired or tampered refuses before any exchange is attempted -- then exchanges the code at oauth.v2.access (the first authorization-code exchange in this repo, deliberately not shaped like the gateway's provider-delegated callback) and hands the bot token to the same verify_connection every paste-form connection already goes through. ChannelsService.install_connection is the upsert entry point: a reinstall composes the same identity, finds the row this project already holds, and edits it in place -- same id, same grants, same spaces, same threads, secret rotated -- rather than inserting a second, grant-less connection behind the first. An identity that resolves to a different project refuses rather than moving silently. Two lifecycle paths close the loop: the ingress path now asks the adapter whether an inbound payload signals a stopped installation (Slack's app_uninstalled/tokens_revoked) and deactivates the connection without touching its grants, spaces or threads; removing a connection from our side now asks the adapter to revoke the installation and supply its own removal notice, defaulting to the existing customer-owned wording when an adapter declines to override it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fern-generated from the updated OpenAPI spec: adds installSlackConnection and slackInstallCallback to the channels resource client, and ChannelSetup.hosted_available / ChannelConnectionFlags.is_hosted to the generated types. No hand edits. Regenerating from the full current spec also surfaces one unrelated drift the last regeneration missed: SessionInteractionTransitionRequest's resolution field is a plain object on the backend, not the narrower SessionInteractionResolution shape the committed client still declared. Updated the one caller to match the corrected, now-simpler type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The second button on the same section area: a new, separate component next to the existing paste-form section, not a restructure of it. Renders nothing when the channel setup declaration reports no hosted app configured for this deployment -- absent, not disabled. The button is a plain link to the install route, scoped to the project in view; the route itself mints the OAuth state and redirects to Slack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Agenta-hosted Slack app: an OAuth install and callback, a verification secret resolved from the deployment for a hosted connection and from the row for a customer-owned one, and a narrowed declaration for the scopes a hosted install actually granted. Also completes Slack's connection identity in verification, so a caller no longer has to supply a field the declaration never asks for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A bridge connection now mints its own signing secret on create and shows it once, beside the URL we post to, so an operator holds both halves of the exchange. Adds the in-process-versus-bridged comparison, which did not exist: one conversation down both paths, both connections built through the write path rather than seeded. Two collisions resolved by hand in the files the merge points serialised: the connections router's imports, and the create-connection tail against the hosted install added beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three questions decide where an agent answers, including the kind-level grant nothing could previously write, and a grant now carries an authored effect rather than a hardcoded allow. Adds the first test to drive a real direct message through the ingress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The create response gained a setup slot carrying the one-time bridge document, and the client did not know about it, so the secret the API returns could not be read by anything downstream. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tings A vendor-prefixed variable puts the attribute after the prefix and suffixes an instance; there is only ever one app here, so the extra scope word said nothing. These are also the names the platform's own documentation uses, which is where a self-hoster copies them from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seven checks confirmed, nothing dead and nothing test-only. Recorded with the counts it stands on, including the 83 tests that are written and unrun because the local stack carries a different schema, so the deployment reads as a checkpoint rather than a formality. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wave's deploy step was one paragraph saying a tunnel is needed. It now carries the settings that get registered with a provider and cannot be changed cheaply afterwards, the two URLs to register, and the instruction to run the written-but-never-executed tests before any app exists. Also files the disagreement found while writing it: the manifest derives the public host from the request and the authorization redirect derives it from configuration, and nothing checks that the two agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One conflict, in the secrets DTO tests: both sides added their own payload helper and their own cases to the same file. Both kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
WIP -- @mahmoud |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) 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 |
| except ChannelConnectionVerificationFailed as e: | ||
| return HTMLResponse( | ||
| status_code=400, | ||
| content=_slack_install_card(success=False, message=str(e)), |
| except ChannelConnectionIdentityConflict as e: | ||
| return HTMLResponse( | ||
| status_code=409, | ||
| content=_slack_install_card(success=False, message=str(e)), |
| { | ||
| "type": "bridge.receipt", | ||
| "idempotency_key": idempotency_key, | ||
| "error": str(e), | ||
| } |
Context
An Agenta agent can be reached from the product and from the API. It cannot be reached from where people already talk. This branch adds a channels domain: a platform conversation arrives over HTTP, gets matched to a connection and a space, gets checked against a permission rule, runs an agent, and gets an answer posted back on the same thread.
Three channels live on it. Slack, in two installation models. A bridge, for a platform we do not implement in process, reached over HTTP with a shared secret. And an Agenta channel, which needs no platform credentials at all, which is what let the whole path be proven on a laptop before any real platform was involved.
The title is accurate and the draft status is deliberate. The most recent wave is merged and has never run against a real deployment. 83 integration and acceptance tests are written and none has executed, because no running stack has ever carried this schema. Nothing about a seam is proven yet.
Changes
The domain follows the house layering.
core/channels/holds the DTOs, the service and the adapter port.dbs/postgres/channels/holds the DAO.apis/fastapi/channels/holds the routes, split into an authenticated router and a public ingress. Adapters live undercore/channels/adapters/<channel>/, and the composition root builds one registry that every caller shares.One adapter port, per channel. An adapter declares what it can do and what it needs to be given, verifies an inbound signature, parses a payload into a neutral event, and renders an outbound answer. Everything else is shared. That is what makes a second platform a folder rather than a project.
Permission is deny first, and it can name a kind rather than a room. A grant carries an effect and matches either a specific space or a whole class of them. The class case is the one that mattered: a direct message is the first thing an operator tries, and a space that has never been opened cannot be pre approved by name.
Slack setup, in the order a human does the work. The blocking defect was reachability, not capability. The manifest a person needs before they own anything sat behind a route that required something to already exist:
The paste form renders from the channel's own declaration rather than from field names written into a component, so a second platform's form is its declaration and nothing else. Values route to the vault or to the connection row by the declared
secretflag, never by name.Two installation models, one adapter. A customer builds an app in their own workspace from the manifest and pastes back what it gives them. Or they install the app we own in one click over OAuth. The signing secret is per app, not per installation, so a hosted connection stores a bot token and nothing else, and the adapter resolves its verification secret from the connection for one model and from the deployment for the other. A deployment that sets no hosted credentials does not offer that button at all and refuses the route with a reason rather than a stack trace.
The bridge shows the operator both halves of its exchange. It mints its own signing secret on create and returns it once, beside the URL we post to. It is the only credential in the system we issue rather than receive.
The defect this domain kept producing
Four capabilities shipped here that worked and could not be reached, and every test suite passed each time. The most recent was found in this branch's last clean up phase:
So
POST /bridge/events/answered 404 for every request in every composition root, and the outbox worker could not deliver a reply. Every bridge test passed, because every bridge test builds its own registry with the bridge in it. The registry's own comment said it was missing.The fix is one line. The guard is the useful part: a test now fails when a channel has a contract suite but neither a registration in the real factory nor a written exemption. The verification phase after the final merge found nothing dead and nothing test only, which is a first for this domain, and that guard is why.
Tests and notes
unit/sessions/test_stream_fill_missing_postgres.pycome frommain, not from here. It is a unit test that opens a Postgres connection and does not guard, so it errors rather than skips.docs/design/channels-research/v2/. They are most of the added line count.What to QA
Nothing here can be exercised without a deployment, and two of the settings get registered with Slack, so they are expensive to change afterwards.
Before bringing it up. Set a reserved tunnel domain, because Slack stores both URLs and an address that rotates on restart invalidates both. Point
AGENTA_API_URLat that same domain, because the OAuth callback composes from configuration while the manifest composes from the request. Set all three ofSLACK_CLIENT_ID,SLACK_CLIENT_SECRETandSLACK_SIGNING_SECRET, or none: the hosted button hides unless all three are present, which is a check rather than a bug.Register these in the app:
Run the 83 written tests before creating any app. They cost minutes and they fail closer to a cause than a manual flow does.
localhost. A page browsed on localhost produces a manifest carrying a localhost request URL. Slack accepts it and no event ever arrives, so the symptom is silence.auth.testpasses, the token lands in the vault and never on the connection row, and a wrong token leaves nothing behind and shows Slack's own reason.