feat(resources): a first-class Resource entity with lifecycle, revisions and parameters - #1410
Closed
Cedric Vidal (cedricvidal) wants to merge 49 commits into
Closed
Cedric Vidal (cedricvidal) wants to merge 49 commits into
Cedric Vidal (cedricvidal) wants to merge 49 commits into
Conversation
Coding agents are routinely asked to work with issues, branches and pull
requests, but `gh` was not available in any worker image, so a task that
needed it could only fail or fall back to hand-rolled API calls.
Install it alongside the other system tools each image already ships
(Python, Go, .NET, Java, Maven, Gradle, PowerShell), pinned to 2.100.0 for
reproducible builds:
- coder-acp-copilot and coder-acp-claude-code: fold the release tarball into
the existing toolchain layer, reusing its `ARCH` so amd64 and arm64 both
resolve (gh's asset names match `dpkg --print-architecture` exactly). The
layer ends in `gh --version` so a bad URL fails the build instead of
silently producing an image without it.
- coder-acp-copilot-windows: install through Chocolatey next to git. No
version check here, because the Chocolatey shim directory only joins PATH
via the later ENV, so `gh` is not yet invokable at that layer.
`GH_VERSION` is exported at runtime so the agent version registration can
report it next to COPILOT_CLI_VERSION.
Verified by building the coder-acp-copilot `base` stage and running the
resulting image:
gh version 2.100.0 (2026-09-03)
GH_VERSION=2.100.0
/usr/local/bin/gh
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The coder services mount the host Docker socket and set DOCKER_HOST so the
agent can run containers during a task. `group_add: ${DOCKER_GID:-0}` gives
the right GID, but that is only half of what the socket needs.
Where the daemon host enforces SELinux — a podman machine always does, and
RHEL/Fedora Docker Engine can — the worker runs as `container_t` while the
socket is labelled `var_run_t`, and policy denies the connect. The agent then
cannot run containers at all, and the failure is easy to misread: it surfaces
as `permission denied ... /var/run/docker.sock`, which looks like a GID
problem that DOCKER_GID has already solved. Even `stat` on the socket is
denied, which is the tell that it is the label and not the mode.
Measured on a podman machine with SELinux enforcing, mounting the socket into
the copilot worker image:
security_opt group_add 0 result
(none) no denied
label=disable no denied
label=type:container_runtime_t no denied
(none) yes denied
label=disable yes OK
label=type:container_runtime_t yes OK
So both are required. Add `security_opt: label=disable` next to the existing
group_add. Docker ignores label options on hosts without SELinux, so this is
a no-op under Docker Desktop and leaves those setups unchanged.
`label=type:container_runtime_t` works equally well and keeps the container
confined, which is preferable in principle, but it depends on container-selinux
providing that type and fails closed when it does not. For a local development
stack the portable option is the better default; the alternative is noted in a
comment.
Verified by recreating coder-acp-copilot from the compose files alone: the
container reports SecurityOpt ["label=disable"], GroupAdd ["0"], and `docker
ps` from inside the agent's user now succeeds.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
A Resource declares something that must be made available for a run, together with the lifecycle that provisions and releases it. It may be backed by a container started through the Docker socket, or by something external such as a cloud database — only the script bodies differ, everything downstream is the same. This exists because a run currently has no way to stand up its own environment. MCP servers are registered during worker setup, before the agent executes anything, and registration opens a live connection — so a backend the agent would start itself kills the run before its first turn. A resource's setup phase runs earlier and publishes the connection details that registration then uses. Modelled closely on the codebase entity, which is the other revisioned catalog type: - `revisionCounter` with `latestRevisionId`/`latestRevisionNumber` so concurrent revision creates get distinct, gap-free numbers and the pointer is guarded against stale writes. - The revision carries `createdAt` and a cascade-only `deletedAt`, and deliberately no `updatedAt`: revisions are created and read, never edited. Editing a resource creates a new revision, so a run pinned to one stays reproducible. The response schema omits `deletedAt` entirely. - Script bodies are keyed by interpreter rather than assumed to be `sh`, so adding `powershell` for the Windows worker later is additive instead of breaking. `exports` declares the names the setup phase promises to publish. Declaring them lets the API reject an MCP server referencing an unprovided variable, and lets the worker fail with the missing name instead of registering a server with an unsubstituted placeholder. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
… output
Three pieces the worker needs to stand a resource up and wire it into a run.
**resource-env** parses the `$SCOPE_SETUP_ENV` file a setup phase appends its
connection details to. A file rather than stdout, so that ordinary script
logging — docker progress, curl retries — cannot corrupt the contract. It splits
on the first `=` only, because connection strings and URLs routinely contain
more; tolerates CRLF; and *reports* malformed lines instead of skipping them,
since a dropped line resurfaces much later as an unresolved `${VAR}` far from
its cause. It also detects a name published by two resources, which would
otherwise make the environment depend on reference order invisibly.
**resource-interpolate** substitutes `${VAR}` into MCP server config so a stored
record can stay static and reusable (`url: ${MCP_URL}`). It covers every field
that can carry a connection detail, which differs by transport: `command`,
`args` and `env` for stdio, `url` and header values for http/sse. An unresolved
placeholder throws and names every offender at once — passing `${MCP_URL}`
through literally fails much later inside the gateway as an opaque transport
error.
Note the ordering this implies, documented on the module: interpolation must run
*after* Token Manager secret hydration, because hydration replaces the whole
`env`/`headers` object rather than merging, and would silently undo it.
**resource-runner** executes a phase with `sh -e`, a bounded timeout, and stdout
and stderr streamed into the run log. `-e` matters: without it a failing command
continues into a half-provisioned state that still reports success. Setup runs in
reference order and fails the run if a phase does not publish what its revision
declared. Teardown runs in reverse so dependants unwind before dependencies, and
is best-effort — letting cleanup failure change the run's outcome would mask the
result the run actually produced.
32 tests, covering real subprocess execution, the timeout path, export
validation, malformed env rejection, and reverse-order teardown.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Wires the resource lifecycle into the Copilot worker. Three orderings matter
here, and each of them is a bug if it moves.
**Resource setup runs before MCP registration.** This is the point of the
feature. Registration opens a live connection to the MCP server and throws if it
is unreachable, so a server backed by something the run brings up itself could
never be registered — setup() failed before the agent's first turn. Provisioning
first means the address exists by the time registration needs it.
**Interpolation happens immediately before registration**, not earlier. The queue
processor hydrates MCP configs with plaintext secrets from Token Manager, and
that hydration *replaces* the whole env/headers object rather than merging into
it. Substituting `${VAR}` any earlier is silently undone, which presents as
"interpolation doesn't work" with nothing in the logs to explain it.
**Resource teardown runs before the container purge.** The purge in teardown()
would otherwise destroy the very containers a teardown script is about to remove,
leaving it to fail or silently no-op. The orphan purge in setup() stays where it
is, before resource setup, so a resource publishing a fixed port cannot inherit a
stale container still holding it.
Published connection details are also merged into the agent's subprocess
environment. buildSubprocessEnv constructs a fixed object and never spreads
process.env, so this is the only way an agent-facing tool can learn where the
run's resources are. The resource values are spread FIRST so the fixed keys win:
a resource must not be able to shadow GITHUB_TOKEN, which is the CLI's own auth,
nor the proxy settings that route model traffic for capture.
Adds a Docker socket preflight check. Without it a container-backed resource
fails inside its own script with a raw `permission denied ... docker.sock`, which
reads like a group-ownership problem even when it is an SELinux label denial —
an expensive thing to misdiagnose, so the error names that possibility.
Failure during setup unwinds whatever already came up, in reverse, rather than
leaving a half-provisioned environment to leak into the next run.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Adds the worker-side client that turns a resource spec into the `ResourceConfig` a worker needs, and threads resolved resources through the queue processor into `processor.setup()`. Resolution happens in the queue processor rather than inside the worker so that a missing or deleted resource fails the run before any provisioning work starts, instead of halfway through a setup phase that has already created containers. A spec may be a slug, `slug@rN`, or a revision id. The request stores `resourceRevisionIds` — the *resolved* ids — rather than the specs, so a run stays explainable after the resource is edited and its latest revision moves on. This mirrors how `codebaseRevisionId` is pinned from a codebase spec. Order is preserved through resolution because it is meaningful: setup runs in reference order and teardown in reverse, so that dependants unwind before the things they depend on. 960 shared tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Introduce the resource store, resolver, API routes, CLI commands, and indexes so lifecycle resources get the same mutable identity plus immutable revision history as codebases. Enforce scoped duplicate checks in the API because Cosmos can downgrade unique indexes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
…time Adds `resources` to run submission, accepting a slug, `slug@rN`, or a revision id, and resolving each to a concrete revision id stored on the request. Pinning at submit time is the point. A run that recorded only a slug would silently change meaning as soon as the resource gained a new revision, and any comparison across past runs would quietly stop being valid. Storing the resolved revision keeps a finished run explainable no matter what happens to the resource afterwards — the same reason `codebaseRevisionId` is pinned from a codebase spec. Resolution happens once, before the variation loop, so every profile in a grouped submission provisions an identical environment. That matters for the comparison the platform exists to make: if two profiles could resolve different revisions, the environment would become an uncontrolled variable. An unknown resource fails the submission with a 400 rather than being dropped. A run that silently started without the environment it asked for would look valid and produce meaningless results. Order is preserved through resolution because it is meaningful: setup runs in reference order and teardown in reverse. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The entity was documented; the runtime half was not, and that is the part most likely to be broken by a later change because each ordering looks arbitrary until it is violated. Three orderings are load-bearing, and each is written down with the bug it prevents: resource setup before MCP registration (registration opens a live connection and throws, so an agent-provisioned backend could never be registered); interpolation after secret hydration (hydration replaces the whole env/headers object rather than merging, so substituting earlier is silently undone); teardown before the container purge (the purge would destroy the containers teardown is about to remove). Also documents a trap found by running it: published values land in the agent's own environment, so a name the agent's tooling already reads will change that tooling's behaviour. Publishing `GH_TOKEN` for a GitHub simulator broke the Copilot CLI, which prefers `GH_TOKEN` over `GITHUB_TOKEN` for its own auth and consequently tried to authenticate against real GitHub with a simulator token. The guidance is to publish neutral names and let the task prompt set tool variables inline, scoped to the command that needs them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Persists, per run, which resources were provisioned and whether MCP servers were actually registered. The failure this guards against is specific and dangerous: a run that completes, reports success, and silently had no MCP tools or no environment. Nothing about such a run looks wrong afterwards, so any comparison involving it is quietly meaningless. The live log shows what happened, but a log is not queryable and is gone from view by the time anyone compares results. Each resource records its pinned ref, whether setup succeeded, which names it published, and whether teardown ran. A failed setup still records the resources it was attempting, so a failed run shows what environment it was trying to stand up rather than nothing at all. Observations are captured after teardown so `teardownRan` is accurate, and recording them is best-effort: failing to write observability must not change the outcome the run exists to report. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
A worker killed between setup and teardown leaks whatever its resources created, and the obvious fix — purge containers at startup — is actively dangerous here. `KubedockClient.isEnabled()` already refuses to run under Docker Compose, and the reason is easy to miss: Compose mounts the *host* Docker socket, so a blanket purge would delete the entire development stack rather than just the run's leftovers. That constraint is worth stating where resource authors will read it, because the natural instinct is to add exactly that sweep. The workable answer is scoped and idempotent: remove your own containers by explicit name at the start of setup. That is safe in both environments and fixes the practical symptom, which is a fixed published port still held by an orphan from an earlier run. Also notes that externally provisioned resources have no safety net at all — a leaked cloud database costs money and no purge reclaims it — so those should carry a server-side expiry or a tag a reaper can find. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
"Resource" sits next to Kubernetes resources, Azure resources and container `resources:` limits in this repo, so the choice deserves an explicit rationale rather than being left to look accidental: the entity describes what a run needs to exist, independent of how it is produced, which is precisely why a container and an external cloud database are the same kind of thing here. Also records a collision found while reviewing the name: the portal already has a nav group called "Resources", for "project-scoped integrations you wire up (MCP, extensions)". The entity belongs in that group semantically, so the disambiguation belongs in the nav label rather than in a rename. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Add portal-side Resource and ResourceRevision shapes plus API client methods so later UI can call the project-scoped resource endpoints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Create the Resource list with the shared list-layout table, search, column customization, deletion, latest revision, and export count columns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Show the mutable resource identity alongside its immutable revision history, with selected-revision scripts and exports so older revisions remain inspectable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Wire the resource list into a contextual preview panel matching codebases so users can inspect identity, latest revision, and exports without leaving the list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Add a reusable create form and page for defining resource identity plus the first setup, teardown, and export-name revision. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Let run authors attach lifecycle resources alongside codebases and submit them as resource specs for server-side revision pinning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Expose the Resources pages in routing and place them in the existing Resources nav group as Lifecycle to avoid a duplicated Resources > Resources label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Surface pinned resource revisions and lifecycle outcomes on run details so resource-backed runs explain what was provisioned and published. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Provide an explicit empty rail body so the Resources list compiles with the shared list-layout API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Move resource outcome typing and rendering to run-state data while preserving the top-level pinned revision fallback, and surface mcpRegistered as an attempt signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The Resource entity landed inside a nav group already called "Resources", so the sidebar read "Resources > Lifecycle" — the item was renamed to avoid a duplicate label. That fixed the collision in the wrong place: "MCP" and "Extensions" name things, while "Lifecycle" names an aspect, so nobody hunting for the Resource catalog would look for it. The group's own doc comment already described it as "project-scoped integrations you wire up", so the group was the misnamed one. Rename the group to Integrations and give the item its real name, Resources. The group id is left alone so persisted sidebar collapse state survives. Layout's hidden-nav test asserted on the text "Resources", which after the rename would have silently matched the item instead of the group. Make both assertions explicit and add the positive case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Two things were implemented but undocumented, and both are easy to get wrong from the outside. The portal pages needed recording because the detail page is what makes the immutability claim checkable: selecting an older revision swaps the scripts, exports and content hash to what that revision declared, so a reviewer can verify by clicking rather than by querying Mongo. Also note why the nav group was renamed instead of the item. The observability fields needed recording because they are nested under `run`, while `resourceRevisionIds` sits at the document root — a discrepancy that already cost me a wrong conclusion when I looked in the wrong place. Spell out that consumers must tolerate their absence on older runs, and that `mcpRegistered: false` is both a legitimate value for the gh surface and the quickest way to spot a run that was submitted without `--mcp-servers` and is therefore silently meaningless. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
A resource whose seed is hardcoded is a fixture, not a capability: the GitHub simulator's lifecycle is identical for every repository, so today a second repository means a second resource. Parameters split "how to provision" (the revision, immutable) from "what to provision it with" (the binding, per run). Declarations live on the revision and are folded into contentSha256, so editing the contract mints a new revision — a setup body and the parameters it reads have to move together or the pairing rots. They are normalized by name first so that a client serializing keys in a different order does not mint a pointless revision, and omitted entirely when empty so parameterless saves keep hashing as before. Replace `resourceRevisionIds: string[]` with a single grouped `resources: ResourceBinding[]`. Two parallel arrays would have to stay the same length and order forever, an invariant nothing enforces and any writer can break — three ids and two parameter maps has no correct reading. Params are stored fully resolved rather than as a diff, so a run is explainable from its own document without re-reading a revision whose defaults may since have moved. This is the same unmerged PR, so the old field is amended in place rather than migrated. resolveResourceParams encodes the precedence the submit path already enforces for worker/model/mcpServers/skills/extensions: the profile wins. A run may fill what the profile left open, or restate a profile value identically, but a differing value is a conflict reported per parameter rather than per binding — otherwise a run would have to restate every pinned value just to fill one open slot. Unknown keys are errors, not ignored: a silently dropped REPOS= typo would seed the default and produce a healthy-looking run answering a different question. The queue processor re-attaches params to configs by revisionId rather than by array position, so a resolver that reorders or dedupes cannot pair a resource with another's parameters. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Setup and teardown now receive the run's resolved parameter values as environment variables, which is what lets one revision serve many scenarios instead of hardcoding its seed. Parameters are merged per resource rather than into the shared phase options, so one resource's values cannot leak into the next one's script -- covered by a test asserting the second resource sees the variable unset. Caller-supplied env deliberately wins over parameters. That env carries worker infrastructure such as DOCKER_HOST, and a resource declaring a parameter sharing that name would otherwise redirect the Docker socket rather than configure itself. SCOPE_* is already refused at declaration time for the same reason; this closes the remaining case, with a test pinning the behaviour. Teardown gets the parameters too. It usually needs them to identify what to remove, and a teardown that cannot name its own containers leaks them. The worker echoes resolved params onto the run outcome, including on the setup-failure path, so a run that failed while standing up its environment still shows what it was trying to stand up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Parameters are the half of the interface that was missing: exports are what setup publishes on the way out, parameters are what a run supplies on the way in, and they are what makes a resource reusable rather than a fixture. Record why the export/parameter name collision is rejected (a name that is both input and output makes the published value depend on phase ordering), and why unknown keys are a 400 rather than ignored — a silently dropped REPOS= typo seeds the default and yields a healthy looking run that answers a different question. Spell out the precedence as an extension of the contract the submit path already enforces for worker/model/mcpServers/skills/extensions rather than as a new rule, including why conflicts are reported per parameter instead of per binding. Correct the observability section: there is no parallel resourceRevisionIds array any more, and the reason is worth keeping -- two lists that must stay the same length and order forever is an invariant nothing enforces. Also state plainly that parameters are not a credential channel, and point at the simulator's own pattern of deriving SIM_TOKEN inside setup and publishing it as an export. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Profile versions need to carry resource binding specs so submit can merge their preset parameters with run-supplied values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Expose parameter contracts in portal types, resource creation, and immutable revision detail so inputs and exports can be reviewed together. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Run submission now pins resource refs into grouped bindings and uses the shared merge contract so profile presets can safely combine with run-supplied values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The CLI now lets users declare revision parameters, preset profile resources, and pass run parameter values while rendering API conflicts clearly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Carry grouped resource bindings through profiles, run submission, and run detail so profile-owned values stay locked while runs fill only open inputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The published API spec needs to include parameterized resource bindings so generated documentation matches the route and shared schema contracts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
…terized too Parameterizing the obvious inputs was not enough to make the simulator reusable. Its setup asserted that issue #11 existed before reporting ready -- a deliberate fail-fast, since a simulator serving an empty seed looks healthy but has nothing to work on. That assertion was exactly what broke the first run against a different repository: setup failed for a reason that had nothing to do with the repository being wrong, and the failure surfaced after the import rather than at submit. Keep the check but make it a parameter. Record the general rule, because the trap generalizes: when parameterizing a resource, the constants worth hunting are the ones describing the scenario rather than the resource, and they tend to be good checks rather than obvious hardcoding. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The profile create and new-version forms could set resource bindings and
parameter presets, but the detail page rendered worker, model, MCP,
skills and extensions and silently omitted resources -- so the one place
a reader goes to answer "what does this profile fix?" could not answer it.
Preset parameter values matter most here, because they are precisely what
a run is not allowed to override. Showing them next to the pinned
revision makes the profile's controlled conditions legible without
reading the API response.
Also improve the resource-create 409: a soft-deleted resource keeps its
slug so existing {slug}@rn refs stay unambiguous, but the message said
only "already exists", sending the caller hunting for a resource that
appears in no listing. Say that a deleted resource holds the slug, and
why.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
I added a Resources section to ProfileDetail believing the page did not render pinned resources. It already did, further down the same file -- my read stopped too early, and the browser check that seemed to confirm the gap was made against a container serving stale source. The result was two Resources sections on one page, visible in a recorded walkthrough frame. Remove mine and keep the original, which is the better of the two: it renders each binding as a card and states "No parameter presets." explicitly rather than showing an empty row. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
This reverts commit 111a633.
A profile-variation submit creates one request per profile and returns
{ ids, variations, submissionId, ... } -- there is no `id` and no
`workerType`. The output path assumed the single-request shape, so it
rendered `undefined` for both and then threw inside chalk with "The
text argument must be of type string".
The submission itself had already succeeded at that point, so every
request existed while the CLI reported a hard failure -- the worst way
to be wrong, since the obvious response is to submit again and double
the runs.
Detect the submission shape and print what it actually contains: how
many requests across how many profiles, the submission id, and the
request ids per variation. Follow-up commands point at the first
request, since streaming follows one request and a submission has
several.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The project row is a flex row, and its text column had no min-w-0. A flex item defaults to min-width:auto, so the column refused to shrink below its content and a one-sentence description rendered ~1070px wide inside a max-w-lg card, spilling out of the card entirely rather than wrapping. Add min-w-0 so the column can shrink, clamp the description to two lines, and truncate the name. Clamping rather than truncating the description keeps a normal sentence readable, which is the common case, while still bounding a pathological one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
… run The processor instance is reused across queue messages. teardown() cleared mcpConfigs but left mcpRegistered set, so once any run registered an MCP server, every subsequent run on that worker reported mcpRegistered: true -- including runs configured with no MCP servers at all. Registration itself was correctly skipped for those runs, since the guard tests mcpConfigs.length, so no tools leaked between runs. That makes the bug worse rather than better: the flag exists specifically to record which surface a run was given, and it was confidently wrong. A comparison that uses it to show one arm had MCP and another did not would have been reporting worker process history, not run configuration. It also made results order-dependent -- the same run reported differently depending on whether an MCP run had preceded it on that worker. Reset mcpRegistered, resourceOutcomes and resourceEnv at the start of setup, next to mcpConfigs, so every observation describes the run being set up rather than whatever ran before it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
A resource's exports served two audiences at once: wiring the platform (MCP server interpolation) and configuring the agent under test. One list, one visibility -- so a connection detail the platform needs could not be supplied without also placing it in the agent's environment. That matters whenever a benchmark compares tool surfaces. If the agent can read a service URL and token from its own environment, calling the REST API directly is the shortest path available, and arms converge on it regardless of the tooling each was given -- the comparison then measures nothing. Setup scripts now have two publish channels, differing only in who can see the result: $SCOPE_SETUP_ENV -> the agent's environment, as before $SCOPE_CONCEALED_ENV -> the platform, later resources, and tooling wrappers Visibility follows from which file a script writes to, so nothing has to be declared and there is no schema or migration change. Scripts already contribute to the revision hash, so a script that starts writing to the concealed channel mints a new revision on its own. The concealed store is run-scoped and accumulates, where $SCOPE_SETUP_ENV is a fresh per-phase temp file. That also closes an existing gap: a resource could not previously read what an earlier resource published, which forced tooling wrappers to read connection details out of the agent's environment for lack of anywhere else to get them. Concealed values still reach MCP interpolation -- they are merged into resourceEnv and subtracted again when the agent's environment is built. This conceals values from the environment, not from the filesystem: resource setup and the agent run as the same uid, so a determined agent can still read the store. Removing the path of least resistance is the point. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
A run's profile determines its whole environment -- worker, model, resources, MCP servers -- but reading it meant opening the Details tab. When comparing the arms of a profile-variation batch, the profile is the most important thing about a run, and it was the one field not on screen. Reuses the profile query the Details card already issues, so this adds no request. Links to the profile and shows the pinned version alongside it, because two runs of the same profile at different versions are not comparable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
The preview panel's Configuration card listed worker, model, version and attempt but not the profile -- the field that determines all of them. Scanning a profile-variation batch from the runs list meant opening each run to find out which arm it belonged to. Mirrors the run header: links to the profile and shows the pinned version, since two runs of the same profile at different versions are not comparable. Falls back to the shortened id while the name loads. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
A collapsed group showed only a "N/M done" bar, which moves solely when a run finishes. During a long batch that is indistinguishable from a stalled one -- the group looks idle for minutes at a time while a run is actively working. Adds a pulsing dot to the group's status cell whenever any member run is processing, with the count in its tooltip. Uses the server-side status aggregate, so it is correct for collapsed groups whose members are not loaded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Two suites still described the tree as it was before this branch, so CI failed on changes that are the point of the change. `check-migrations` hard-codes the required migration list, and this branch adds `029-create-resource-indexes.ts`. The fixtures and the three applied-count assertions now include it; the "extra applied migrations" case counts one more because it deliberately applies every required migration plus an unknown future one. The CLI command-tree snapshot predates the `resource` command group, the `--resources`/`--resource-param` options on profile and run, and `--count` on run submit. Regenerated. The diff is purely additive -- no option or subcommand disappeared -- which is what that snapshot exists to prove. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
`selectModel()` gives up on three paths and logs a different warning on each: the agent advertises no model capability, `session/set_model` throws, or `session/set_config_option` throws. The integration test asserted only the first, so the other two failed with "expected a capability warning in logs" — a message describing the test's assumption rather than what happened, which sends the reader hunting for a capability problem that is not there. Accept any of the three, and print the captured logs when none matches, so a genuinely new failure mode is readable from the CI output instead of requiring a local repro. This does not change the worker. It is why `Integration Tests (copilot-acp)` has been red on main since 2026-09-10 with an assertion that names the wrong cause. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
`$SCOPE_SETUP_ENV` was documented and `$SCOPE_CONCEALED_ENV` was not, so the only description of the second channel lived in the code that implements it. Covers what reads a concealed value (MCP interpolation, later resource phases, tooling wrappers that read the file at call time), why the store is run-scoped and accumulates where `$SCOPE_SETUP_ENV` is per-phase, and the motivating case: a tool-surface comparison stops measuring anything once every arm can read the endpoint out of its own environment. States the limit plainly rather than implying a sandbox. Resource setup and the agent share a uid, so this conceals from the environment, not the filesystem. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Five defects, each of which let a run report a result for an environment that was not the one it declared: - Resource-backed runs were routable to workers that ignore resources entirely. Only coder-acp-copilot provisions them, but nothing checked: the run's declared simulator or database was silently never stood up and the benchmark still reported a result. Adds a supportsResources agent capability alongside the existing ones, declared honestly per worker. - Resubmitting dropped all resource bindings. The rerun executed in a different environment while looking comparable, which invalidates the comparison a resubmit exists to make. Pinned bindings are now carried over, or re-resolved when a resubmit selects a different profile. - Binding the same revision twice gave both occurrences the first binding's parameters, because configs were paired to bindings with find() by revisionId. Two simulators intended for different repos both targeted the first. Pairing is now driven by the bindings. - Publishing one key to both the public and concealed channels resolved silently: the concealed value won for MCP interpolation, the public value was discarded, and the key was withheld from the agent. No leak, but three surprising outcomes at once. Now fails the run. - Setup failures discarded the resource observations the worker had just recorded, because persistence lived inside a try/finally that setup runs before. run.resources was absent on exactly the runs that needed it most. Persistence is extracted and now covers both paths. Adds 11 regression tests; the duplicate-binding one fails against the old implementation. Full suite 2769 passed. Docs updated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Three P1s from review, two of which both reviewers found independently. Partially provisioned resources leaked on setup failure. runResourceSetups() reported the attempted prefix only in its return value, which a throw discards — so releaseResources() found an empty list and tore down nothing, including the failing resource whose script may already have created containers. The prefix is now reported through an onProvisioned callback as each setup begins. Failure outcomes also stop marking every resource failed: the prefix distinguishes provisioned, failing, and never-attempted. Teardown did not cover initialization. Resource provisioning happens inside processor.setup(), but setup sat outside the try/finally calling teardown(), so a failure in MCP registration, codebase seeding, skill extraction, or gate-prompt resolution leaked every provisioned resource even when all setups succeeded. The boundary now opens before setup; teardown() is already idempotent. This supersedes the narrow observation-persisting catch, which the finally now covers. Resubmit re-resolved resources when it should have preserved them — a regression from the previous review pass. activeProfileVersion is populated even when the caller keeps the original profile, so an ordinary resubmit re-resolved the profile's specs: simulator@r1 with REPO=run/repo came back as simulator@r2 with the revision's default, 422 when the parameter is required, or with resources dropped entirely when the profile declares none. Keyed off an explicitly supplied replacement profile instead, extracted as planResubmitResources() so the rule is directly testable. Adds 9 regression tests. Full suite 2778 passed. Docs updated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 507f8ebd-cc32-489c-afca-8941c5f8dba1
Cedric Vidal (cedricvidal)
requested review from
Josh Duffney (duffney) and
Wassim Chegham (manekinekko)
September 18, 2026 20:21
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.
Why
MCP servers are registered before the coding agent starts. That ordering makes a whole class
of benchmark scenario impossible to express: anything the agent would have to provision itself —
a seeded API simulator, a database, a service the MCP server points at — cannot exist in time to
be registered against.
There was no place in the model to say "this must exist before the run". Scenarios worked
around it with ambient, hand-managed infrastructure, which is neither reproducible nor
describable in a run record.
Walkthrough
The change spans schemas, API, CLI, storage, worker lifecycle and portal, which is more than a
reviewer can reasonably stand up. Two recorded walkthroughs cover the same arc on both surfaces:
author a resource, earn a revision, build two profiles on it, show both submission paths, then read
the evidence from completed runs. Neither submits a run — the submit step is shown and deliberately
not executed.
CLI Video
walkthrough-cli.mp4
Portal video
walkthrough-portal.mp4
What
A first-class Resource entity: something that must be made available for a run, together with
the lifecycle that provisions and releases it.
at submit time, so a run stays explainable after the resource is edited. Identical content
deduplicates against the latest revision rather than bumping the number.
setup/teardownscripts with declaredexports— the connection details setuppublishes by appending
KEY=VALUElines to$SCOPE_SETUP_ENV. A file rather than stdout, soordinary script logging cannot corrupt the contract.
$SCOPE_CONCEALED_ENVis the same contract with a differentaudience: values written there reach MCP interpolation, later resources, and tooling wrappers,
but never the agent's environment. Without it, an endpoint the platform needs is also an
endpoint the agent can read — and in a tool-surface comparison that hands every arm the same
shortcut, which is how a benchmark ends up measuring nothing. Concealment is from the
environment, not the filesystem: setup and the agent share a uid.
REPO/REFrather than hardcodingthem is what makes one resource serve many scenarios instead of becoming a fixture.
what the profile left open; it may not change what the profile fixed.
${VAR}interpolation into MCP server URLs and headers, so a server can point at somethingthat did not exist when it was configured.
Three orderings that are load-bearing
Each is a bug if moved, so each is stated in the code and in
docs/architecture/resources.md:the endpoint is unreachable — the entire reason this feature exists.
env/headersobjects, sosubstituting earlier is silently undone.
about to remove, turning a clean release into a confusing one.
Design decisions worth reviewing
resources[]on the request, not parallel arrays of ids and parameter maps.Two lists would have to stay the same length and order forever, an invariant nothing enforces;
three ids beside two parameter maps has no correct interpretation.
explainable from its own document even after a later revision changes those defaults.
DOCKER_HOST; a resource declaring a same-named parameter would otherwise redirect the Dockersocket rather than configure itself. The
SCOPE_prefix is refused at declaration time for thesame reason.
400, never ignored: a silently droppedREPOS=typo would seed the default and produce a healthy-looking run that answers a differentquestion than the one asked. Same reasoning for a run conflicting with a profile-pinned value,
reported per parameter rather than per binding.
documented pattern derives a token inside setup and publishes it as an export.
Testing
merge rules, and the resolver.
vitest run packages/shared/src apps/api/src apps/portal/src— 2024 tests across 161 files.identical resolved parameters, differing only in tool surface, both passing their full criteria
set. Guardrails confirmed live — overriding a profile-pinned parameter and mistyping a
parameter name each return
400naming the offending parameter.Docs
docs/architecture/resources.mdcovers the entity, the orderings above, the parameter contractand precedence, why a soft-deleted slug stays reserved, and why there is no blanket container
sweep.