[feat] Channels: Slack, bridge, and Agenta channel (takeover of #6051) - #6644
Draft
mmabrouk wants to merge 484 commits into
Draft
[feat] Channels: Slack, bridge, and Agenta channel (takeover of #6051)#6644mmabrouk wants to merge 484 commits into
mmabrouk wants to merge 484 commits into
Conversation
…-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>
Researched against the platforms' own docs and written into channels.md,
each with the values that are uncertain or tier-dependent called out rather
than rounded, and every non-obvious number cited.
The research broke the schema in four places, all of them one scalar being
asked to hold two real values, so the schema changed:
- files splits by direction. Telegram is 50 MB up / 20 MB down and Discord's
bot cap is not its user cap, so one max_bytes was wrong for two of three
first-class channels.
- Limits are now stated as "what the adapter renders against", not the
largest the platform documents. Slack publishes 4000 as guidance while
Block Kit enforces 3000; Telegram is 4096 plain but 1024 for a caption;
Discord 2000 plain but 4096 in an embed. Under-declaring degrades,
over-declaring fails at send.
- identity.scope can depend on the install, not just the platform: Slack
under Enterprise Grid issues one global user id. Declare the narrower
workspace value and let the wider install be a superset — the reverse
cannot distinguish two people sharing an id across workspaces.
- buttons.max: Slack has two live APIs with two real caps (legacy 5,
Block Kit 25). Declare the one the adapter builds against.
Grouping pass over the declaration, same theme as the last commit:
rendering.controls.{update,ephemeral}, addressing.commands.{native,
in_conversation}, conversation.default, identity.keys.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gate Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
C0 of the channels workstreams: the frozen interface surface every parallel package codes against. Declarations only — no behaviour, no persistence, no routes mounted. - core/channels/dtos.py: entities, flags, policy documents, capability declaration, requests and responses - core/channels/types.py: the domain exception hierarchy - core/channels/interfaces.py: ChannelsDAOInterface - core/channels/adapters/interface.py: ChannelAdapterInterface, seven methods - core/channels/utils.py: compose_external_key and the key derivations; resolve_policy left NotImplementedError for WP1 - entrypoints/routers.py: the Channels tag and the mount scaffold compose_external_key is uuid5 over the adapter's declared identity.keys for a grain, so no platform field set is hardcoded in core. The seed tests assert the properties WP2's contract suite will hold every adapter to: grain distinctness, canonicalisation, the no-threads None, and a raise on an incomplete locator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds ChannelsIngressRouter with the two literal public routes (/slack/events/, /bridge/events/) per entities.md §9: signature verification through WP2's adapter port, dedup-tolerant write through WP1's service, bounded-timeout enqueue onto WP4's dispatch_task, and nothing else -- no routing or resolution, which stays WP4's. WP1's ChannelsService and WP2's ChannelAdapterRegistry don't exist in this worktree yet, so ingress.py types against them under TYPE_CHECKING and the test suite fakes both to their spec'd shape. Adds the four _PUBLIC_ENDPOINTS entries per route (bare, /api/, /preview/, /api/preview/) to middlewares/auth.py -- WP3's sole owned edit to that file -- plus a guard test proving a bare "/channels/" prefix is never registered and that WP8's config routes under /channels/ still require auth. api/entrypoints/routers.py wiring is left for the C1 checkpoint merge per the collision rule in workstreams/README.md.
…t suite Adds ChannelAdapterRegistry (mirrors TriggersGatewayRegistry) and normalise_capabilities (zero-clamping, absurd-value clamping, unknown-key drop via pydantic's default extra=ignore, identity.keys passthrough). Ships the reusable adapter contract suite as run_contract_suite(adapter), a plain async function any package can call against its own adapter — gated assertions for controls.update, buttons.max, backfill, signature verification, and four identity.keys assertions (distinctness, canonicalisation, incompleteness, no-threads). Proven against a well-behaved fake and three deliberately lying fakes (edit-is-a-new-post, ungated buttons, too-small identity.keys), each producing an AssertionError naming the violated capability. Fixes a C0 shape bug found while writing the bridge.hello test: ChannelConversation.units/default were typed as ChannelSessionScope (thread|message, the policy-level enum) instead of ChannelKeyGrain (thread|space, capabilities.md's actual declared vocabulary) — a platform with no threads degenerates to "space", never "message". Corrected in dtos.py and the matching seed test fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ce, resolve_policy, migration oss000000021
Builds the channels domain's persistence stack per entities.md §2-3, §7-8 on
top of the frozen C0 seed (dtos.py, types.py, interfaces.py, adapters/interface.py):
- dbs/postgres/channels/{dbas,dbes,dao,mappings}.py: the seven tables, every
constraint/index from §3, and a ChannelsDAO implementing ChannelsDAOInterface
following TriggersDAO's session-per-call, ON CONFLICT DO NOTHING/DO UPDATE
shapes (record_inbox_event dedups to None, record_outbox_event falls back to
a fetch of the existing row, transition_* update in place by id).
- core/channels/utils.py: resolve_policy, the pure five-level intersection
(capability -> channel default -> agent/space/grant), with decided_by
tracking which level produced each field per D25.
- core/channels/service.py: ChannelsService's configuration and policy thirds
(agents/spaces/grants/threads, fetch_capabilities, resolve_effective_policy,
discover_spaces); routing/delivery methods declared per §8 and left raising
NotImplementedError for WP3/WP4/WP5.
- oss000000021: the one channels migration; applies and downgrades
symmetrically (verified via alembic's offline SQL renderer). Carries a
coordination note for WP7's identity-link tables, added once WP7's own
worktree defines their shape.
- Flagged gap: entities.md §8 assigns close_thread a real WP1 implementation,
but threads are append-only with no edit_thread in the frozen §7 list. Added
ChannelsDAOInterface.close_thread (update flags.is_active in place by id,
mirroring transition_inbox_trigger) since nothing else lets the service do
what its own spec requires — flagged for the WP1/WP7 checkpoint.
- Tests: DTO/exception/compose_external_key coverage plus new DAO-level
integration tests (inbox dedup+ordering, trigger dedup+transition, outbox
conflict-returns-row+one-row-for-its-life, the two partial-unique-index
rejections, entity round-trips, migration round-trip), all under
pytest.mark.integration with a reachability skip mirroring sessions'
pattern. resolve_policy tests cover every case in specs-wp1.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ation Verified all 115 items against the actual code (dbas/dbes/dtos/types/ interfaces/utils/service/dao/mappings/migration) and the unit test suite rather than trusting the commit message. 107 checked, 8 unchecked. Ran the offline suite (26 passed) and confirmed the 23 DAO/migration integration tests skip on the Postgres-reachability probe in this worktree (unreachable here) rather than having actually proven anything — those stay unchecked with a "Deferred to C1" annotation instead of being marked done. Flagged in place, without touching code: the documented close_thread gap (DAO method added beyond entities.md §7's frozen list, with its own docstring explaining why), a count_grants SELECT-id+len() vs SELECT-count(*) shape difference, and the known resolve_policy/capabilities.md conversation- units vocabulary mismatch (constructing units=["space"] today raises a Pydantic ValidationError one layer earlier than expected, since ChannelSessionScope has no SPACE member -- confirmed by reproduction). Added an "Open at checkpoint C1" section grouping the 8 unchecked lines into needs-a-deployment (6, covering 23 test functions), blocked-on-WP7 (1), and genuinely-unfinished (1: no per-exception .message unit test).
Restores f4748b9772, which a concurrent task-list agent discarded with a reset. The test layer is the folder: run-tests.py resolves oss/tests/pytest/<layer>/, so DAO and migration tests under unit/ were only reachable via an explicit path and self-skipped on a Postgres probe when run. Moves the six DB-dependent files to integration/channels/ and drops both skip probes. An integration run with no database now fails instead of reporting green; the unit layer is env-free: 26 pass, 0 skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the one genuinely-unfinished item the reconciliation found: all ten ChannelsError subclasses now assert .message, str(e) == e.message, and catchability as the domain base, plus that ChannelSignatureInvalid carries nothing but the channel. Also repoints the ledger's deferred annotations at integration/channels/ and drops the skip-probe language, which no longer describes the tests. Unit layer: 49 pass, 0 skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The serialised collision-file edits no package could make, plus the one cross-package defect the merge surfaced. - entrypoints/routers.py: ChannelsDAO + ChannelAdapterRegistry (empty until WP6) + ChannelsService, and the ingress router mounted at /channels and /preview/channels. dispatch_task stays None until WP4. - resolve_policy: conversation.units is a grain vocabulary (thread|space), not a scope one (thread|message). MESSAGE is always available and never declared, so the capability ceiling can only take THREAD away. Indexing a grain against SESSION_SCOPE_ORDER raised ValueError for any platform without threads -- including contract.md's own bridge example. Unit layer: 2202 pass, 0 channels skips. Both ingress routes appear in the OpenAPI schema and all eight path variants resolve as public; configuration routes under /channels/ stay authenticated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It spawns `pnpm exec tsx` against services/runner and executes that component's TypeScript -- its own section header already called it an integration test while it sat in unit/. run-tests.py resolves the layer by folder, so `--unit` was running it. Also fixes the guard: it skipped on missing pnpm, but the real dependency is tsx, which only exists after `pnpm install` in services/runner. In a fresh worktree pnpm resolves and tsx does not, so instead of skipping the CLI printed 'undefined' and the test failed on a JSON decode error that named nothing useful. SDK unit layer: 1974 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SQLAlchemy's Enum() persists the member NAME, and every other enum type in the database is uppercase for that reason (folder_kind_enum APPLICATIONS, secretkind_enum PROVIDER_KEY) even though those DTOs also carry lowercase values. The channels migration created lowercase labels instead, so every insert failed: invalid input value for enum channelspacekind: "TOPIC". Fixes the migration rather than the column: keeping Column(Enum(X)) bare means channels reads like the rest of the codebase, where a values_callable on five columns would not. Also drops the migration round-trip test. A round-trip downgrades, which against a shared dev database drops the channels tables from under whatever else is using them; the task ledger records it as verify-by-hand. Channels integration: 22 pass against a real database. Unit: 2202 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
env.py hardcoded @postgres:5432, the compose service name, so anything run outside the network -- every integration test -- could not resolve it. The .env.*.dev files cannot carry localhost either: run.sh feeds the same file to compose via env_file, so an uncommented POSTGRES_HOST=localhost would break every container. Adds POSTGRES_HOST defaulting to "postgres", so with the variable unset all three composed URIs are byte-identical to before -- verified for both editions. An explicit POSTGRES_URI_* still wins, and POSTGRES_PORT still does not feed the in-network URIs, as its comment requires. py-run-tests exports POSTGRES_HOST=localhost (only when unset), so load-env alone is now enough to run the integration layer: 22 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… is unreachable Attachment teardown reads the object store directly to prove the object is really gone. The bundled SeaweedFS publishes no host port at all -- compose exposes 8333 only on its own network -- so unlike Postgres this cannot be reached by pointing at localhost, and the test failed on name resolution. Follows the Postgres guard already in this conftest: a probe plus an autouse skip, keyed on a new store_required marker so only the file that reads the store directly is affected. Its sibling attachment tests go through the API and are untouched. These two only surfaced now because making POSTGRES_HOST configurable let the existing Postgres guard pass, so the tests got one dependency further. API acceptance: 802 pass, 15 skip, 0 fail. 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>
Both this branch's add_channels migration and the release's add_session_streams_references claimed revision oss000000021 (the collision arrived when the branch merged v0.112.1). Alembic stops with 'multiple head revisions', the API never starts, and no deployment of this branch can migrate any database that includes the release. Renumber add_channels to oss000000022 on top of the release migration. Caveat for existing dev databases that already applied the old id: bump the alembic_version_oss row from oss000000021 (channels) to oss000000022 by hand, or the runner will try to re-apply the channels DDL.
…-channels # Conflicts: # api/oss/tests/pytest/unit/secrets/test_dtos.py
The channels work continues on this branch. This merge takes JP's feat/add-channels branch onto the current release branch and resolves the 18 conflicts: - The pi_agenta harness is gone on the release, so its adapter, template, and tests are dropped. The mock harness stays, because the channels integration and acceptance suites drive it. It now carries the platform instructions like every other harness. - The secrets DTO keeps the release's shared validator and adds the channel secret branch to it. The channels service builds the vault update payload with the release's update DTO. - The web preferences page keeps the release flags and adds the Agenta channel probe flag. The voice input flag no longer exists. - The channels migration is renumbered to oss000000029 on top of the release chain. - The streaming SDK test takes the branch version, whose CLI boundary test moved to the integration tier. Claude-Session: https://claude.ai/code/session_01R3RytXiaiv24fGoxd8KkXN
Slack puts thread_ts only on messages inside a thread; a top-level message has none. The THREAD key composition declares thread_ts required, so every plain channel message raised ChannelLocatorIncomplete in the dispatch worker. Slack showed nothing, with no error anywhere the operator can see. parse_event now falls back to the message's own ts, which is Slack's own threading model: a top-level message roots the thread its replies live in (a reply's thread_ts IS the parent's ts). This also makes the agent's answer land as a thread reply under the mention it answers. The one existing test that pinned the old shape asserted a locator the rest of the system could not consume; it now pins the new semantics: a parent and the replies threaded under it resolve to one unit. (cherry picked from commit f26abb5)
The delivery worker's first post for an item has no receipt yet, and that branch passed an empty locator to the adapter. The Slack adapter needs locator['channel'], so it raised KeyError and the first answer on any thread was silently never delivered — the playground showed the answer, Slack showed 'Working…' forever. The thread row already holds the full target (team, channel, thread_ts); _send now takes the thread and uses its locator for the no-receipt post. Same defect class as the bridge's F63: a read with no writer, surviving because fixtures seeded the receipt directly. (cherry picked from commit 60a54b1)
…ry path Three defects found and hotfixed during a full day of live QA on Slack, then run for the rest of the day against the live deployment. The bot's own indicator edit came back as a `message_changed` event whose author sits in the nested message, so the authorship check read it as human input: the adapter rooted a phantom thread off the edit's synthetic ts, ran a paid turn, posted, edited that post, and went round again. Edit and delete subtypes now drop at parse, and authorship is read one level down too. A turn-ended event can outrun the final record commit, so the fold read an empty answer and rendered an empty Slack section, which Slack rejects wholesale -- the indicator then sat on "Working..." forever. The read is now retried twice before rendering; an empty section block is never emitted; a still-empty fold leaves the indicator alone and logs loudly rather than delivering a blank bubble; and a blocks rejection falls back to text-only with the rejected payload logged. (cherry picked from commit 0b62e15)
…ploy Thirteen entries, F69-F81, from deploying the branch on bighetzner against a real Slack workspace on 2026-08-14: the migration id collision, the connection create that could never succeed, the missing url_verification handshake, the top-level thread_ts crash, the unresolvable 'main' reference key, the empty-locator first post, the approval-surface trio, the pending-choice replay, dropped deliveries, lost turn-ended events, the space_kind pass-through gap, backfill concatenation, and the first-ever run of the written suites. Three carry fixes in review already: PR #6070 (F69), PR #6071 (F72), PR #6072 (F74). Three more run as local hotfixes on the test stack and need a design decision each (F70, F71, F73). (cherry picked from commit 0cafeaa)
Thirty findings from a full day driving the branch on a real Slack workspace, the agenta bench as a no-Slack control, and a real Telegram bridge. Two are fixed in PR #6079 (the bot-echo cascade, F82; the empty-answer delivery path, F85). One is a product decision rather than a defect (F86, the durable agent folder spanning every space and user). Eight came from building a bridge against the contract, F97 and F105-F109, and F97 corrects F63's premise: the create path does write delivery_url and delivery round-trips cleanly once capabilities exist, so the missing piece is the hello route alone. The rest span the trigger policy nobody reads, forwardfill's flood-or-amnesia key mismatch, two edit models that treat omitted fields as "reset" (one of which permanently bricks a connection), the ungated `ask` permission mode, and the approval surface that cannot work under per-message session scope. (cherry picked from commit 595ce53)
Two unit tests from the release did not know the channels additions: - The commit-time harness validator now lists mock among the allowed harness kinds, since the mock harness stays on this branch. - The redaction map that names each secret kind's primary credential gets an entry for channel secrets, so the probe route and the value preview treat the bot token as the credential. Claude-Session: https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
This was referenced Sep 8, 2026
Closed
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
3 participants
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.
Context
JP's channels branch (#6051) had no owner after he left. This PR carries that work forward on the current release branch. It is a draft while the live QA and the fix stack land on top of it.
What this branch contains today:
feat/add-channelsmerged ontorelease/v0.115.4, with the 18 conflicts resolved. The removedpi_agentaharness stays removed. The mock harness stays, because the channels suites drive it.oss000000029on top of the release chain.What comes next
Stacked PRs on top of this one, each small and reviewed: the decisions from the findings triage, the feature flag, and every defect the live Slack QA finds.
The takeover page and the decision record live under
docs/design/channels-research/v2/.https://claude.ai/code/session_01HXt8WBUK9MrWKNTYbUKpaT