Skip to content

Run the two typecheck scripts nothing ran, and build the images from their lockfiles - #121

Closed
Hotragn wants to merge 1 commit into
CopilotKit:mainfrom
Hotragn:run-the-checks-the-repo-already-wrote
Closed

Run the two typecheck scripts nothing ran, and build the images from their lockfiles#121
Hotragn wants to merge 1 commit into
CopilotKit:mainfrom
Hotragn:run-the-checks-the-repo-already-wrote

Conversation

@Hotragn

@Hotragn Hotragn commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

What this changes

Two gates that were written, committed, and never reached. Both cover agent-computer, which is the
process a Bot's input reaches a child process in.

1. Two typecheck scripts that never ran

agent-computer/package.json and supervisor/package.json each ship
"typecheck": "tsc --noEmit". Root typecheck is bun run --filter '*' typecheck, which enumerates
workspaces, and workspaces is ["app", "server", "worker"]. ci.yml runs bun run typecheck and
nothing else typechecks anything, so neither script has ever run in CI.

That is the process holding the only spawn in the deployment, and the only one holding a Docker
socket (supervisor/Dockerfile:1), reaching an image without a type check. #68 changed shell.ts
without one seeing it.

Both pass today, so this is a gate turned on rather than a backlog cleared:

agent-computer $ bunx tsc --noEmit     # clean
supervisor     $ bunx tsc --noEmit     # clean

A separate job rather than another line in static, because each installs from its own lockfile and
the root install cannot produce their dependency trees. Matrixed so each reports its own result with
fail-fast: false, and added to verify so branch protection needs no new entry — which is what #64
built that gate for.

agent-bot, agent-langgraph and the examples/ packages have no typecheck script at all. That is
a separate question and I have left it alone.

2. Lockfiles that were not merely unenforced — they were not in the build context

Every deployable but the root one installed with a plain bun install, and none of them copied its
bun.lock:

Dockerfile before
Dockerfile:48-49 COPY agent-computer/package.json … then bun install
agent-computer/Dockerfile:14-15 COPY agent-computer/package.json ./ then bun install
supervisor/Dockerfile:13-14 COPY supervisor/package.json ./ then bun install
agent-bot/Dockerfile:8-9 COPY … agent-bot/package.json … then bun install
agent-langgraph/Dockerfile:8-9 COPY … agent-langgraph/package.json … then bun install

So the committed lockfile could not have been honoured however the command was written — the file was
not there. Each build resolved afresh.

The root Dockerfile argues against this itself, twenty-three lines above the install that ignored it:

Bun is pinned. The installer takes whatever is newest otherwise, so the runtime drifts from the one
the lockfile was resolved against and an image built next month is not the image built today.

It also bears on #64. That change signs a build provenance attestation for the image digest, and
provenance over a tree resolved at build time attests to less than it looks like: the digest is exact,
and part of what went into it was decided by nothing in this repository.

All four now copy their lockfile and install --frozen-lockfile.

The examples/ packages keep their lockfiles and are untouched: nothing installs them in CI or in an
image, so there is no build to pin.

Closes #112

Where it runs

  • New state that outlives a request? None. No runtime code changes at all — this is CI
    configuration and four image builds.
  • What happens on the second replica? Nothing differs. No process behaviour changes; the images
    are built from a pinned dependency tree rather than a freshly resolved one, which is the same on
    every replica by construction.
  • Anything serialised? N/A.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No new listener or port. One new CI job, which runs per pull
    request like the others and is covered by verify.

Boundary and audit

  • No runtime path is touched, so resolve → decide → audit → act is unchanged.
  • No new refusals and no new rows.
  • Nothing new is trusted from a client. Strictly less is trusted from the network at build time: an
    image's dependency tree is now decided by a committed file rather than by whatever the registry
    serves that day.

Changelog

Added under UnreleasedChanged: "The images are built from the lockfiles that are committed."

Proof

The four installs were checked against their committed lockfiles before the flag was added, so
this pins what is already resolved rather than asking for a refresh. Each in a clean directory
containing only its package.json and bun.lock:

agent-computer   bun install --frozen-lockfile   63 packages
supervisor       bun install --frozen-lockfile   73 packages
agent-bot        bun install --frozen-lockfile   43 packages
agent-langgraph  bun install --frozen-lockfile   39 packages

Both new typechecks run clean locally, which is the whole of what the new job asserts:

agent-computer $ bunx tsc --noEmit     # clean
supervisor     $ bunx tsc --noEmit     # clean

ci.yml parses, and the new job is wired into the gate rather than only defined:

jobs:          static, test, build, migrations, image, deployables, verify
verify needs:  static, deployables, test, build, migrations, image
matrix:        {package: [agent-computer, supervisor]}

.dockerignore does not exclude bun.lock, so the new COPY lines resolve. The image job builds
the root Dockerfile on every pull request, so the changed build is exercised here rather than
first at a release.

