Repository navigation
orchestrator: git forge, chat and model router are roles; LiteLLM and GitLab fill them - #1793
Draft
nikhilunni wants to merge 38 commits into
Draft
nikhilunni wants to merge 38 commits into
nikhilunni wants to merge 38 commits into
Conversation
…hat name a provider Two tests that hold while the provider role seams land. The hash test keeps the PR review and Slack threads definitions byte-identical, because a changed hash makes the boot seed stop every active run. The guard test lists the 48 files outside src/providers that name GitHub, Slack, or OpenRouter. The list only shrinks. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…ovider POST /api/v1/integrations/:provider/events replaces the four per-provider route files at the same URLs. The connector's declared webhook ingress (scheme and sealed secret) now selects the verifier; before, the declaration was read by nothing and each route wired its own. A provider supplies only what a verified delivery means, under src/providers/<name>/events.ts. The private secret caches and HMAC code for GitHub and Slack are deleted in favour of the shared resolver in automations/verify.ts, and the Slack interactivity route verifies through it too. An event path is exempt from the IAP bridge when its connector declares an ingress. This adds the Vanta path, which was mounted but never exempt, so its deliveries could not reach the route on an IAP deployment. Review pass: removed history comments from the ingress spine and the IAP bridge, dropped the retry-number debug log, and a source whose connector declares no ingress is now a startup error. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A provider declares a ModelRouterSpec: its connector, protocols, default and title models, catalog parser, model page URL, chat client, and an optional native decisions client. The registry derives the key's secret name from the connector. OpenRouter moves to providers/openrouter/ and is the only spec for now. Chokepoints, each replacing copies: - resolveRouterSelection checks router, harness protocol, and model for the profile, automation, and task-create RPCs. - connectedRouters is the only reader of "which routers have a key"; the inline secret listings in task-create and the model-routers RPC are gone. - model-routers/client.ts makes every in-process model call (task titles, the decide block) on the first connected router. No caller names a router. Behaviour that moved, not changed: - The decisions default model lives in the OpenRouter client, not in the decide block. The wire request is the same. - The decide block makes one call. The runtime returns null when no connected router answers, which replaces the separate connected() probe and its second listing of secret names. - The title consumer gets null from the model seam when no router is connected, which replaces its own connection check. The review pass removed the two duplicate secret-name client interfaces, an `as unknown as` cast on the org-secret client, and the hard-coded model page URL in the RPC. The guard list drops ten files. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…vider on Fixes from the review of the shared event route: - An unknown provider falls through to the next handler, so a later route on the same path pattern can still answer. It was a 404 from this route. - The route owns the clock. The ingress seams no longer take a second `now`, which could disagree with the one used to verify and extract. - The interactivity route takes the same injected clock. - The header comment said the route always answers 200; a provider's post-delivery hook can answer otherwise. New tests: the default sources equal the connectors that declare a webhook ingress; an unknown provider is 404; a wrong secret and an unreadable secret are both 401 and write nothing to the ledger. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Fixes from the review of the model-router seam: - The catalog refresher listed every org-secret name once per router. It now reads the connected routers once per pass. `isRouterConnected` had no other caller and is deleted. - `selectRouterProtocol` and `DecisionsClientOptions` are private; nothing outside their files used them. - The policy RPC returns early for an unknown router. - Two comments are fixed: the `draftBlockCatalog` doc sat above another item, and the `orgSecret` dep described a use it no longer has. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A LiteLLM proxy is a model router: sessions launch through it on both harness protocols, the catalog comes from its model list, and titles and decide blocks can run on its models. What the seam needed for a router whose models are the admin's own: - A model per role. A router declares built-in models for `default`, `title`, and `decision`; LiteLLM declares none. The new `router_settings` table holds an admin's choice per role, and `UpdateModelRouterSettings` sets it. `routerModels` is the one reader: the admin's choice, else the built-in model. OpenRouter's pinned decision model moves from its client into its spec, so an admin can change it the same way. - Decisions without a native endpoint. `decideByChat` answers the same contract with one structured chat completion. Its numbers are the model's own estimates, not calibrated probabilities. - In-process calls use the first connected router that has a model for the role, so an org with only LiteLLM gets titles and decisions once an admin picks the models. - A routed launch with no model and no default fails with a message that says so. It was "a routed launch requires a model". - The Codex adapter names its provider "router", not "openrouter". The name is local to the config file the adapter writes at each launch. Limits, by design of the platform and not of this seam: the proxy must answer over HTTPS on port 443 at the root path, because the egress proxy intercepts only that. A scheme, a port, or a path prefix needs an egress change first. The migration adds one table and changes no existing one. On a rollback, a profile that names `litellm` fails closed as an unknown router. `@ai-sdk/openai-compatible` is pinned to the release built against the same provider package as `ai`; a newer one fails the type check. Not verified against a live proxy: the `/v1/models` body shape and the Codex base path follow the OpenAI API, which LiteLLM documents as compatible. The dev-stack check for this PR covers them. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…ings Fixes from the review of the LiteLLM router: - A decide block that names its own model goes to the first connected router. It returned "not decided" when that router had no decision model. - A failure to read the decision model is a retryable error. It was a plain error, which failed the block for good on a short database fault. - The chat decision call has a 20 s timeout and no SDK retries; the decide block makes the one retry. It had no timeout and up to three attempts. - A choice answer is a plain string in the schema. An enum of every option label can pass the limit a strict schema accepts; the decide block already rejects a label that is not an option. - `UpdateModelRouterSettings` keeps a role whose field is absent and clears one whose field is empty. It replaced all three roles. - A chosen model must be in the router's catalog. A decision model on a router with a native decisions endpoint is exempt, because that endpoint serves models the chat catalog does not list. - A router with no host set is a typed error. The router list shows no egress host for it and passes any other failure on; a routed launch answers FailedPrecondition with what to set. - The permanent-error check for a model call has one definition, shared by the title consumer and the chat decision. Not covered by a test: the router choice inside `decide`, which reads the default control-plane and database clients. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…through the proxy
Two delivery paths, one rule: a forge that mints a short-lived credential
serves it to the guest's askpass helper; a forge with a static token gets
it from the egress proxy as a header, so the token never enters the guest.
Coordinator:
- `fetch_git_credential` is the one place that picks a forge and mints. It
replaces two copies of `resolve("github")` and two copies of the Basic
check. The forge is the provider among the session's capabilities that
serves the requested host.
- This closes a gap: a session with a broker token and no capability for
the forge could ask for a default-scoped credential. It now gets none.
A failed capability read is an error; it was read as "no capabilities".
- `session_forges` is the one definition of "the session's forges", used
by the credential path and by the forge env injection.
Header templates:
- `render_header_template` in engram-core replaces four copies of
`template.replace("{}", secret)` in the proxy and the coordinator. It
adds `{basic:USER}`, the HTTP Basic credential base64(USER:secret), for
a server that takes a token only as a Basic password.
- A connector operation may carry its own `template`, which replaces the
credential's on that operation's path. One validator checks every
template in a connector; it replaces two copies.
GitLab:
- Clone, fetch and push over HTTPS work with `repository:read` and
`repository:write`. The proxy adds the Basic header on the four smart
HTTP paths; every other path on the host stays gated as before.
- The host is a setting, default gitlab.com, so a self-managed instance
works. A connection with no stored setting keeps gitlab.com.
- The reach of a static token is the token's own: the grant gates the
operation, not the repository.
Not changed: the guest still defaults the askpass host to github.com in
three places. That is correct while GitHub is the only forge that mints;
a second one needs the host passed to the guest.
Not verified against a live GitLab: that the proxy-added Basic header is
enough for git with no credential helper. The dev-stack check covers it.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Azure DevOps Services takes a personal access token as an HTTP Basic
password with an empty username, on the REST API and on git. The connector
declares that once, as the `Basic {basic:}` template, and the egress proxy
adds it; the token never enters the guest.
Powers: `code:read` (the git REST API, clone, fetch), `code:write` (push),
`pull_requests:write` (create, update, threads and comments),
`work_items:read` / `:write`, `pipelines:read` / `:write`. The paths come
from the REST API reference at api-version 7.1.
Limits:
- The grant gates the operation, not the organization or repository: the
reach is the token's own.
- Paths match by case. A client that spells `pullRequests` another way
than the reference does is refused.
- No connection test: every path starts with the organization, which the
connector does not know.
- No webhook, so no built-in review yet. A service hook has no signature;
it authenticates with Basic or a header, which needs a new ingress
scheme.
Not verified against a live organization.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A git operation's permission depends on the request path and, for
`info/refs`, on the `service` query. The default glob lets `*` cross `/`
and `?`, so a rule for the fetch paths also took a push whose query held
the fetch suffix: a session with only the read grant could push.
A `path-query:` glob matches `PATH[?QUERY]`: the path part globs the
request path only, the query must be equal, and a path with a `%` escape
or a `..` never matches. The GitLab and Azure DevOps git rules use it.
GitLab remotes must end in `.git`, which keeps the rules off the REST API
paths.
Also from the review pass:
- The connector validator refused fewer templates than the renderer
misreads. A `{basic:` that is not a whole placeholder is now an error.
- An operation template needs a credential with one header: one template
cannot render two.
- A git credential request for a host the session has no forge for is a
403, not a 500.
- `SessionForge` carries the capability provider it was resolved for;
`inject_forge_env` and the error text read it in place of asking the
integration a second time.
- Two comments still described the header template as `{}` only.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The review product, the PR-link consumer and the forge actions called GitHub by name. They now reach a code host through `GitForge` (`forges/forge.ts`): read a pull request, list its review comments, post a review, find a marker, build and parse a pull-request URL, and tell a permanent failure from a transient one. `forges/registry.ts` lists the forges by connector provider. GitHub's implementation is `providers/github/forge.ts`; its webhook classifier moved beside it. What the seam replaces: - The control plane, the target hydrator and the review blocks take the forge from the review's provider. A target of a provider that is not a forge fails as before. - `<provider>.post_pr_review` is one action, registered for each forge. - Two scans of a pull request's reviews for a marker (one of them with no production caller) are one `hasReviewWithMarker`. It reads up to 1000 reviews; the action's scan read 150. - The comment marker guard asks the forge, so a connector that is not a forge skips the scan instead of calling a GitHub path on its host. - Handles and trigger scopes fold case by one rule, `canonicalIdentifier`, which asks the forge. A set of provider names and a second copy of the rule in dispatch are gone. - Four copies of the owner/name pattern are one `SAFE_REPO_PATH`. It is a charset rule and allows more than two path segments. `pr_ref` gains `provider`, and its unique key is (provider, repo, pr_number), so two forges can hold the same repository name and number. The PR-link consumer reads the provider from the asset event and skips an asset from a provider that is not a forge. `lookup_pr_session` and `review_open_pass` configs with no provider mean GitHub. Rollout: the migration replaces the unique index, so a replica of the previous build fails its PR-link upsert. The consumer retries the event without moving its cursor, so the link lands when a new replica takes the session. After a rollback, new PR links wait until the roll forward. The review pass also turned up: - The control plane's clone step had no caller: the review built-in clones in its own block and always asked to skip it. The step, its `skipClone` flag, `REVIEW_CAPABILITIES`, and the tests that drove the clone through the exec journal are removed; the staging methods take only what they read. - `linear.create_comment` was on the builtin allow-list with no implementation. A test now holds the list equal to the action table. - `BuiltinActionDeps.runOp` had one reader, the marker scan. The live-Postgres test for the new key runs in CI only; no local database was available. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
`blob.backend`, `kek.provider` and `secrets.backend` (control-plane chart) and `blob.backend` and `egress.caSource` (host-fleet chart) defaulted to their GCP values, so an install on another cloud that missed one got a GCP backend. Each is now empty by default, and the render fails with the valid choices when one is unset or unknown. A neutral default such as `local` would be worse: an install that set only its bucket would store chunks on the pod's disk. The host-fleet chart also fails when the bucket for the chosen blob backend is empty; it rendered an empty bucket name before. Upgrade: an install that relied on a default must set the value before it takes this chart: `blob.backend: gcs`, `kek.provider: gcp-kms`, `secrets.backend: gcp`, `egress.caSource: gcp-secret-manager`. The example overlays already set all of them. `helm upgrade` fails at render, before it changes the release. CI renders each chart with each selector unset and requires the failure. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…e review fixes A review of the git-forge seam found that the index swap stalls sessions during a roll. A replica of the previous build upserts with `ON CONFLICT (repo, pr_number)`, which fails once that index is gone. The listener retries the event without end, and a session waits for its consumers to drain before it runs its terminal handlers or releases its lease. So each session that opened a pull request on an old replica stayed leased until that replica stopped, and after a rollback until the roll forward. The migration now only adds: the `provider` column and the (provider, repo, pr_number) index. `pr_ref_repo_pr_number_unique` stays, so both builds upsert. The migration file changed in place; no build with it was deployed. Until a follow-up drops the old index, two forges cannot both link the same repository name and number. The PR-link consumer logs that case and continues, because a retry cannot succeed. Other fixes from the review: - `SAFE_REPO_PATH` refused no `.` or `..` name, so `a/..` passed a rule that says it is safe in a shell command and a URL path. - A 2xx forge response with a body that is not JSON, or not a list, is a `ForgeRequestError` with its status. It was a plain `Error`, which the review action did not convert to an action error. - `findCommentWithMarker` returns null for a 2xx body it cannot read, as its contract says. It threw. - `PrContext` moved to `forges/`: the seam imported it from the review product. `readPrContext` moved to `providers/github/pr.ts`, so the webhook classifier is free of I/O imports again. - Comments: `parsePullRequestUrl` does not check the host; two coordinator comments said a pull-request asset has the provider "forge". Not changed here, and due with the second forge: `postReview` reports inline comments as all posted or none, and the review built-in's clone step names GitHub's host and ref. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The Decide block, smart routing and the block menu asked whether the router with id `openrouter` had a key. They now ask `useCanDecide`: a router with a key and a decision model, which is the server's own rule. `useConnectedRouters` and the block spec's router id are gone; a block kind states only that it needs decisions. The copy names "a model router". The model routers panel: - sets the default, title and decision model of each router. A field saves on blur or Enter, and an empty value clears the role. The listed models are suggestions, because a native decisions endpoint serves models the catalog does not list; the server checks the id. - links to the connector's page when a router has no host yet, which is the state of a router with an admin-set base URL. `ModelRouter` gains `connector` for that link. - labels the key field and the model link with the router's own name, and shows no model link when the router has no model page. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The session relay, the relay and choice blocks, the identity gate, the channel picker and the interactivity route called Slack by name. They now reach a chat tool through `ChatProvider` (`chat/chat.ts`): the thread renderer, a user's email address, the channel list, and a thread's workstream handle. `chat/registry.ts` lists the providers. Slack's implementation and all of its modules are under `providers/slack/`. What the seam replaces: - A relay row's `provider` selects the renderer. The relay wrote "slack" and built the Slack policy without reading the row. - `relay_session`, `slack_choice` and `run_agent` take any registered chat provider. A config with no provider means Slack, as before. The block type names and the `slack_answer` signal name are stored, so they stay. - Three copies of "post the closing message and remove the relay" (the session's end, `relay_close`, `close_agent`) are one `closeRelay`. All three now settle the mentions a re-point left mid-turn, and remove the relay even when the post fails. Only the session's end did so before. - `resolve_user` asks the chat provider for the email address. The user lookup is in `chat/identity.ts` and has no provider check. - A chat provider's interactivity app mounts at `/api/v1/integrations/<provider>/interactivity`, and that path is public only for a provider that has one. The literal route and public path are gone. - The channel walk and the four Slack actions moved to the provider. The action table spreads them in. Fifteen files leave the provider-literal guard list. The review pass also turned up: `relayFactsOf` had no use apart from the three close paths; `index.ts` imported the Slack client for a side effect that nothing needed; header comments in the relay, the policy and the resolve-user block described retired workflows. `dbos-workflow-hashes.ts --check-only` reports a changed body for `AutomationRunWorkflow`. The function is the same as on the base commit, so the report does not come from this branch. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…nflict With two unique indexes on `pr_ref`, Postgres guards only the index that `ON CONFLICT` names. Two first inserts of the same pull request at one time can both pass that check, and the second then fails on the (repo, number) index. The consumer logged that as another forge's row and dropped the second writer's fields. It now runs the upsert once more, which finds the row and updates it, and skips only when the second attempt has the same conflict. Also from the review: - `SAFE_REPO_PATH` refuses a name that starts with `-`, which a command reads as an option when the path is a bare argument. - A pull request response with no head or base is a `ForgeRequestError`, as the other unusable 2xx bodies are. - A posted review whose response body cannot be read gives no review id. It failed the action after the review was on the pull request. - The marker scan's action error keeps the forge's message for a 2xx response; "returned 200" read as a success. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
From the review of the model-router panel: - A role field cleared its draft when the save started, so the old value showed until the refetch, and a failed save lost what was typed. The draft now stays until the save succeeds. The fields are not disabled during a save, which took focus from the next field; a save in flight only stops a second send. - The panel says that an empty role uses the router's built-in model. The server falls back to it, so "clears the role" was wrong for a router that has one. - `useCanDecide` is `undefined` while the routers load. Smart routing showed its "paused" notice, and the Decide inspector its warning, for that moment. - The Decide inspector's warning does not show for a block that names its own model: the server runs that block on any connected router. Its model field named one router's decision model as the default. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…per forge `builtins/review.ts` builds the review graph every forge shares. A `ReviewVariant` fills what a forge decides: the events that start and close a pass, the facts step, the checkout command, the worker's capabilities and network, the connector action ids, the workstream and concurrency keys, and the default mention. A forge's spec carries its variant, and the seed registers one built-in for each. Each variant is its own row with its own key, switch and inputs. GitHub's variant is `providers/github/review-variant.ts`. Its definition is byte-identical to the one before, so its content hash is the same and the seed does not insert a version. Readers of the built-in ask for it by forge: - The enrolment lookup takes the provider. The retry RPC passes the review row's provider. - A dispatch finds the built-in of the coordinate's provider. - A retry uses the automation of the original run. It looked the built-in up by one fixed key. `REVIEW_NETWORK` moved from the review control plane to GitHub's variant; the builder was its only reader. `DEFAULT_CONNECTION_PLACEHOLDER` moved to the built-in registry, which the chat built-in also reads. The variant type and its templates are in a module with no product imports, because a provider module reads them while the forge registry loads. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
… per chat provider `builtins/chat-threads.ts` builds the thread graph every chat provider shares: admission, the identity gate, smart routing, the turn gate, the agent, and the failure post. A `ThreadsVariant` fills what a provider decides: the mention events, the facts step, the steps that read the thread and fold it into a prompt, the message action ids, the thread's key and label, and the system prompt. A chat provider's spec carries its variant, and the seed registers one built-in for each. Slack's variant is `providers/slack/threads-variant.ts`. Its definition is byte-identical to the one before, so its content hash is the same and the seed does not insert a version. Instance precedence stands down the chat-threads built-in of the event's provider. It named one fixed key. The variant type is in a module with no product imports, because a provider module reads it while the chat registry loads. The review pass shortened comments that told history, and joined two doc comments on one constant. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
… review fixes A review of the chat seam found these. - `closeRelay` removed the relay when the closing post failed. For the `relay_close` and `close_agent` blocks that lost the message: a retry found no relay and reported success. Now a failed post throws and keeps the relay. The session's end still removes the relay after a failed post, because nothing closes that relay again. - The chat registry read a provider's spec while it loaded. A test that loaded Slack's provider module first failed on an uninitialized spec. The registry builds its table on first use. - The interactivity path was stated in the auth bridge, in the seam's doc, and in Slack's route. The connector's ingress now declares it, the server mounts the provider's app at that path, and the app handles `POST /`. The auth bridge reads the connector, so it no longer loads the chat providers. The server refuses to start when the connector and the provider disagree. - A re-point from another chat provider kept the row's first provider and rendered the new place with it. It is now an error. - The blocks read the relay's renderer from the installed deps, as the session listener does. There were two readers of that wiring. - The choice block builds its place with the relay's mapping. Tests: the three close cases, the re-point refusal, and Slack's provider (email lookup, thread handle, the declared interactivity path). Comments that named Slack in neutral modules, or told history, are fixed. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A review found these. - The three role fields shared one "saving" flag, and a field returned early while it was set. A value typed in a second field during the first field's save was not sent, and the field still showed it. Each field now tracks the value of its own save in flight: it does not send that value twice, and a different value or a different field saves on its own. - A save that succeeds clears the typed value only when the field still shows the saved value. - Escape drops the typed value, so a failed edit can be left. - The Decide inspector hid its warning for a block with its own model. Such a block still needs a router with a key; the warning now shows when no router has one. - `useCanDecide` returned `undefined` when the routers query failed, and the routing section stayed hidden. A failed query is now `false`. - The Slack threads page showed "every mention is ignored" while the routers loaded. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The post does not fail, but the review row then records no id and the findings keep their state. The warning makes that case visible. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A review of the two built-in families found these. Both definition hashes are unchanged. - A retry read the automation of the original run, and its gate checked the enrolment of the forge's built-in. The two could be different automations. The retry now runs the built-in of the review's forge, the one the gate checked. - The dispatch edge built GitHub's payload shape for every forge. The payload is now the variant's `dispatchPayload`. - The builders set the ids of the variant's steps (`facts`, `replies`, `turn`); a variant cannot name them wrongly. - `ThreadsVariant` now states `smart` in the facts and `has_text` in the turn; the builder read both. - `closedEvents` is a list: a forge can close a pull request with more than one event. - The builder adds the review-tools capability; a variant lists only what its forge needs. - The head SHA template had two definitions. - For a provider with no chat-threads built-in, dispatch skips the read of the handle owners; nothing consumed it. - The forge registry builds its table on first use, as the chat registry does. Comments in the dispatch path that told incident history are shortened. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A tool definition named a presenter for each surface (`slack`, `web`). No code read the names: a chat provider's policy and the web client each decide how to show a tool. The field, the exemption field, and the test that required one of them are removed. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The repository picker read the default connection of one named forge; it reads the legacy default forge's. Four comments named a provider as an example. The guard's list drops the five files. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
`parseReviewCommand` and `normalizeMentionHandle` read a comment body and a handle; nothing in them is GitHub's. They move from GitHub's webhook module to `forges/review-command.ts`, with their tests, so a second forge reads the same commands. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
GitLab is the second forge. It gets the code-review built-in as its own variant, `gitlab_review`, from the same builder as the GitHub one. - `providers/gitlab/forge.ts` reads and comments on merge requests through the coordinator's IntegrationOp. A project is its path, encoded as one URL segment. - `providers/gitlab/events.ts` and the `gitlab_webhook` ingress scheme take GitLab hooks on the common event route. The stored secret selects the check: a signing token (`whsec_`) needs a Standard Webhooks signature, and any other secret is compared with `X-Gitlab-Token`. A delivery cannot select the weaker check. - The variant's facts step is the new typed block `review_facts`. The forge says what an event is (`reviewEvent`), and `forges/review-facts.ts` holds the admission policy once for every forge that uses the block. - GitLab's hook has no access level for the author of a comment, so a review command asks the members API and needs Developer access. - `<provider>.upsert_pr_comment` and `<provider>.update_pr_comment` are built-in actions of a forge that has comment methods. GitHub keeps its HTTP actions, so its definition hash does not change. - A review posts its inline comments as draft notes and publishes each one. When GitLab refuses a position, the drafts are deleted and the summary quotes every finding. - The GitLab connector records a created merge request as a `pull_request` asset, the kind the PR-link consumer, the chat recap, and the web card read. It was `merge_request`, which nothing read. The review pass turned up: - The bulk publish call also publishes every other draft of the token's user on the merge request. Each draft is published by its id. - A retried post left the first attempt's drafts in place and made them again. The post deletes drafts that carry its marker before it starts. - The comment actions were registered for GitHub, which has no comment methods. They are registered only for a forge that has them. - Two request helpers repeated each other, and four errors built the same "2xx with an unusable body" error by hand. Not verified against a live GitLab: the status GitLab returns for a position outside the diff, the `changes.draft` shape of an update hook, and `GITLAB_HOST` in a review worker session. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…catalog The provider catalog entry gains four fields: whether the connector declares events, whether it declares actions, and the key of its variant of the code-review and chat-threads built-ins. - The trigger and action inspectors had the list `github, slack, linear` each. Both read the catalog, so a connector with events or actions is a choice without a web change. A new integration trigger starts with no provider. - The review repositories page read the built-in `pr_review`. It reads the built-in of the selected forge, and shows a forge picker when more than one forge has a review. Connected forges come first. A repository path can have more than two names, as a GitLab project in a subgroup has. The review pass turned up a `!` on the automation id in the repositories table; the table renders only when the id is present. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…iew fixes A review of the GitLab forge commit found these. - A failure between two publishes left the review incomplete for good: the published comments carry the marker, so the next attempt saw a posted review and posted no summary. Only the summary note now says that a review is complete, and an attempt publishes the comments that are not on the merge request yet. - The post anchored its comments on the merge request's current diff. A push after the review moved the lines. The post compares the head with the reviewed commit and posts the full summary when they differ. - GitLab wants both line numbers for a line that both sides have, and the old path of a renamed file. The post reads the diff for them. One refused comment made the review summary-only, so a finding on an unchanged line cost every inline comment. - A draft that failed for a reason other than its position left the earlier drafts in place. They are deleted before the error returns. - A comment by the connection's own user could start a review: a review quotes its findings, and a finding can quote a command. The author of a command is compared with the token's user. - A list longer than the page cap returned a part of itself. It is an error now, and the status-comment lookup reads every page. - `GitForge` took the four members of the typed review one by one, so a forge could have a part of them. They are one optional member. - The GitLab test file did not register the engine blocks and failed when it ran alone. The header of the review blocks states what the blocks are, and no longer tells their history. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…changes A review of seven earlier commits found these. - `close_agent` stopped when the closing message could not be posted, so an archived channel kept the session and the workstream open on every attempt. The failure is logged; the session ends and its end removes the relay. - A retry of a review that another automation wrote ran the forge's built-in with that automation's trigger, and the run ended filtered with a success answer. The retry is refused with a message. - The interactivity path was written in the route mount, in the public-path rule, and in a comment. `interactivityPath` is the one definition. The startup check now covers every connector: a declaration with no handler would make a path public with nothing behind it. The public-path rule has a test. - The role model fields share one mutation, and TanStack Query runs the callbacks of the last `mutate` call only. A save that settled while another field saved kept its draft and could not be sent again. Each field awaits its own save and shows its own error. - `useCanDecide` and `useHasRouterKey` answered false when a background refresh failed, though the last answer was still there. - The key picker of a map input listed the default forge's repositories for every automation. It reads the connection of the automation's trigger. - A test built a Slack client with `as unknown as`. It uses a real client with one method replaced. - The warning for a review with no readable id names the repository and the pull request. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…s them The Reviews page built GitHub URLs for every review: the pull request, the posted review, a finding's file, and a finding's thread. A GitLab review got links to github.com and to anchors GitLab does not have. - `GitForge.links` builds the four links. GitHub's are the ones the web built. GitLab's start from the merge request's URL, because the host is the connection's. - The review API sends `posted_review_url` on a pass, and `file_url` and `thread_url` on a finding. `pr_url` falls back to the forge's URL from the coordinates when the record has none. - The web reads the links and no longer builds them. The four builders in `review-format.ts` are removed. A pass with no link shows no link. During a roll, a new web against a previous orchestrator shows no file link and no thread link, and links to the pull request only when the record has its URL. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The review page looked up the built-in `pr_review` for every pass, so the "Run" link of a GitLab review opened the GitHub review automation with a run id that automation does not have. - The page reads the built-in key of the pass's provider from the catalog. - `useBuiltinAutomation` sends no request for an empty key. - `PR_REVIEW_BUILTIN_KEY` had no reader; it is removed. The review pass also shortened two comments in the page header. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
- DESIGN.md gains the facts of the three roles (git forge, chat, model router), each with the test that guards it. - The site gains "Add a provider" under Contributing. It says what a connector alone gives, what each role implements, and what is not there yet (Azure DevOps review, Teams, two hosts of one provider). - The model-routers guide covers LiteLLM and the role models. The reviews page covers the GitLab review and how it differs from the GitHub one. The automations page no longer says a Decide block needs OpenRouter. - CHANGELOG: three upgrade notes (Helm selectors, hosts before the control plane for GitLab git, rollback), and the added and changed entries. - The provider-name guard states why each of its four allowed files names a provider. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
… notes A review of the GitLab forge found that the draft-note design rested on a wrong fact. GitLab accepts a draft at any position and checks the position only at publish. A project or group token then loses the comment with no error, and a personal token gets a 500 on every attempt, so the summary never posts. The post no longer uses drafts: - It reads the diff version whose head is the reviewed commit. The refs and the lines come from that one version, so a push between two reads cannot mix them, and a review of an earlier head keeps its inline comments. - It checks every finding against the lines the diff shows before it posts. With one finding the diff does not show (a collapsed file, a line outside a hunk), no inline comment posts and the summary quotes every finding. - Each inline comment is a discussion, which GitLab checks at create. A 400 stops the inline comments and the summary quotes every finding. - A retry matches a posted comment by its position and counts per position. A body match failed when GitLab stored another body, and lost one of two findings with the same text. - A file is found by its old path too. Also from the review: - A line that starts with "/" is escaped in every body the forge posts, outside a code block. GitLab runs such a line as a quick action (`/approve`, `/merge`) with the token's role, and the text comes from a model that read the merge request. - A note is the forge's own only when the token's user wrote it. Any commenter can post a marker; a status comment found that way made the update fail with a 403 on each run. Not changed: `findCommentWithMarker` returns null when the list cannot be read, as GitHub's does, so an outage can post a second status comment. A comment from the token's own user starts no review; the site says to use a bot or a service account. Not verified against a live GitLab: the shape of a diff version, and the status of a refused position. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…view fixes A review found that the run link still guessed. The web took the built-in of the pass's forge, but a duplicate of the built-in, or a user automation with the review blocks, also writes a pass. Its link opened an automation that does not have the run. - GetReview sends `automation_id`, read from the run's row. The web links to it and no longer looks up a built-in. A pass whose run is gone has no link. - A stored empty `pr_url` counts as absent. It gave links that were only a fragment. - `useReviewForges` is pending until the connectors load too: its order reads them, and a late reorder remounted the repositories page. - A failed save of a role model shows its error only while that value is still the one sent. - The interactivity check in `index.ts` reads `declaredIngress`, as the public-path rule does. - `ReviewDeps.retryAutomation` states its provider argument. - Comments: the dossier header and the suggested-fix note name no forge; a `const` between imports moves below them. Not changed, for the PR description: during a roll, a new web against the previous orchestrator shows no forge on the review repositories page and no provider in the builder's pickers. In the builder, a user-built GitLab review has no provider field on `review_open_pass`, a GitLab trigger gets no per-merge-request scope preset, and "Mine" on Reviews reads the GitHub login only. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The rebase onto the 0.13.0 release left them in the 0.13.0 section. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow CI / buf (pull_request).
|
nikhilunni
marked this pull request as draft
November 11, 2026 20:39
`rejects.toSatisfy` gives the predicate the promise, not the error, so the check for a unique violation failed on a correct refusal. The test needs a live Postgres, which only CI had; the full suite now passes against one locally. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This branch has not been deployed
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
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.
Summary
Git forges, chat services and model routers are roles. GitHub, Slack and OpenRouter become
implementations, and a second provider fills two of the roles to prove the seams:
ModelRouterSpec. OpenRouter moves behind it; LiteLLM is new.GitForge. GitHub moves behind it; GitLab is new, with git through theegress proxy and a merge-request review. An Azure DevOps connector gives git over
HTTPS, pull requests, work items and pipelines, with no review yet.
ChatProvider. Slack moves behind it. No second provider; the seam has oneimplementation.
provider fills the provider-shaped slots. Each variant is its own built-in row.
DESIGN.mdhas the facts of the roles. The site has "Add a provider" for contributors.What this retires
secret caches of each provider.
two inline secret listings.
sends them.
src/providers/names a provider. Four files areallowed, each with its reason.
Deploy and rollback
test pins both. No active run stops.
blob.backend,kek.provider,secrets.backend(engram) andblob.backend,egress.caSource(engram-host-fleet)before it takes these charts. The deploy repository needs that change first.
serves the git paths.
pr_refgainsproviderand a(provider, repo, pr_number)uniqueindex. The old
(repo, pr_number)index stays for one release, because a replica of theprevious build upserts against it. Until a follow-up drops it, two forges cannot both
link the same repository name and number; the consumer logs that case and continues.
previous build. Nothing else changes back.
review repositories page, no provider in the builder's pickers, and fewer links on a
review. Each recovers when the orchestrator finishes its roll.
One decision for the owner
A session now gets git credentials only for a forge that its profile grants. Before,
the coordinator checked the broker token and not the capability. A session that cloned
from GitHub with no GitHub capability on its profile is refused after this change. This
needs the count of such sessions from production before the merge.
Not verified
public API references. For GitLab that includes the shape of a diff version, the status
of a refused position,
GITLAB_HOSTin a worker session, and whether the capabilityopens the host under a deny network.
to end on this branch, and a session created on
mainthat continues on this build.Known limits
Developer access, and a comment from the token's own user starts nothing. There is no
stopcommand.outage can post a second status comment.
fail. GitLab now checks the author.
user-built automation has no provider field on
review_open_passand noper-merge-request scope preset; a GitLab handle shows as plain text. The chat-threads
page is still Slack's page.
SourceMention, the choice limits, the channelnoun). The second chat provider will show where.
slack_brain,pr_review,slack_choice,github_review_id).What the review passes turned up
Opus subagents reviewed each group of commits; the fixes are in the commits named
"review fixes". The larger ones:
and query now match apart.
pr_refindex would have left sessions leased on a previous-buildreplica; the index stays for one release.
positions first and uses discussions.
/approve,/merge); such lines areescaped.
author is compared with the token's user.
server sends both.
Gates
just check, orchestratorbun run typecheckandbun test, webformat:check,lint,testandbuild, sitepnpm build,helm templateof both charts with the CI valuesand with a selector unset.
🤖 Generated with Claude Code