Run build, tests, and static checks in CI for PRs

[k5s-bot]

Closes #151.

What

Adds .github/workflows/ci.yml — a CI pipeline that runs the build, tests, and static checks for every pull request into main. Until now there was no PR-time CI; release.yml only runs on push to main, and validate-crds.yml only covers CRD samples.

How it works

Three parallel jobs, one per toolchain in the monorepo, so a failure names the thing that broke instead of one opaque red X:

Job	Covers	Steps
node	all npm workspaces (packages/*, apps/*, tools/*, tools-local/http-get-node, e2e)	npm ci → npm run build → npm run typecheck → npm run test (the root workspace fan-out scripts)
go (matrix)	sidecars/localtool-executor, tools-local/http-get-go	gofmt -l check → go build ./... → go vet ./... → go test ./...
core-controller	controllers/core-controller (kubebuilder)	make lint (golangci-lint: static analysis + gofmt/goimports) → make test (envtest-backed suite) → make build

The core-controller job reuses the project's existing Makefile targets so CI runs exactly what a developer runs locally, rather than a second, drifting definition of "the checks". Each target self-installs its pinned tool (golangci-lint, controller-gen, setup-envtest) on first use.

Design notes

• Runners: every job runs on GitHub-hosted ubuntu-latest — never the self-hosted arc-runner-set-agent-controller used by release.yml — because PRs (fork PRs included) are untrusted content that must not reach the internal cluster/registry. This mirrors the same rationale already documented in validate-crds.yml.
• Permissions: contents: read only.
• Concurrency: one in-flight run per PR branch; a new push cancels the previous run.
• Path filtering: paths-ignore: "**/*.md" skips pure-docs PRs. Because paths-ignore only skips when every changed file matches, a PR touching both Markdown and code still runs.

Verification

Ran the pipeline's commands locally against this branch's tree:

• Node: npm ci + build + typecheck + test — all green (19 tests across the workspaces that define them).
• sidecars/localtool-executor and tools-local/http-get-go: gofmt clean, go build, go vet, go test — all green.
• controllers/core-controller: go build ./... and go vet ./... green under Go 1.25.7; gofmt clean. (The full make test/make lint — which download envtest binaries and the custom golangci-lint — are exercised by this PR's own CI run.)

Opening this PR triggers the new workflow, so its own checks are the first live run.

---

Maintainers: apply the ai-review label to this PR to request an automated code review, or the ai-triage label to have review feedback addressed and the branch brought back in sync with its base. (The automation can't apply either label to its own PR, so a human needs to.)

[k5s-bot]

🤖 Starting work on this now. Watch live or take over the session here: https://claude.ai/code/session_01Ev6J4rJzJ1ghKd5nxqFAkm

[k5s-bot]

See summary below.

[k5s-bot]

Blocking: this Build step fails on a clean checkout. npm run build --workspaces --if-present runs workspaces in discovery order, not topological order. packages/agent-runtime builds before packages/messaging, but it depends on @controller-agent/messaging whose types resolve to dist/index.d.ts — which doesn't exist yet on a fresh checkout (no tsconfig project references to build it on demand). Result: Cannot find module '@controller-agent/messaging' (see run #30285125563). The description's "Node … all green" only held locally because a stale dist/ was present. Fix in the root build script — build leaf packages first (topo order / tsc references / a -w @controller-agent/messaging prestep) — before this check can pass.

[k5s-bot]

Blocking: make lint fails on pre-existing code. This runs the full .golangci.yml including the logcheck custom linter, which the current tree doesn't satisfy (multiple "accept either a context or a logger, but not both" violations in run #30285125563). The PR honestly notes make lint/make test weren't run locally. Because lint is the first step, test and build never run. Either fix the violations or narrow the linter set before this becomes a required check, otherwise the job is red on arrival.

[k5s-bot]

Correct call using ubuntu-latest here — verified release.yml runs on the self-hosted arc-runner-set-agent-controller (lines 172/273), so keeping untrusted PR content off that infra (as validate-crds.yml already documents) is the right security posture. contents: read + pull_request (not pull_request_target) reinforce it.

[k5s-bot]

paths-ignore: "**/*.md" semantics are correct (skips only when every changed file matches). One risk if these jobs become required status checks: a docs-only PR skips the whole workflow, the required contexts never report, and the PR hangs on "Expected — Waiting for status." If you intend them required, add a companion always-runs gate job (or make them required-if-run).

[k5s-bot]

Review — Add PR CI (ci.yml)

Solid, well-documented workflow. The structure (one job per toolchain), the security posture, and the reuse of the Makefile targets are all the right calls, and the inline comments explaining why are excellent. I verified the security rationale directly: release.yml does run on the self-hosted arc-runner-set-agent-controller (lines 172/273), so pinning every job here to ubuntu-latest with contents: read and plain pull_request (not pull_request_target) is exactly right for untrusted PR content. Toolchain coverage is complete — all three go.mod modules and all 16 workspaces are accounted for.

However, as of this review the workflow's own first run (#30285125563) is red on two of four jobs, so this CI cannot yet go green on the very PR that introduces it. Neither failure is a bug in the YAML — the workflow is correctly surfacing pre-existing problems — but both must be resolved before this becomes a useful (let alone required) check. Details inline; summary:

1. node → Build fails on a clean checkout (blocking)

npm run build --workspaces --if-present runs workspaces in discovery order, not topological order. packages/agent-runtime (alphabetically first) depends on @controller-agent/messaging, whose types resolve to dist/index.d.ts (packages/messaging/package.json), and there are no tsconfig project references to build it on demand. On a fresh CI checkout dist/ doesn't exist yet, so the consumer builds before the dependency and tsc fails with Cannot find module '@controller-agent/messaging' (confirmed in the run log). The "Node … all green" claim in the PR description passed locally only because a stale dist/ was present. This is a repo build-ordering gap rather than a flaw in ci.yml, but the new check stays red until the root build builds leaf packages first (topological order, tsc references, or an explicit -w @controller-agent/messaging prestep).

2. core-controller → Lint fails on pre-existing code (blocking)

make lint runs the full .golangci.yml (incl. the logcheck custom linter), which the current tree doesn't pass (multiple "accept either a context or a logger, but not both" violations). The PR honestly discloses that make lint/make test weren't run locally — CI now shows make lint failing before test/build even run. Decide whether to fix the violations or narrow the linter set before wiring this in as required.

3. paths-ignore + required checks (risk worth noting)

If these jobs are made required, a PR that touches only **/*.md skips the whole workflow, so the required contexts never report and the PR can hang on "Expected — Waiting for status." If you intend these to be required, add a companion always-runs gate job (or make them required-if-run) so docs-only PRs aren't blocked.

4. Minor

• actions/checkout@v4, setup-node@v4, setup-go@v5 emit Node 20 deprecation warnings on current runners — non-blocking, worth a future bump.

CI status as of this review: node ❌, core-controller ❌, both go matrix jobs ✅.

Nice work overall — once the build ordering and lint debt are handled, this is a genuinely valuable addition.

[k5s-bot]

Review posted. Here's my summary.

What I did

Reviewed PR #170 — "Run build, tests, and static checks in CI for PRs" (adds .github/workflows/ci.yml, a single 136-line file). I checked out the branch, examined the diff, mapped the monorepo's toolchains, verified the design claims against the repo, and inspected the PR's own first CI run.

Key findings

The workflow's structure and security posture are sound, but its own first run (#30285125563) is red on 2 of 4 jobs — the CI correctly surfaces two pre-existing problems, but they block it from going green:

1. node → Build fails on a clean checkout (blocking). npm run build --workspaces runs in discovery order, not topological order. packages/agent-runtime builds before packages/messaging but depends on it (types resolve to dist/, no tsconfig project references) → Cannot find module '@controller-agent/messaging'. The description's "all green locally" only passed because a stale dist/ existed. Fix belongs in the root build script.
2. core-controller → Lint fails on pre-existing code (blocking). make lint runs the full .golangci.yml (incl. logcheck), which the current tree doesn't pass. The PR honestly disclosed it didn't run this locally.
3. paths-ignore + required-checks risk: docs-only PRs skip the workflow, so if these become required contexts the PR can hang waiting for status.
4. Minor: Node 20 action-deprecation warnings.

I verified and cre…

---

✅ Opened pull request: imaustink/agent-controller#170

[k5s-bot]

⚠️ Your linked Claude Remote Control account's credential expired, so this request couldn't complete. To finish it, please link your Claude Remote Control account — I'll pick this request back up automatically as soon as you do.