bun run lint clean (including --error-on-warnings, from #120). This change touches only YAML,
Dockerfiles and Markdown, none of which biome formats, so format:check is unaffected by it.

What is not covered

  • The examples/ lockfiles, per above — nothing builds them.
  • agent-bot and agent-langgraph still have no typecheck script. Adding one is writing a new
    gate rather than turning on a written one, and might surface real errors; worth doing, worth doing
    separately.
  • The two new typechecks do not run in the image job, which builds rather than typechecks. They
    are their own job for that reason.
  • Nothing here refreshes a lockfile. If one of the four ever drifts from its package.json, the
    build now fails and says so, which is the point — but it does mean a dependency bump has to update
    the lockfile in the same commit.

…their lockfiles

Two gates were written, committed, and never reached.

`agent-computer` and `supervisor` each ship `"typecheck": "tsc --noEmit"`. Root
`typecheck` is `bun run --filter '*' typecheck`, which enumerates workspaces,
and `workspaces` is app, server and worker. So neither script has ever run:
the process holding the only `spawn` in the deployment, and the only one
holding a Docker socket, reached an image without a type check. Both pass
today, so this is a gate turned on rather than a backlog cleared.

A separate job rather than another line in `static`, because each installs
from its own lockfile and the root install cannot produce their dependency
trees. Matrixed so each reports its own result, and in `verify` so branch
protection needs no new entry.

The lockfiles are the other half, and they were worse than unenforced. Every
deployable but the root one installed with a plain `bun install`, and its
`bun.lock` was not in the build context at all, so the committed file could
not have been honoured however the command was written. Each build resolved
afresh: an image built next month was not the image built today, which is the
drift the note about pinning Bun is already there to prevent, twenty-three
lines above the install that ignored it. It is also what CopilotKit#64's provenance
attestation signs, so the digest was exact and part of what went into it was
decided by nothing in this repository.

`agent-computer`, `supervisor`, `agent-bot` and `agent-langgraph` now copy
their lockfile and install `--frozen-lockfile`. All four were checked against
their committed lockfile first, so this pins what is already resolved rather
than asking for a refresh.

The `examples/` packages keep their lockfiles and are left alone: nothing
installs them in CI or in an image, so there is no build to pin.
@Hotragn

Hotragn commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

One note for whoever picks this up: the checks here need approving before they run. Both runs are sitting at action_required rather than queued, because this changes .github/workflows/ci.yml and comes from a fork. So the green tick that would normally be the argument for this PR is the one thing I cannot produce for it.

What I could check locally is in the Proof section, and it is the part that decides whether the new job is worth having: both typecheck scripts pass clean today, and all four installs accept --frozen-lockfile against their committed lockfiles — 63, 73, 43 and 39 packages. So neither half should turn CI red on merge.

The one thing only CI can answer is whether bun install --frozen-lockfile in agent-computer stays quick on a clean runner. It pulls playwright, and I have set PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 on the job rather than relying on bun not running an untrusted postinstall. Locally that install is ~28s with no browser download. If it turns out slow on a runner I am happy to split the two packages differently or drop the matrix.

There is also a small irony worth naming: a PR whose whole subject is checks that never ran is a PR whose own checks will not run until somebody presses a button.

@davidmckayv

davidmckayv commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Thank you for this. Closing it in favour of #123, which covers the same ground and landed tonight, but the diagnosis here is yours: you raised #112 and this went up alongside it.

You were right on both halves. Two typecheck scripts committed and never reached, because --filter '*' enumerates workspaces and neither package is one. And lockfiles that were not merely unenforced but not in the build context at all, so the committed file could not have been honoured however the command was written. The observation that this also undercuts what #64 signs (a provenance attestation over a tree resolved at build time attests to less than it appears to) is the sharpest line in either description, and it is why this got picked up tonight.

One technical note that is worth having, because it would have bitten as soon as CI ran. Your deployables job installs in the package directory only:

- run: bun install --frozen-lockfile
  working-directory: ${{ matrix.package }}
- run: bun run typecheck
  working-directory: ${{ matrix.package }}

Both packages set types: ["bun"] while @types/bun is a root devDependency, so tsc resolves it by walking up to the root node_modules. On a fresh checkout with no root install:

agent-computer $ bun install --frozen-lockfile && bun run typecheck
error TS2688: Cannot find type definition file for 'bun'.

CI never ran here (fork PRs need releasing by a maintainer), so nothing surfaced it. The merged version adds a root install first, which is also the order the root Dockerfile already uses. Everything else about your job is the same shape and the same reasoning: separate job rather than another line in static, matrixed, fail-fast: false, wired into verify.

Two of yours we did not take, both worth recording:

PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD on the job was a sound instinct. Left off because bun does not run an untrusted package's postinstall anyway and the agent-computer job finishes in 14 seconds, so there is nothing being downloaded to skip. Good thinking, just not load-bearing here.

Your changelog entry is better than having none, which is what merged. That file is being caught up separately and your framing of the drift is the clearer one.

For what it is worth, I built all five images before merging and confirmed the effect you predicted: adding an unreviewed package to supervisor/package.json without touching its lockfile now fails the build, where on main it installed and the build reported success. In the image of the process holding the Docker socket. Your issue was right about why that matters.

Please do send more. Genuinely good find, and the write-up was a pleasure to read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The typecheck skips the two packages that run a Bot's computer, and six lockfiles are never enforced

2 participants