diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index da5e35e3f3..d673884d32 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -138,6 +138,78 @@ jobs: - name: Install dependencies run: bun ci + # THE ROUND TESTS HAD BEEN SKIPPING IN CI, EVERY RUN SINCE THEY WERE + # WRITTEN. + # + # `queen-round.test.ts` guards its behaviour cases with `it.if(present)`, + # where `present` means "the policy binary exists on this machine". + # Nothing in this workflow ever built it, so all seven skipped and the job + # reported green - the file that exists BECAUSE nothing in the repository + # called `runRound`, and that caught a critic deleting the lease guard, + # was proving nothing at all here. + # + # Built with the SAME compiler the Dockerfile uses, so the policy under + # test is the one Railway ships rather than a lookalike compiled by + # whatever toolchain a runner happens to carry. + # + # DYNAMICALLY LINKED, exactly as Railway builds it, with the runtime + # lifted out of the image beside it. + # + # The first attempt linked dynamically and failed every test with `queend + # exited 127: error while loading shared libraries` - a bare runner has no + # `libswiftCore.so`. The obvious repair, `--static-swift-stdlib`, worked + # and is WRONG here: the static toolchain links + # `/usr/lib/swift_static/linux/libFoundationEssentials.a`, a different + # Foundation from the dynamic one, and four daily-cap cases that pass on a + # dynamic build failed under it. A gate that changes the thing it measures + # is measuring itself. + # + # So the linkage matches production and the runtime travels with the + # binary. Same sources, same compiler, same Foundation. + - name: Build the Queen policy binary + if: matrix.suite == 'server-api' + run: | + set -euo pipefail + docker run --rm \ + -u "$(id -u):$(id -g)" \ + -e HOME=/tmp \ + -v "$PWD/queen-core":/src \ + -w /src \ + swift:6.0-jammy \ + swift build -c release 2>&1 | tail -30 + # The runtime the binary needs, taken from the image that built it. + docker run --rm \ + -u "$(id -u):$(id -g)" \ + -v "$PWD/queen-core":/out \ + swift:6.0-jammy \ + sh -c 'mkdir -p /out/.build/swift-runtime && cp -a /usr/lib/swift/linux/*.so* /out/.build/swift-runtime/' + echo "LD_LIBRARY_PATH=$PWD/queen-core/.build/swift-runtime" >> "$GITHUB_ENV" + # Assert the artifact, not the exit code: a build that prints an + # error and still exits 0 would otherwise leave the tests skipping + # exactly as before, under a step that looked like it had worked. + test -x queen-core/.build/release/queend + # And that it can START here, not merely that a file exists. A + # dynamically linked binary passes the test above and then fails every + # case with exit 127, which is how the first attempt at this step + # went. + # + # ONLY 127 IS THE FAILURE. The second attempt asserted a clean exit + # and broke the step itself: `queend` answers a JSON question on + # stdin, so `--help` and a bare `{}` both exit non-zero by design, and + # a gate that demanded zero was testing the question rather than the + # linkage. Any other code means the process ran, which is the whole + # claim being made. + set +e + LD_LIBRARY_PATH="$PWD/queen-core/.build/swift-runtime" \ + sh -c 'printf "{}" | queen-core/.build/release/queend' >/dev/null 2>&1 + started=$? + set -e + if [ "$started" -eq 127 ]; then + echo "queend cannot load its runtime here (exit 127): the build is" + echo "linked against shared libraries this runner does not have." + exit 1 + fi + - name: Resolve BrowserOS cache key if: matrix.needs_browser == true id: browseros-cache-key diff --git a/trios/agent-server/apps/server/tests/api/queen-round.test.ts b/trios/agent-server/apps/server/tests/api/queen-round.test.ts index f108e47872..5c8d90b7f3 100644 --- a/trios/agent-server/apps/server/tests/api/queen-round.test.ts +++ b/trios/agent-server/apps/server/tests/api/queen-round.test.ts @@ -14,10 +14,7 @@ import { runRound, } from '../../src/api/services/queen-tick' import { logger } from '../../src/lib/logger' -import { - queendPathEnvVar, - resolveQueendPath, -} from '../__helpers__/queend-path' +import { queendPathEnvVar, resolveQueendPath } from '../__helpers__/queend-path' /** * The round itself, driven against the real policy binary. @@ -161,6 +158,33 @@ afterEach(() => { }) describe('queen round, lease lost', () => { + /** + * A QUIET SKIP IS HOW A GATE REPORTS SUCCESS IT NEVER EARNED, and this file + * said exactly that in its own header while doing it. + * + * Every `it.if(present)` case below skipped on every CI run since they were + * written, because nothing in the workflow built the policy binary. The + * sentinel beneath this one does not catch that: it asserts the PATH STRING, + * which is true whether or not anything is at the end of it. + * + * A laptop without the binary is a fair place to skip - that is what the + * guard is for. CI is not: it is the only machine whose green anybody reads + * as coverage. So the absence fails THERE and nowhere else, and it names the + * command that fixes it rather than merely refusing. + */ + it('has the policy binary wherever green is read as coverage', () => { + if (!process.env.CI) return + if (!present) { + throw new Error( + 'the policy binary is missing in CI, so every behaviour test in this ' + + 'file skipped and the job would have reported green. Build it with ' + + '`swift build -c release` inside trios/agent-server/queen-core, as ' + + 'the Dockerfile and the "Build the Queen policy binary" step do.', + ) + } + expect(present).toBe(true) + }) + it('drives the binary the container drives', () => { // The hook above points the round at BIN through the same variable // production reads; the shared resolver must name that binary back, or diff --git a/trios/agent-server/apps/server/tests/api/queend-choose.test.ts b/trios/agent-server/apps/server/tests/api/queend-choose.test.ts index abe7546e94..918acd36c2 100644 --- a/trios/agent-server/apps/server/tests/api/queend-choose.test.ts +++ b/trios/agent-server/apps/server/tests/api/queend-choose.test.ts @@ -5,8 +5,8 @@ import { containerQueendPath, DOCKERFILE_PATH as DOCKERFILE, productionQueendFallback, - QUEEN_TICK_PATH as TICK, resolveQueendPath, + QUEEN_TICK_PATH as TICK, } from '../__helpers__/queend-path' /** @@ -122,7 +122,7 @@ describe('queend chooses the next bee', () => { const answer = ask(board([1176], [task(1176, 'running')])) // Swift omits a nil rather than encoding null, so the key is absent. expect(answer.chosen ?? null).toBeNull() - expect(String(answer.skipped)).toContain('a worker has it') + expect(String(answer.skipped)).toContain('a worker already has it') }) // rejected means the Queen sent it back and the same bee is expected to @@ -131,7 +131,9 @@ describe('queend chooses the next bee', () => { const answer = ask(board([1175], [task(1175, 'rejected')])) // Swift omits a nil rather than encoding null, so the key is absent. expect(answer.chosen ?? null).toBeNull() - expect(String(answer.skipped)).toContain('expected back') + expect(String(answer.skipped)).toContain( + 'claimed, but no worker is attached yet', + ) }) // A retry running over a past failure is claimed by the retry, whichever @@ -142,7 +144,7 @@ describe('queend chooses the next bee', () => { ) // Swift omits a nil rather than encoding null, so the key is absent. expect(answer.chosen ?? null).toBeNull() - expect(String(answer.skipped)).toContain('a worker has it') + expect(String(answer.skipped)).toContain('a worker already has it') }) /** @@ -213,11 +215,31 @@ describe('queend refuses to start a bee once the day is spent', () => { // $15 per million input tokens for claude-opus in ModelPricing.table, so // 800k input tokens is $12.00 exactly. Dated now, because the budget is a // DAILY one and a task updated yesterday must not count against today. + // + // SECONDS, NO FRACTION - the shape `boardTask` emits, not the shape + // `toISOString()` does. + // + // This fixture wrote `new Date().toISOString()`, which always carries + // milliseconds, and `queend` decodes with Swift's `.iso8601` strategy, which + // does not accept a fractional second. On Linux it answers + // `{"kind":"error","error":"could not decode the question: ... Expected date + // string to be ISO8601-formatted."}` and chooses nothing, so all four cases + // below failed there while passing on a Mac. + // + // The PRODUCT has been right about this for months: `isoSeconds` in + // `queen-tick.ts` strips the fraction before any task reaches the policy, and + // its comment records the same error at `codingPath: ["tasks", "Index 67"]`. + // The fixture was reproducing a bug the product had already fixed, and + // nothing noticed because these cases had never run in CI. + // + // A fixture that builds a shape production never emits tests a program that + // does not exist. function spentTask(issue: number, inputTokens: number) { + const seconds = new Date().toISOString().replace(/\.\d{3}Z$/, 'Z') return { ...task(issue, 'accepted'), - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), + createdAt: seconds, + updatedAt: seconds, provider: 'anthropic', model: 'claude-opus-4.5', inputTokens, @@ -230,7 +252,11 @@ describe('queend refuses to start a bee once the day is spent', () => { TRIOS_SWARM_BILLING_MODE: 'api_metered', TRIOS_SWARM_DAILY_CAP_USD: '5', }) - expect(answer.allowed).toBe(false) + // MATCHED AGAINST THE WHOLE ANSWER, so a failure prints what the binary + // actually said. `expect(answer.allowed).toBe(false)` reports only + // "Received: undefined", which is the least useful half of the fact when + // the policy is answering from a platform you cannot reproduce locally. + expect(answer).toMatchObject({ allowed: false }) // Swift omits a nil rather than encoding null, so the key is absent. expect(answer.chosen ?? null).toBeNull() // ModelPricing.format drops the cents above $10, so $12.00 prints as $12. @@ -260,7 +286,12 @@ describe('queend refuses to start a bee once the day is spent', () => { // Yesterday's spend is not today's. Without the day filter the cap would // latch shut permanently the first time a swarm had an expensive afternoon. it.skipIf(!present)('ignores spend from another day', () => { - const yesterday = new Date(Date.now() - 36 * 60 * 60 * 1000).toISOString() + // Seconds, no fraction - see `spentTask` above. Overriding `updatedAt` + // here put the milliseconds straight back and kept this one case red + // after the shared fixture was fixed. + const yesterday = new Date(Date.now() - 36 * 60 * 60 * 1000) + .toISOString() + .replace(/\.\d{3}Z$/, 'Z') const stale = { ...spentTask(999, 800_000), updatedAt: yesterday } const answer = ask(board([1201], [stale]), { TRIOS_SWARM_DAILY_CAP_USD: '5', diff --git a/trios/agent-server/apps/server/tests/api/send-back-valve-row.test.ts b/trios/agent-server/apps/server/tests/api/send-back-valve-row.test.ts index 96dbf9d5bd..19979dabbe 100644 --- a/trios/agent-server/apps/server/tests/api/send-back-valve-row.test.ts +++ b/trios/agent-server/apps/server/tests/api/send-back-valve-row.test.ts @@ -336,9 +336,11 @@ describe('the send-back valve on the stored rows for #1316 and #1318', () => { expect( String(answer.skipped), 'control - both issues skipped as claimed, the sentence the claimed bucket is made of', - ).toContain('#1316: a worker has it or is expected back (rejected)') + ).toContain( + '#1316: it is rejected - claimed, but no worker is attached yet', + ) expect(String(answer.skipped)).toContain( - '#1318: a worker has it or is expected back (rejected)', + '#1318: it is rejected - claimed, but no worker is attached yet', ) }, )