diff --git a/docs/UNVERIFIED.md b/docs/UNVERIFIED.md index 5d858da2..6d169a10 100644 --- a/docs/UNVERIFIED.md +++ b/docs/UNVERIFIED.md @@ -471,8 +471,10 @@ Each has a runbook mode. None has been run by an operator. | The whole M1 lifecycle against a real forge | **Mode Q** | Cancel, steer, the watchdog, the push gate and the charge ledger have only ever met a WireMock LLM and a local origin | | Corporate-only bundle → the failure it produces | Mode R §5 | The documented trap (internal forge works, model API fails) is asserted nowhere; it is the mistake an operator will actually make | | A private-registry pull | Mode S §4 | Nothing pulls from a private registry in any test. `authFor` and the attachment are unit-tested; the *pull* is not | -| **Codex CLI 0.156.1 in the agent image** (2026-09-23) | none yet | Raised from 0.146.0 so the model list includes the gpt-6 models. Re-checked on 0.156.1: every flag the adapter passes, the API-key login, and the shape of the file it writes (`auth_mode=apikey`). NOT re-checked: the `--json` event stream the usage parser reads, and the device sign-in output. Both need a paid run or a real sign-in, and the first of each proves or breaks them | +| **Codex CLI 0.156.1 in the agent image** (2026-09-23) | none yet | Raised from 0.146.0 so the model list includes the gpt-6 models. Re-checked on 0.156.1: every flag the adapter passes, the API-key login, and the shape of the file it writes (`auth_mode=apikey`). The device sign-in output was then proved by a real sign-in (2026-09-25), and one live `codex exec` on a subscription produced the same `--json` event types and the same five usage buckets. Still NOT re-checked: a multi-turn run with tool calls, which exercises the rest of the parser | | **Runs pinned to the image their model list came from** (M3.5 part M, 2026-09-23) | none yet | Choosing the pin is unit-tested against given daemon answers, and one real-daemon test pins a LOCAL build by its image id. No test pulls a registry image and pins it by its registry digest, and none runs two workers. So "two workers holding different images under one tag run the same one" is argued from the code, not watched. A local-only image is pinned by an id that exists on one daemon only: on a second worker such a run fails to pull — by design, but unobserved | +| **A build paid by a Codex subscription** (M3.5 part F, 2026-09-25) | none yet | Tests prove the emptied refresh token, one seat per account, seat rotation, the unmetered charge lines and the file the adapter writes. One live `codex exec` accepted a file with its refresh token emptied, three hours after sign-in with the id token expired. No item build has run on a seat yet. NOT observed: two builds using one seat at the same moment (seats are shared by the operator's decision of 2026-09-26; whether the vendor accepts two sessions of one account at once is unknown), a run that outlives the access token (10 days — the seat then needs a new sign-in), whether the vendor's CLI ever tries to refresh mid-run with an empty refresh token, and whether `tokens.account_id` is stable across sign-ins of one account | +| **Run-worker log lines are scrubbed of run secrets** (review of PR #178, 2026-09-25) | none yet | `SecretLogFilter` is unit-tested on a JBoss log record and its console configuration is asserted. Not observed: the filter on the JSON console handler of a running worker. And by design it cannot scrub a unit this process did not start: after a worker restart, the watchdog's log lines about an older unit carry that unit's secrets unscrubbed, as `RunFailures` already states for failure details | | **OIDC sessions actually renew instead of re-authenticating** | **Mode J check 11** (2026-09-10) | The bug it fixes needs a real browser, a real Keycloak and **fifteen elapsed minutes**. No suite here has any of the three: there are zero WebSocket client tests, and nothing observes a token reaching its `exp`. `OidcSessionsAreRenewedTest` asserts the four `application.yml` files *say* renewal is on — it cannot assert Quarkus *does* it | **Evidence needed.** An operator pass per mode. These are cheap and the runbooks are written. diff --git a/docs/superpowers/specs/2026-09-16-factory-m35-one-ticket-to-a-build-design.md b/docs/superpowers/specs/2026-09-16-factory-m35-one-ticket-to-a-build-design.md index 6f79786d..39b6dd83 100644 --- a/docs/superpowers/specs/2026-09-16-factory-m35-one-ticket-to-a-build-design.md +++ b/docs/superpowers/specs/2026-09-16-factory-m35-one-ticket-to-a-build-design.md @@ -216,6 +216,19 @@ EXECUTION-LAYER §3.3 with their date and the CLI version: 5. What a usage-limit refusal looks like on the NDJSON stream and in the exit code, so the pool can tell `rate_limited` from `rejected`, and which usage buckets a subscription run reports. +**Measured on 2026-09-25** from the first real sign-in made through 5.2's screen, with +`@openai/codex@0.156.1`. Field names, types and token lifetimes only; no value was printed: + +| Asked | Answer | +|---|---| +| 1. What a ChatGPT-mode `auth.json` holds | `auth_mode` = `chatgpt`; `OPENAI_API_KEY` null; `tokens.id_token` (JWT, **1 hour**); `tokens.access_token` (JWT, **10 days**); `tokens.refresh_token` (opaque); `tokens.account_id`; `last_refresh`. | +| Does `codex login --with-access-token` take that access token? | **No.** It expects an agent-identity JWT and refuses: "agent identity JWT payload is not valid JSON". 5.6's first plan does not work. | +| Does a file with an EMPTY refresh token work? | **Yes.** A missing `refresh_token` field is refused as malformed; an empty one is accepted ("Logged in using ChatGPT"). A live `codex exec` three hours after sign-in — the id token already expired — answered and exited 0. | +| 2. Does a run rewrite the file? | **Not that run:** the file was byte-identical afterwards. A run near the access token's expiry is not yet measured. | +| Usage on a subscription run | The same five buckets as an API-key run: input, cached input, cache write, output, reasoning. | + +Questions 3–5 (refresh-token rotation, a quota-free renewal command, the usage-limit refusal) remain open. + **Nothing is built on an unmeasured answer.** Where one is missing, the design takes the option that is safe when the guess is wrong: no automatic refresh, and a sign-in that is used until the vendor refuses it. @@ -240,6 +253,9 @@ it. ### 5.5 The lease is on agent activity, not on the item +> **Superseded 2026-09-26.** Seats are shared; there is no lease. The paragraphs below record the lease +> as designed and as reviewed; the decision and its reasons are the notes at the end of this section. + A sign-in serves one agent at a time. The lease must therefore start when the agent container starts and end when that container is gone — not when the item finishes: @@ -258,12 +274,31 @@ takes no sign-in and never replays the agent (`WorkRunWorker.java:91`). Cases th duplicate command delivery, worker death before release, a failed stop, a late release from an old lease, dispatch failure before the container exists, and two uploads of the same sign-in. +- **Decided 2026-09-26: no lease. Seats are shared like API keys.** A per-run lease was built and + reviewed twice (PR #178), and each round found another race between workers: a queued command + starting after its lease ran out, a failure result freeing the seat of an agent still running, a + worker that does not own a unit reporting it stopped. Closing them needs a durable start permit — + the same weight as the publication permit. The reason for the lease is gone instead: an agent's copy + of the sign-in has its refresh token emptied (§5.6), so two agents cannot refresh one sign-in and log + each other out. The operator chose to share seats. Several builds may use one seat at once; the + subscription's own limits decide how much they get, and seats rotate least recently used first. + **Not verified:** whether the vendor accepts two sessions of one account at the same moment. +- **One seat per account.** `account_ref` is filled from the measured `tokens.account_id`, an enabled + seat's account is unique per harness (V85), and a seat with no known account is never used. Two seats + on one account would share one subscription's limits while looking like two. Signing in again to an + account that has a seat replaces that seat's file — which is also how a seat whose access token + expired is renewed. Seats stored before this are identified at startup, under an advisory lock so two + orchestrators cannot both do it; older duplicates are switched off and cannot be switched back on + beside the newest. + ### 5.6 Injection and refresh — the agent never hands a credential back -- The worker passes the credential kind beside `HarnessInvocation.CREDENTIAL`. `CodexAdapter` pipes an - **access token** into `codex login --with-access-token` on stdin, exactly as it pipes an API key into - `--with-api-key` today (F0 measured both). Then it starts `codex exec`. No credential file is written - into the agent container, and nothing reaches argv or the environment. +- **Revised 2026-09-25 (5.3):** `--with-access-token` refuses a ChatGPT access token, so the first plan — + piping the access token into it — cannot work. Instead the worker hands the adapter the stored sign-in + file **with its refresh token emptied**, and `CodexAdapter` writes that file as the agent's + `auth.json` before `codex exec`. It arrives the same way an API key does today: in the environment + under the neutral credential name, never on argv. The access token and id token go in; the refresh token + does not. - **The refresh token never leaves the orchestrator.** An access token expires on its own; a refresh token does not, and an agent that reads one holds the sign-in until a person revokes it. Handing the agent the short-lived half is therefore not a detail of the plumbing — it is the whole difference @@ -315,6 +350,10 @@ translation, a retry-time field on the refusal, durable handling of that result, credential **version**, and the matching change to the arch guard `spire-arch/src/test/java/dev/codespire/arch/CredentialRefusalHasNoProducerTest.java`. +**Not built in the first part F change (2026-09-25).** A seat whose quota runs out is reported the way +any provider failure is today (collapsed as described above); nothing marks the seat resting. The first live runs show the operator the real quota +message, which is what this path must then classify. + ### 5.9 Risks, stated plainly - **The sign-in file is a person's account credential,** placed in a container that runs ticket text. A diff --git a/spire-contract/src/main/java/dev/codespire/contract/command/RunCommand.java b/spire-contract/src/main/java/dev/codespire/contract/command/RunCommand.java index 3de03c59..fc33997d 100644 --- a/spire-contract/src/main/java/dev/codespire/contract/command/RunCommand.java +++ b/spire-contract/src/main/java/dev/codespire/contract/command/RunCommand.java @@ -81,7 +81,7 @@ record ExecuteRun(String runId, RepoRef repo, String remoteUri, List protectedPaths, long maxWallClockSeconds, String scmCredential, String harnessCredential, boolean existingBranch, String protectedBranch, - String reasoningEffort) implements RunCommand { + String reasoningEffort, boolean harnessSignIn) implements RunCommand { // Every call site that predates ADR-040 keeps working and keeps the M0 rule — the // additive treatment the other wire records take. A run already on the bus reads as @@ -93,7 +93,7 @@ public ExecuteRun(String runId, RepoRef repo, String remoteUri, String scmCredential, String harnessCredential) { this(runId, repo, remoteUri, baseBranch, baseCommit, branch, prompt, harness, model, agentImage, protectedPaths, maxWallClockSeconds, scmCredential, - harnessCredential, false, "", null); + harnessCredential, false, "", null, false); } /** @@ -108,7 +108,22 @@ public ExecuteRun(String runId, RepoRef repo, String remoteUri, boolean existingBranch, String protectedBranch) { this(runId, repo, remoteUri, baseBranch, baseCommit, branch, prompt, harness, model, agentImage, protectedPaths, maxWallClockSeconds, scmCredential, - harnessCredential, existingBranch, protectedBranch, null); + harnessCredential, existingBranch, protectedBranch, null, false); + } + + /** + * Every caller written before subscriptions existed passes an API key (M3.5 part F). A run already + * on the bus decodes with false, which is what every such run carried. + */ + public ExecuteRun(String runId, RepoRef repo, String remoteUri, + String baseBranch, String baseCommit, String branch, + String prompt, String harness, String model, String agentImage, + List protectedPaths, long maxWallClockSeconds, + String scmCredential, String harnessCredential, + boolean existingBranch, String protectedBranch, String reasoningEffort) { + this(runId, repo, remoteUri, baseBranch, baseCommit, branch, prompt, harness, model, + agentImage, protectedPaths, maxWallClockSeconds, scmCredential, + harnessCredential, existingBranch, protectedBranch, reasoningEffort, false); } public ExecuteRun { @@ -170,7 +185,7 @@ public boolean pushesToAnExistingBranch() { public ExecuteRun onExistingBranch(String destination) { return new ExecuteRun(runId, repo, remoteUri, baseBranch, baseCommit, branch, prompt, harness, model, agentImage, protectedPaths, maxWallClockSeconds, scmCredential, - harnessCredential, true, destination, reasoningEffort); + harnessCredential, true, destination, reasoningEffort, harnessSignIn); } /** @@ -181,7 +196,18 @@ public ExecuteRun onExistingBranch(String destination) { public ExecuteRun atEffort(String level) { return new ExecuteRun(runId, repo, remoteUri, baseBranch, baseCommit, branch, prompt, harness, model, agentImage, protectedPaths, maxWallClockSeconds, scmCredential, - harnessCredential, existingBranch, protectedBranch, level); + harnessCredential, existingBranch, protectedBranch, level, harnessSignIn); + } + + /** + * The same run, where {@code harnessCredential} is a sealed sign-in file rather than an API key + * (M3.5 part F). The worker needs to know which, because the two are handed to the harness + * differently — and guessing from the bytes is how a key ends up written as a file. + */ + public ExecuteRun paidBySignIn() { + return new ExecuteRun(runId, repo, remoteUri, baseBranch, baseCommit, branch, prompt, + harness, model, agentImage, protectedPaths, maxWallClockSeconds, scmCredential, + harnessCredential, existingBranch, protectedBranch, reasoningEffort, true); } @Override @@ -198,6 +224,7 @@ public String toString() { + ", protectedPaths=" + protectedPaths + ", maxWallClockSeconds=" + maxWallClockSeconds + ", existingBranch=" + existingBranch + + ", harnessSignIn=" + harnessSignIn + ", protectedBranch=" + protectedBranch + ", reasoningEffort=" + reasoningEffort + ", promptChars=" + prompt.length() diff --git a/spire-contract/src/main/java/dev/codespire/contract/work/PayWith.java b/spire-contract/src/main/java/dev/codespire/contract/work/PayWith.java new file mode 100644 index 00000000..a15fdab1 --- /dev/null +++ b/spire-contract/src/main/java/dev/codespire/contract/work/PayWith.java @@ -0,0 +1,29 @@ +package dev.codespire.contract.work; + +/** + * How a build pays for its model calls (M3.5 part F). + * + *

Two values, as names rather than a boolean: a third way to pay is easy to imagine, and a flag called + * "subscription" would have to be rewritten to admit one. Every value written before this existed is an + * API key, because until then that was the only way a run could pay. + */ +public final class PayWith { + + /** Per token, with a key from the credential pool. */ + public static final String API_KEY = "API_KEY"; + + /** A signed-in seat: no per-token price, the real token counts still recorded. */ + public static final String SUBSCRIPTION = "SUBSCRIPTION"; + + private PayWith() { + } + + /** The value, with blank read as {@link #API_KEY}; anything else is refused rather than guessed. */ + public static String normalise(String value) { + if (value == null || value.isBlank()) return API_KEY; + String stripped = value.strip(); + if (!stripped.equals(API_KEY) && !stripped.equals(SUBSCRIPTION)) + throw new IllegalArgumentException("A build pays with " + API_KEY + " or " + SUBSCRIPTION + ", was: " + value); + return stripped; + } +} diff --git a/spire-contract/src/main/java/dev/codespire/contract/work/WorkPreparation.java b/spire-contract/src/main/java/dev/codespire/contract/work/WorkPreparation.java index e60efbcd..70dd7bb6 100644 --- a/spire-contract/src/main/java/dev/codespire/contract/work/WorkPreparation.java +++ b/spire-contract/src/main/java/dev/codespire/contract/work/WorkPreparation.java @@ -11,7 +11,7 @@ /** Pinned references and execution coordinates only; tracker artifact text never enters aggregate history. */ public record WorkPreparation(Artifact specification, Artifact plan, String baseBranch, String baseCommit, String harness, String model, String registeredBy, int bindingVersion, - String effort) { + String effort, String payWith) { /** Where an artifact's approved bytes live. Absent in stored history means {@link Origin#TRACKER}. */ public enum Origin { @@ -68,15 +68,28 @@ public record Artifact(WorkIssueLocation location, String sha256, Origin origin, */ public static final int EFFORT_BINDING = 3; + /** + * Adds how the build pays (M3.5 part F). Part of the binding because it decides what a build costs: + * a plan approved to run on a subscription must not start billing an API key per token because the + * build setup changed after the decision. Its own version for the reason the others have one. + */ + public static final int PAY_WITH_BINDING = 4; + + /** Every preparation written before payment modes existed pays with an API key. */ + public WorkPreparation(Artifact specification, Artifact plan, String baseBranch, String baseCommit, + String harness, String model, String registeredBy, int bindingVersion, String effort) { + this(specification, plan, baseBranch, baseCommit, harness, model, registeredBy, bindingVersion, effort, null); + } + public WorkPreparation(Artifact specification, Artifact plan, String baseBranch, String baseCommit, String harness, String model, String registeredBy) { - this(specification, plan, baseBranch, baseCommit, harness, model, registeredBy, TRACKER_BINDING, null); + this(specification, plan, baseBranch, baseCommit, harness, model, registeredBy, TRACKER_BINDING, null, null); } /** Every preparation written before thinking levels existed carries the model's own default. */ public WorkPreparation(Artifact specification, Artifact plan, String baseBranch, String baseCommit, String harness, String model, String registeredBy, int bindingVersion) { - this(specification, plan, baseBranch, baseCommit, harness, model, registeredBy, bindingVersion, null); + this(specification, plan, baseBranch, baseCommit, harness, model, registeredBy, bindingVersion, null, null); } public WorkPreparation { @@ -90,13 +103,18 @@ public WorkPreparation(Artifact specification, Artifact plan, String baseBranch, baseCommit = baseCommit.toLowerCase(java.util.Locale.ROOT); // Absent in older stored JSON, where every artifact was a tracker ticket. bindingVersion = bindingVersion == 0 ? TRACKER_BINDING : bindingVersion; - if (bindingVersion != TRACKER_BINDING && bindingVersion != STORED_BINDING && bindingVersion != EFFORT_BINDING) + if (bindingVersion < TRACKER_BINDING || bindingVersion > PAY_WITH_BINDING) throw new IllegalArgumentException("Unknown preparation binding version " + bindingVersion); effort = ThinkingLevel.normalise(effort); // A level under a version that does not hash it would be carried to the build without being part // of what was approved -- exactly what the version exists to prevent. if (effort != null && bindingVersion < EFFORT_BINDING) throw new IllegalArgumentException("A thinking level needs binding version " + EFFORT_BINDING); + payWith = PayWith.normalise(payWith); + // A subscription under a version that does not hash it would reach the build without being part + // of what was approved. + if (!payWith.equals(PayWith.API_KEY) && bindingVersion < PAY_WITH_BINDING) + throw new IllegalArgumentException("Paying with " + payWith + " needs binding version " + PAY_WITH_BINDING); if (bindingVersion == TRACKER_BINDING && (specification.origin() != Origin.TRACKER || plan.origin() != Origin.TRACKER)) throw new IllegalArgumentException("A version 1 binding describes tracker artifacts only"); @@ -123,6 +141,7 @@ public String binding() { String level = effort == null ? "" : effort; value.append(level.length()).append(':').append(level); } + if (bindingVersion >= PAY_WITH_BINDING) value.append(payWith.length()).append(':').append(payWith); return digest(value.toString()); } diff --git a/spire-contract/src/test/java/dev/codespire/contract/command/ExecuteRunBranchModeTest.java b/spire-contract/src/test/java/dev/codespire/contract/command/ExecuteRunBranchModeTest.java index 3ff27797..d8c443a0 100644 --- a/spire-contract/src/test/java/dev/codespire/contract/command/ExecuteRunBranchModeTest.java +++ b/spire-contract/src/test/java/dev/codespire/contract/command/ExecuteRunBranchModeTest.java @@ -47,6 +47,15 @@ void theThinkingLevelSurvivesEveryRebuildAndTheOthersSurviveIt() { assertThrows(IllegalArgumentException.class, () -> run().atEffort("high'; rm")); } + /** A sign-in is written as a file and a key is piped into a login; losing the flag swaps the two. */ + @Test + void aRunPaidBySignInSaysSoThroughEveryRebuild() { + RunCommand.ExecuteRun run = run().paidBySignIn().atEffort("high").onExistingBranch("develop"); + + assertTrue(run.harnessSignIn()); + assertFalse(run().harnessSignIn(), "every run before part F paid with a key"); + } + @Test void aCommandThatSaysNothingUsesTheNamespaceMode() { assertFalse(run().pushesToAnExistingBranch()); diff --git a/spire-contract/src/test/java/dev/codespire/contract/work/WorkPreparationBindingVersionTest.java b/spire-contract/src/test/java/dev/codespire/contract/work/WorkPreparationBindingVersionTest.java index 2871bb1e..a6d9b341 100644 --- a/spire-contract/src/test/java/dev/codespire/contract/work/WorkPreparationBindingVersionTest.java +++ b/spire-contract/src/test/java/dev/codespire/contract/work/WorkPreparationBindingVersionTest.java @@ -137,6 +137,34 @@ void aLevelThatIsNotAPlainWordIsRefused() { assertEquals(null, atLevel(" ").effort(), "blank is the model's own default, not a level called blank"); } + private static WorkPreparation paying(String payWith) { + UUID specId = UUID.fromString("00000000-0000-4000-8000-000000000071"); + UUID planId = UUID.fromString("00000000-0000-4000-8000-000000000072"); + return new WorkPreparation( + new WorkPreparation.Artifact(ticket("71"), SPEC_SHA, WorkPreparation.Origin.STORED, specId), + new WorkPreparation.Artifact(ticket("72"), PLAN_SHA, WorkPreparation.Origin.STORED, planId), + "main", COMMIT, "codex", "TEST-model", "system", WorkPreparation.PAY_WITH_BINDING, null, payWith); + } + + /** How a build pays decides what it costs, so an approval for one is not an approval for the other. */ + @Test + void howABuildPaysIsPartOfWhatIsApproved() { + assertNotEquals(paying(PayWith.API_KEY).binding(), paying(PayWith.SUBSCRIPTION).binding()); + assertEquals(PayWith.API_KEY, paying(null).payWith(), "no choice is the API key every build used before"); + assertNotEquals(atLevel(null).binding(), paying(PayWith.API_KEY).binding(), "version 4 is not version 3"); + } + + /** Versions 1 to 3 do not hash the payment, so they may not carry a subscription to the build. */ + @Test + void aSubscriptionUnderAVersionThatDoesNotHashItIsRefused() { + UUID specId = UUID.randomUUID(), planId = UUID.randomUUID(); + assertThrows(IllegalArgumentException.class, () -> new WorkPreparation( + new WorkPreparation.Artifact(ticket("71"), SPEC_SHA, WorkPreparation.Origin.STORED, specId), + new WorkPreparation.Artifact(ticket("72"), PLAN_SHA, WorkPreparation.Origin.STORED, planId), + "main", COMMIT, "codex", "TEST-model", "system", WorkPreparation.EFFORT_BINDING, null, PayWith.SUBSCRIPTION)); + assertThrows(IllegalArgumentException.class, () -> paying("TEST-free")); + } + @Test void anUnknownVersionIsRefusedRatherThanHashedSomeOtherWay() { assertThrows(IllegalArgumentException.class, () -> new WorkPreparation( diff --git a/spire-contract/src/test/resources/contract-schema.txt b/spire-contract/src/test/resources/contract-schema.txt index 5cc33bca..aa50994c 100644 --- a/spire-contract/src/test/resources/contract-schema.txt +++ b/spire-contract/src/test/resources/contract-schema.txt @@ -43,7 +43,7 @@ RefuseFinding(reviewId: java.lang.String, repo: dev.codespire.contract.scm.RepoR # RunCommand CancelRun(runId: java.lang.String, reason: java.lang.String) -ExecuteRun(runId: java.lang.String, repo: dev.codespire.contract.scm.RepoRef, remoteUri: java.lang.String, baseBranch: java.lang.String, baseCommit: java.lang.String, branch: java.lang.String, prompt: java.lang.String, harness: java.lang.String, model: java.lang.String, agentImage: java.lang.String, protectedPaths: java.util.List, maxWallClockSeconds: long, scmCredential: java.lang.String, harnessCredential: java.lang.String, existingBranch: boolean, protectedBranch: java.lang.String, reasoningEffort: java.lang.String) +ExecuteRun(runId: java.lang.String, repo: dev.codespire.contract.scm.RepoRef, remoteUri: java.lang.String, baseBranch: java.lang.String, baseCommit: java.lang.String, branch: java.lang.String, prompt: java.lang.String, harness: java.lang.String, model: java.lang.String, agentImage: java.lang.String, protectedPaths: java.util.List, maxWallClockSeconds: long, scmCredential: java.lang.String, harnessCredential: java.lang.String, existingBranch: boolean, protectedBranch: java.lang.String, reasoningEffort: java.lang.String, harnessSignIn: boolean) ExecuteWorkRun(runId: java.lang.String, execution: dev.codespire.contract.command.RunCommand$ExecuteRun, work: dev.codespire.contract.work.WorkRunBinding) HoldWorkRun(runId: java.lang.String, work: dev.codespire.contract.work.WorkRunBinding) PublishWorkRun(runId: java.lang.String, permit: dev.codespire.contract.work.WorkPublicationPermit, scmCredential: java.lang.String) diff --git a/spire-harness-codex/src/main/java/dev/codespire/harness/codex/CodexAdapter.java b/spire-harness-codex/src/main/java/dev/codespire/harness/codex/CodexAdapter.java index d5f230ae..3e5a0943 100644 --- a/spire-harness-codex/src/main/java/dev/codespire/harness/codex/CodexAdapter.java +++ b/spire-harness-codex/src/main/java/dev/codespire/harness/codex/CodexAdapter.java @@ -52,6 +52,12 @@ public final class CodexAdapter implements HarnessAdapter { /** What the Codex process reads its key from. Vendor knowledge, and this arm's alone. */ private static final String API_KEY_VARIABLE = "OPENAI_API_KEY"; + /** + * Where the sign-in file waits for the login step to write it (M3.5 part F). This arm's own name: the + * script below reads it once, writes the file Codex reads, and unsets it before Codex starts. + */ + private static final String SIGN_IN_VARIABLE = "CODEX_SIGN_IN_FILE"; + /** * The exit code the login step uses when it cannot authenticate, so a run that never reached * the model is not reported as one the model failed to answer. @@ -122,8 +128,7 @@ public List command(HarnessInvocation invocation) { // nothing lingers to interpret anything, and the two interpolations are quoted and refused // if they could close the quote. The prompt still arrives on the harness's stdin, which the // entrypoint redirects into the whole command; the login reads its own stdin from the pipe. - String script = "printenv " + API_KEY_VARIABLE + " | codex login --with-api-key >/dev/null" - + " || exit " + LOGIN_FAILED + "; " + String script = login(invocation) + "exec codex exec --json --sandbox danger-full-access --skip-git-repo-check" + " --model " + quoted(invocation.model(), "model") + effort(invocation.reasoningEffort()) @@ -132,6 +137,27 @@ public List command(HarnessInvocation invocation) { return List.of("sh", "-c", script); } + /** + * How this run signs Codex in: a key piped into {@code --with-api-key}, or a sign-in file written + * where Codex reads one. + * + *

The file is written because no login flag takes it. {@code --with-access-token} was the plan and + * refuses a ChatGPT access token (measured 2026-09-25, codex-cli 0.156.1: design §5.3); the file, + * with its refresh token emptied by the orchestrator, is accepted and runs. Written owner-only + * ({@code umask 077}), and the variable that carried it is unset before Codex starts, so the process + * that runs ticket text does not also inherit it. The agent can still read the file, as it can read + * a key today — what it cannot hold is a refresh token, because none was ever sent. + */ + private static String login(HarnessInvocation invocation) { + if (invocation.credentials().containsKey(HarnessInvocation.SIGN_IN)) { + return "mkdir -p \"$HOME/.codex\" && (umask 077 && printenv " + SIGN_IN_VARIABLE + + " > \"$HOME/.codex/auth.json\") || exit " + LOGIN_FAILED + "; " + + "unset " + SIGN_IN_VARIABLE + "; "; + } + return "printenv " + API_KEY_VARIABLE + " | codex login --with-api-key >/dev/null" + + " || exit " + LOGIN_FAILED + "; "; + } + /** * The thinking level as a config override, or nothing, so the model's own default applies. * @@ -177,6 +203,10 @@ public Map environment(HarnessInvocation invocation) { if (apiKey != null) { credentials.put(API_KEY_VARIABLE, apiKey); } + String signIn = credentials.remove(HarnessInvocation.SIGN_IN); + if (signIn != null) { + credentials.put(SIGN_IN_VARIABLE, signIn); + } return EnvironmentPolicy.merge(credentials, OWN_SETTINGS); } @@ -341,7 +371,7 @@ public TerminalOutcome classify(int exitCode, RunEventSummary seen) { // never started, so nothing about the model or the work item is implicated. The first // live dispatch spent its diagnosis on the model because this case had no name. return TerminalOutcome.failure(FailureCause.HARNESS_EXIT_NONZERO, - "codex login did not accept the API key, so the harness never ran"); + "codex login failed with the run's credential (a key or a sign-in file), so the harness never ran"); } if (!seen.sawAnyOutput()) { // Distinct and nameable: the model spent its whole budget and said nothing. Reported as diff --git a/spire-harness-codex/src/test/java/dev/codespire/harness/codex/CodexAdapterTest.java b/spire-harness-codex/src/test/java/dev/codespire/harness/codex/CodexAdapterTest.java index 85698291..1f28cbfe 100644 --- a/spire-harness-codex/src/test/java/dev/codespire/harness/codex/CodexAdapterTest.java +++ b/spire-harness-codex/src/test/java/dev/codespire/harness/codex/CodexAdapterTest.java @@ -55,6 +55,31 @@ void theChosenThinkingLevelReachesCodexAndNoLevelMeansNoOverride() { assertFalse(script(adapter.command(invocation())).contains("model_reasoning_effort")); } + /** + * A sign-in is WRITTEN where Codex reads one, owner-only, and the variable that carried it is unset + * before Codex starts; no key is piped into a login (M3.5 part F — design §5.3 measured that no login + * flag takes a ChatGPT sign-in). + */ + @Test + void aSignInIsWrittenAsTheFileCodexReadsAndNotPipedIntoALogin() { + HarnessInvocation signedIn = new HarnessInvocation("run_abc", "fix the bug", "/workspace", "gpt-5.6", + Map.of(HarnessInvocation.SIGN_IN, "{\"auth_mode\":\"TEST\"}"), Duration.ofMinutes(30)); + + String script = script(adapter.command(signedIn)); + Map env = adapter.environment(signedIn); + + assertTrue(script.contains("umask 077 && printenv CODEX_SIGN_IN_FILE > \"$HOME/.codex/auth.json\""), script); + assertTrue(script.contains("unset CODEX_SIGN_IN_FILE; exec codex exec"), script); + assertFalse(script.contains("--with-api-key"), script); + assertEquals("{\"auth_mode\":\"TEST\"}", env.get("CODEX_SIGN_IN_FILE")); + assertFalse(env.containsKey("OPENAI_API_KEY")); + } + + @Test + void anApiKeyIsStillPipedIntoTheLogin() { + assertTrue(script(adapter.command(invocation())).startsWith("printenv OPENAI_API_KEY | codex login --with-api-key")); + } + @Test void theTypeIsCodex() { assertEquals(HarnessType.CODEX, adapter.type()); diff --git a/spire-harness/src/main/java/dev/codespire/harness/HarnessInvocation.java b/spire-harness/src/main/java/dev/codespire/harness/HarnessInvocation.java index 9138dd33..c87560e8 100644 --- a/spire-harness/src/main/java/dev/codespire/harness/HarnessInvocation.java +++ b/spire-harness/src/main/java/dev/codespire/harness/HarnessInvocation.java @@ -31,6 +31,14 @@ public HarnessInvocation(String runId, String prompt, String workspacePath, */ public static final String CREDENTIAL = "HARNESS_CREDENTIAL"; + /** + * The key under which the worker supplies a SIGN-IN FILE instead of an API key (M3.5 part F): the + * vendor CLI's own sign-in record, with its refresh token already emptied by the orchestrator. A + * separate key rather than a flag beside {@link #CREDENTIAL}, so an arm that knows only API keys + * never mistakes a file for a key and pipes it into a login that would echo it. + */ + public static final String SIGN_IN = "HARNESS_SIGN_IN"; + public HarnessInvocation { Objects.requireNonNull(runId, "runId"); Objects.requireNonNull(prompt, "prompt"); diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/BuildDefaults.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/BuildDefaults.java index e13021d1..f6e68ed0 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/BuildDefaults.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/BuildDefaults.java @@ -24,6 +24,7 @@ public class BuildDefaults { @Inject LlmModelRegistry models; @Inject dev.codespire.orchestrator.llm.LlmModelPricer pricer; @Inject HarnessCatalogues catalogues; + @Inject HarnessCredentialPool pool; /** * @param revision 0 when the repository has none yet, with null coordinates — "not set" is a state @@ -31,18 +32,28 @@ public class BuildDefaults { */ /** * @param effort the thinking level, or null for the model's own default — a real choice, not a gap + * @param payWith {@code API_KEY} or {@code SUBSCRIPTION} (M3.5 part F) */ public record Defaults(long revision, String baseBranch, String harness, String model, String effort, - String updatedBy, Instant updatedAt) { - public static Defaults none() { return new Defaults(0, null, null, null, null, null, null); } + String payWith, String updatedBy, Instant updatedAt) { + public static Defaults none() { return new Defaults(0, null, null, null, null, null, null, null); } public boolean set() { return revision > 0; } } - /** @param effort null for the model's own default */ - public record Input(long expectedRevision, String baseBranch, String harness, String model, String effort) { + /** + * @param effort null for the model's own default + * @param payWith null or blank for an API key, which is what every setup paid with before part F + */ + public record Input(long expectedRevision, String baseBranch, String harness, String model, String effort, + String payWith) { /** Every caller written before thinking levels existed keeps the model's own default. */ public Input(long expectedRevision, String baseBranch, String harness, String model) { - this(expectedRevision, baseBranch, harness, model, null); + this(expectedRevision, baseBranch, harness, model, null, null); + } + + /** Every caller written before payment modes existed pays with an API key. */ + public Input(long expectedRevision, String baseBranch, String harness, String model, String effort) { + this(expectedRevision, baseBranch, harness, model, effort, null); } } @@ -63,14 +74,14 @@ public Defaults get(UUID repository) { * against the saved one INSIDE the transaction that registers the result (M3.5 part C). */ public Defaults get(Connection c, UUID repository, boolean lock) throws SQLException { - String sql = "SELECT revision,base_branch,harness,model,effort,updated_by,updated_at" + String sql = "SELECT revision,base_branch,harness,model,effort,pay_with,updated_by,updated_at" + " FROM repository_build_defaults WHERE repository_id=?" + (lock ? " FOR UPDATE" : ""); try (PreparedStatement ps = c.prepareStatement(sql)) { ps.setObject(1, repository); try (ResultSet rs = ps.executeQuery()) { if (!rs.next()) return Defaults.none(); return new Defaults(rs.getLong(1), rs.getString(2), rs.getString(3), rs.getString(4), - rs.getString(5), rs.getString(6), rs.getTimestamp(7).toInstant()); + rs.getString(5), rs.getString(6), rs.getString(7), rs.getTimestamp(8).toInstant()); } } } @@ -97,14 +108,23 @@ public Defaults save(UUID repository, Input input, String actor) { throw new Refused("harness_unconfigured"); if (model == null || !DispatchRequestParser.isModelName(model)) throw new Refused("model_name_invalid"); checkAgainstTheHarness(harness, model, effort); - // Exactly what the dispatch refuses, asked here: the model must be offered, and it must price - // every token type this harness can report. A model disabled AFTER this save is refused at - // dispatch too (WorkRunAssembly), so the two no longer disagree in either direction. - if (models.list().stream().noneMatch(known -> known.enabled() && known.name().equals(model))) - throw new Refused("model_unknown"); - var unpriced = pricer.unpricedTypes(model, harness); - if (!unpriced.isEmpty()) throw new Refused("model_pricing_incomplete:" - + unpriced.stream().map(Enum::name).collect(java.util.stream.Collectors.joining(","))); + String payWith; + try { payWith = dev.codespire.contract.work.PayWith.normalise(input.payWith()); } + catch (IllegalArgumentException unknown) { throw new Refused("pay_with_invalid"); } + if (payWith.equals(dev.codespire.contract.work.PayWith.SUBSCRIPTION)) { + // A subscription is not priced per token, so the model needs no rates and no catalogue entry + // — the harness's own list above is what says it can run. What it does need is a seat. + if (!pool.hasSubscription(harness)) throw new Refused("subscription_not_signed_in"); + } else { + // Exactly what the dispatch refuses, asked here: the model must be offered, and it must price + // every token type this harness can report. A model disabled AFTER this save is refused at + // dispatch too (WorkRunAssembly), so the two no longer disagree in either direction. + if (models.list().stream().noneMatch(known -> known.enabled() && known.name().equals(model))) + throw new Refused("model_unknown"); + var unpriced = pricer.unpricedTypes(model, harness); + if (!unpriced.isEmpty()) throw new Refused("model_pricing_incomplete:" + + unpriced.stream().map(Enum::name).collect(java.util.stream.Collectors.joining(","))); + } try (Connection c = dataSource.getConnection()) { // Same lock order as the policy save: the repository row first, so a save cannot interleave // with a repository or account edit that decides whether these coordinates can run at all. @@ -115,14 +135,15 @@ public Defaults save(UUID repository, Input input, String actor) { if (get(c, repository, true).revision() != input.expectedRevision()) throw new Refused("build_defaults_changed"); try (PreparedStatement ps = c.prepareStatement(""" - INSERT INTO repository_build_defaults(repository_id,base_branch,harness,model,effort,updated_by) - VALUES (?,?,?,?,?,?) + INSERT INTO repository_build_defaults(repository_id,base_branch,harness,model,effort,updated_by,pay_with) + VALUES (?,?,?,?,?,?,?) ON CONFLICT (repository_id) DO UPDATE SET base_branch=excluded.base_branch,harness=excluded.harness, model=excluded.model,effort=excluded.effort,updated_by=excluded.updated_by,updated_at=now(), + pay_with=excluded.pay_with, revision=repository_build_defaults.revision+1 """)) { ps.setObject(1, repository); ps.setString(2, branch); ps.setString(3, harness); - ps.setString(4, model); ps.setString(5, effort); ps.setString(6, actor); + ps.setString(4, model); ps.setString(5, effort); ps.setString(6, actor); ps.setString(7, payWith); ps.executeUpdate(); } // Saving a setup is the repair for "this repository has no build setup", so the items that diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/FactoryRunProjection.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/FactoryRunProjection.java index f1ba7f73..0b2b64b9 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/FactoryRunProjection.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/FactoryRunProjection.java @@ -148,6 +148,25 @@ public Optional modelOf(String runId) { * The caller must treat both the same way and mark nothing: guessing a member here would take * a working key out of rotation on the strength of a run that never used it. */ + /** + * How a run paid: {@code API_KEY} or {@code SUBSCRIPTION}, empty for a run this deployment never + * queued. Unlike the credential, a re-arm cannot clear it. + * + * @throws IllegalStateException when the row cannot be read. Not "empty": an unreadable row read as + * an API key would price a subscription run at a key's rates. + */ + public Optional paidByOf(String runId) { + try (Connection c = dataSource.getConnection(); + PreparedStatement ps = c.prepareStatement("SELECT paid_by FROM factory_run WHERE run_id = ?")) { + ps.setString(1, runId); + try (ResultSet rs = ps.executeQuery()) { + return rs.next() ? Optional.of(rs.getString("paid_by")) : Optional.empty(); + } + } catch (SQLException e) { + throw new IllegalStateException("How run " + runId + " paid could not be read", e); + } + } + public Optional harnessCredentialOf(String runId) { String sql = "SELECT harness_credential_id FROM factory_run WHERE run_id = ?"; try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(sql)) { @@ -174,7 +193,8 @@ public record RunView(String runId, String status, String pushedRef, List blocked, String failureCause, String failureDetail, String unitId, String prUrl, String prError, Instant agentStartedAt, - String kind, String harness, String model, String credentialLabel, String credentialType, String providerType, + String kind, String harness, String model, String credentialLabel, String credentialType, + String credentialAuthMode, String providerType, String workspace, String slug, String subject, int attempt, String baseBranch, String baseCommit, String branch, String pushedAs, String reviewId, String findingRef, String taskSummary, @@ -370,11 +390,34 @@ private static Instant instant(ResultSet rs, String column) throws SQLException * Deliberately NOT part of the re-arm comparison: a retry may * legitimately draw a different member, which is the rotation working * rather than a different request. + * @param paidBy {@code API_KEY} or {@code SUBSCRIPTION}: how the run pays, which decides + * how it is charged. Kept apart from the credential because a re-arm + * clears that and must not change this (M3.5 part F). */ public record QueuedRun(String runId, String harness, String model, String baseBranch, String baseCommit, String branch, String pushedAs, UUID harnessCredentialId, String kind, String reviewId, - String findingRef, String commentId) { + String findingRef, String commentId, String paidBy) { + + public QueuedRun { + paidBy = dev.codespire.contract.work.PayWith.normalise(paidBy); + } + + /** Every run before M3.5 part F paid with an API key. */ + public QueuedRun(String runId, String harness, String model, String baseBranch, + String baseCommit, String branch, String pushedAs, + UUID harnessCredentialId, String kind, String reviewId, + String findingRef, String commentId) { + this(runId, harness, model, baseBranch, baseCommit, branch, pushedAs, + harnessCredentialId, kind, reviewId, findingRef, commentId, null); + } + + /** The same row, paid by a signed-in subscription seat. */ + public QueuedRun paidBySubscription() { + return new QueuedRun(runId, harness, model, baseBranch, baseCommit, branch, pushedAs, + harnessCredentialId, kind, reviewId, findingRef, commentId, + dev.codespire.contract.work.PayWith.SUBSCRIPTION); + } /** A build run: what every dispatch was before M2, and what the REST endpoint still sends. */ public QueuedRun(String runId, String harness, String model, String baseBranch, @@ -415,7 +458,7 @@ public QueuedRun asFixFor(String reviewId, String findingRef, String commentId) + "it, or a redelivery of that comment buys a second run with no symptom"); } return new QueuedRun(runId, harness, model, baseBranch, baseCommit, branch, pushedAs, - harnessCredentialId, RunKind.FIX.name(), reviewId, findingRef, commentId); + harnessCredentialId, RunKind.FIX.name(), reviewId, findingRef, commentId, paidBy); } } @@ -496,8 +539,8 @@ public boolean queued(QueuedRun row, String taskSummary, UUID repositoryId) { INSERT INTO factory_run (run_id, provider_type, workspace, slug, subject, attempt, status, harness, model, base_branch, base_commit, branch, pushed_as, harness_credential_id, kind, review_id, finding_ref, - comment_id, task_summary, repository_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + comment_id, task_summary, repository_id, paid_by) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (run_id) DO UPDATE -- The credential is NULLED on a re-arm, not carried and not overwritten, and this -- is a correctness rule rather than tidiness. The re-arm exists because the FIRST @@ -522,7 +565,10 @@ -- the feedback class states as its own rule. SET status = EXCLUDED.status, failure_cause = NULL, failure_detail = NULL, ended_at = NULL, harness_credential_id = NULL, task_summary = EXCLUDED.task_summary - WHERE factory_run.status = ? AND factory_run.failure_cause = ? + WHERE factory_run.status = ? + -- How the run pays is part of what it IS: a retry that pays another way is a + -- different request, refused like any other differing component. + AND factory_run.paid_by = EXCLUDED.paid_by AND factory_run.failure_cause = ? AND factory_run.harness = EXCLUDED.harness AND factory_run.model = EXCLUDED.model AND factory_run.base_branch = EXCLUDED.base_branch AND factory_run.base_commit = EXCLUDED.base_commit AND factory_run.branch = EXCLUDED.branch @@ -567,8 +613,9 @@ -- the feedback class states as its own rule. // column, and a finished run knows only a branch name and a list of paths. ps.setString(19, taskSummary); ps.setObject(20, repositoryId); - ps.setString(21, FAILED); - ps.setString(22, DISPATCH_FAILED); + ps.setString(21, row.paidBy()); + ps.setString(22, FAILED); + ps.setString(23, DISPATCH_FAILED); // 1 on insert and on a re-arm; 0 when ON CONFLICT matched a row the WHERE declined to // touch. That 0 used to be discarded, and the dispatch went ahead anyway. changed = ps.executeUpdate() == 1; @@ -1015,14 +1062,14 @@ public boolean exists(String runId) { public Optional find(String runId) { // The credential names WHO paid for this run and how it is billed. Its secret is never - // selected here; the label and type are the operator metadata that name the key. + // selected here; the label, type and auth mode are the operator metadata that name the key. String sql = """ SELECT r.status, r.pushed_ref, r.blocked_changes, r.failure_cause, r.failure_detail, r.unit_id, r.pr_url, r.pr_error, r.agent_started_at, r.kind, r.harness, r.model, r.provider_type, r.workspace, r.slug, r.subject, r.attempt, r.base_branch, r.base_commit, r.branch, r.pushed_as, r.review_id, r.finding_ref, r.task_summary, r.started_at, r.ended_at, r.work_item_id, r.checkpoint_head, r.work_ready_at, r.active_wall_seconds, - h.label AS credential_label, h.type AS credential_type + h.label AS credential_label, h.type AS credential_type, h.auth_mode AS credential_auth_mode FROM factory_run r LEFT JOIN harness_credential h ON h.id = r.harness_credential_id WHERE r.run_id = ? @@ -1051,7 +1098,7 @@ private static RunView readView(String runId, ResultSet rs, RunSpend spend) thro rs.getString("unit_id"), rs.getString("pr_url"), rs.getString("pr_error"), instant(rs, "agent_started_at"), rs.getString("kind"), rs.getString("harness"), rs.getString("model"), rs.getString("credential_label"), rs.getString("credential_type"), - rs.getString("provider_type"), rs.getString("workspace"), + rs.getString("credential_auth_mode"), rs.getString("provider_type"), rs.getString("workspace"), rs.getString("slug"), rs.getString("subject"), rs.getInt("attempt"), rs.getString("base_branch"), rs.getString("base_commit"), rs.getString("branch"), rs.getString("pushed_as"), rs.getString("review_id"), rs.getString("finding_ref"), diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java index 50dfd33c..0095b14c 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java @@ -15,6 +15,7 @@ import java.time.Instant; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.UUID; /** @@ -163,12 +164,11 @@ public Selection select() { WHERE id = ( SELECT id FROM harness_credential -- API keys only, and this is a CONTAINMENT boundary rather than a filter. A - -- subscription row holds the whole sign-in file; the harness arm would feed it - -- to --with-api-key, and the agent container -- which runs untrusted ticket - -- text at full shell access -- could then read it, refresh credential and all. - -- Until selection, injection and charging exist for a subscription, no run may - -- reach one. The rule lives here rather than in a caller's discipline because - -- three dispatch paths call this and each would have to remember. + -- subscription row holds the whole sign-in file, refresh token included; it is + -- reached only through selectSubscription, whose caller hands the agent a copy + -- with the refresh token removed. The rule lives here rather than in + -- a caller's discipline because three dispatch paths call this and each would + -- have to remember. WHERE enabled AND auth_mode = 'API_KEY' AND rejected_at IS NULL @@ -205,6 +205,77 @@ public Selection select() { return whyNothingIsAvailable(); } + /** + * Picks a signed-in seat of this harness and hands out its sign-in file (M3.5 part F). + * + *

Shared like an API key: several builds may use one seat at once, and the vendor's own limits + * decide how much they get. An exclusive per-run lease was built and reviewed, and every round found + * another race between workers; the reason for it — two agents refreshing one sign-in and logging each + * other out — is gone, because an agent's copy carries no refresh token (operator decision, + * 2026-09-26). Rotation is the same least-recently-used order the key pool uses. + * + * @return the member with its sign-in file as {@code apiKey}, or empty when no identified seat is + * enabled, unrefused and not resting. The caller reports that as one refusal. + */ + public Optional selectSubscription(String harness) { + String sql = """ + UPDATE harness_credential + SET last_used_at = now(), updated_at = now() + WHERE id = ( + SELECT id FROM harness_credential + WHERE enabled + AND auth_mode = 'SUBSCRIPTION' + AND type = ? + AND rejected_at IS NULL + AND (rate_limited_until IS NULL OR rate_limited_until <= now()) + -- A seat whose account is unknown could be a second seat on one account. + AND account_ref IS NOT NULL + ORDER BY exhausted_at NULLS FIRST, last_used_at NULLS FIRST + -- FOR UPDATE, not SKIP LOCKED: a seat is shared, so a dispatch picking it at the + -- same moment is no reason to refuse this one — it waits a moment instead. The lock + -- is what makes the wait re-check the row: a seat switched off meanwhile is not + -- returned (review of PR #178). + LIMIT 1 + FOR UPDATE) + RETURNING id, label, type, base_url, api_key + """; + try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(sql)) { + ps.setString(1, harness); + try (ResultSet rs = ps.executeQuery()) { + return rs.next() ? Optional.of(decrypt(rs.getObject("id", UUID.class), rs)) : Optional.empty(); + } + } catch (SQLException e) { + throw new IllegalStateException("The harness credential pool could not be read", e); + } + } + + /** How a member pays: {@code API_KEY} or {@code SUBSCRIPTION}. Empty for an unknown id. */ + public Optional authModeOf(UUID id) { + try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement( + "SELECT auth_mode FROM harness_credential WHERE id = ?")) { + ps.setObject(1, id); + try (ResultSet rs = ps.executeQuery()) { + return rs.next() ? Optional.of(rs.getString(1)) : Optional.empty(); + } + } catch (SQLException e) { + throw new IllegalStateException("The harness credential pool could not be read", e); + } + } + + /** Whether any enabled, unrejected seat is signed in for this harness — the question a build setup asks. */ + public boolean hasSubscription(String harness) { + try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(""" + SELECT 1 FROM harness_credential + WHERE enabled AND auth_mode = 'SUBSCRIPTION' AND type = ? AND rejected_at IS NULL + AND account_ref IS NOT NULL LIMIT 1 + """)) { + ps.setString(1, harness); + try (ResultSet rs = ps.executeQuery()) { return rs.next(); } + } catch (SQLException e) { + throw new IllegalStateException("The harness credential pool could not be read", e); + } + } + private PoolMember decrypt(UUID id, ResultSet rs) throws SQLException { return new PoolMember(id, rs.getString("label"), rs.getString("type"), rs.getString("base_url"), encryption.decryptString(rs.getString("api_key"), aad(id))); @@ -301,19 +372,18 @@ public boolean clearRejection(UUID id) { /** * What the settings surface shows. Never carries the key. * - * @param authMode {@code API_KEY} or {@code SUBSCRIPTION}. On the view because the screen must be - * able to say that a subscription cannot pay for a run yet: it is deliberately unreachable by - * the selector, and a row rendered as plain "Available" told the operator the opposite. + * @param authMode {@code API_KEY} or {@code SUBSCRIPTION}, so the screen can say which kind a row is. + * @param identified false for a seat whose account is unknown: it is never used until signed in again. */ public record MemberView(UUID id, String label, String type, String baseUrl, boolean enabled, Instant rateLimitedUntil, Instant rejectedAt, Instant lastUsedAt, - String authMode) { + String authMode, boolean identified) { } public List list() { String sql = """ SELECT id, label, type, base_url, enabled, rate_limited_until, rejected_at, last_used_at, - auth_mode + auth_mode, auth_mode <> 'SUBSCRIPTION' OR account_ref IS NOT NULL AS identified FROM harness_credential ORDER BY label """; List members = new ArrayList<>(); @@ -323,7 +393,7 @@ public List list() { members.add(new MemberView(rs.getObject("id", UUID.class), rs.getString("label"), rs.getString("type"), rs.getString("base_url"), rs.getBoolean("enabled"), instant(rs, "rate_limited_until"), instant(rs, "rejected_at"), - instant(rs, "last_used_at"), rs.getString("auth_mode"))); + instant(rs, "last_used_at"), rs.getString("auth_mode"), rs.getBoolean("identified"))); } return members; } catch (SQLException e) { @@ -358,7 +428,7 @@ INSERT INTO harness_credential (id, label, type, base_url, api_key) ps.setString(4, baseUrl); ps.setString(5, encryption.encryptString(apiKey, aad(id))); ps.executeUpdate(); - return new MemberView(id, label, type, baseUrl, true, null, null, null, "API_KEY"); + return new MemberView(id, label, type, baseUrl, true, null, null, null, "API_KEY", true); } catch (SQLException e) { if ("23505".equals(e.getSQLState())) { throw new DuplicateLabelException(label, e); @@ -394,20 +464,134 @@ public boolean hasLabel(Connection c, String label) throws SQLException { * @return the new member's id */ public UUID addSubscription(Connection c, String label, String type, String body) throws SQLException { + String account = SignInFiles.accountOf(body) + .orElseThrow(() -> new IllegalArgumentException("subscription_unidentified")); UUID id = UUID.randomUUID(); try (PreparedStatement ps = c.prepareStatement(""" - INSERT INTO harness_credential (id, label, type, base_url, api_key, auth_mode) - VALUES (?, ?, ?, '', ?, 'SUBSCRIPTION') + INSERT INTO harness_credential (id, label, type, base_url, api_key, auth_mode, account_ref) + VALUES (?, ?, ?, '', ?, 'SUBSCRIPTION', ?) """)) { ps.setObject(1, id); ps.setString(2, label); ps.setString(3, type); ps.setString(4, encryption.encryptString(body, aad(id))); + ps.setString(5, account); ps.executeUpdate(); } return id; } + /** The enabled seat already signed in to this account, locked for the caller's transaction. */ + public Optional seatFor(Connection c, String type, String account) throws SQLException { + try (PreparedStatement ps = c.prepareStatement(""" + SELECT id FROM harness_credential + WHERE auth_mode = 'SUBSCRIPTION' AND enabled AND type = ? AND account_ref = ? + FOR UPDATE + """)) { + ps.setString(1, type); + ps.setString(2, account); + try (ResultSet rs = ps.executeQuery()) { + return rs.next() ? Optional.of(rs.getObject("id", UUID.class)) : Optional.empty(); + } + } + } + + /** + * A new sign-in to an account that already has a seat replaces that seat's file rather than adding a + * second seat — which is also how a seat whose access token expired is signed in again. A refusal the + * old file earned is cleared: the new file has not been refused. A build already running keeps its + * own copy. + */ + public void replaceSubscription(Connection c, UUID id, String body) throws SQLException { + try (PreparedStatement ps = c.prepareStatement(""" + UPDATE harness_credential SET api_key = ?, rejected_at = NULL, updated_at = now() WHERE id = ? + """)) { + ps.setString(1, encryption.encryptString(body, aad(id))); + ps.setObject(2, id); + ps.executeUpdate(); + } + } + + /** + * Fills the account of every seat stored before accounts were recorded, from its own file. The newest + * seat of an account keeps it; older seats of the same account are switched off, because one account + * has one seat. A file that names no account, or cannot be read, stays unidentified and is never used. + * + *

One transaction under an advisory lock, taken BEFORE the unidentified seats are read: two + * orchestrators starting together would otherwise both read the same seat, and the second would find + * the first's work and switch that very seat off as its own duplicate (review of PR #178). Under the + * lock the second reads after the first commits, and finds nothing left to identify. + * + * @return how many seats were switched off as duplicates + */ + public int identifySeats() { + try (Connection c = dataSource.getConnection()) { + c.setAutoCommit(false); + try { + int switchedOff = identifySeats(c); + c.commit(); + if (switchedOff > 0) { + LOG.warnf("%d subscription seat(s) were switched off: another seat is signed in to the same" + + " account", switchedOff); + } + return switchedOff; + } catch (SQLException | RuntimeException failure) { + c.rollback(); + throw failure; + } + } catch (SQLException e) { + throw new IllegalStateException("Subscription seats could not be identified", e); + } + } + + /** Any constant shared by every orchestrator; it names this one piece of startup work. */ + static final long IDENTIFY_LOCK = 0x5EA7_1D_E7L; + + private int identifySeats(Connection c) throws SQLException { + try (PreparedStatement lock = c.prepareStatement("SELECT pg_advisory_xact_lock(?)")) { + lock.setLong(1, IDENTIFY_LOCK); + lock.execute(); + } + int switchedOff = 0; + try (PreparedStatement ps = c.prepareStatement(""" + SELECT id, type, api_key FROM harness_credential + WHERE auth_mode = 'SUBSCRIPTION' AND account_ref IS NULL ORDER BY updated_at DESC + """); ResultSet rs = ps.executeQuery()) { + while (rs.next()) { + UUID id = rs.getObject("id", UUID.class); + Optional account; + try { + account = SignInFiles.accountOf(encryption.decryptString(rs.getString("api_key"), aad(id))); + } catch (RuntimeException unreadable) { + LOG.warnf("subscription seat %s could not be read to find its account; it stays unused", id); + continue; + } + if (account.isEmpty()) continue; + boolean duplicate = seatFor(c, rs.getString("type"), account.orElseThrow()).isPresent(); + try (PreparedStatement set = c.prepareStatement( + "UPDATE harness_credential SET account_ref = ?, enabled = enabled AND NOT ? WHERE id = ?")) { + set.setString(1, account.orElseThrow()); + set.setBoolean(2, duplicate); + set.setObject(3, id); + set.executeUpdate(); + } + if (duplicate) switchedOff++; + } + } + return switchedOff; + } + + void identifyOnStart(@jakarta.enterprise.event.Observes io.quarkus.runtime.StartupEvent started) { + identifySeats(); + } + + /** Raised when an enabled seat already uses this account, so the caller can answer 409. */ + public static class SeatTakenException extends IllegalStateException { + SeatTakenException(UUID id, Throwable cause) { + super("another enabled seat is signed in to the same account as " + id, cause); + } + } + /** * Take a member out of rotation, keeping the row. * @@ -425,8 +609,15 @@ public boolean remove(UUID id) { /** Bring a disabled member back, because disabling is not deletion and must not be one-way. */ public boolean enable(UUID id) { - return update("UPDATE harness_credential SET enabled = TRUE, updated_at = now()" - + " WHERE id = ? AND NOT enabled", id) == 1; + try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement( + "UPDATE harness_credential SET enabled = TRUE, updated_at = now() WHERE id = ? AND NOT enabled")) { + ps.setObject(1, id); + return ps.executeUpdate() == 1; + } catch (SQLException e) { + // One seat per account: switching a second seat of an account back on is refused by name. + if ("23505".equals(e.getSQLState())) throw new SeatTakenException(id, e); + throw new IllegalStateException("The harness credential " + id + " could not be updated", e); + } } private static Instant instant(ResultSet rs, String column) throws SQLException { diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java index 146e211d..d8a555bc 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java @@ -118,7 +118,14 @@ public Response disable(@PathParam("id") String id) { @Path("/{id}/enable") @Consumes(MediaType.WILDCARD) public Response enable(@PathParam("id") String id) { - if (!pool.enable(uuid(id))) { + boolean enabled; + try { + enabled = pool.enable(uuid(id)); + } catch (HarnessCredentialPool.SeatTakenException taken) { + throw new ClientErrorException(Response.status(Response.Status.CONFLICT) + .entity("subscription_account_taken").type(MediaType.TEXT_PLAIN).build()); + } + if (!enabled) { throw new NotFoundException("no disabled harness credential with id: " + id); } return Response.noContent().build(); diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessSignIns.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessSignIns.java index 9f1b3104..cde91cfc 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessSignIns.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessSignIns.java @@ -279,6 +279,24 @@ public void completed(HarnessSignInResult.Completed result) { fail(c, id, HarnessSignInResult.Failed.WRONG_MODE); return; } + // One seat per account. Signing in again to an account that has a seat replaces that + // seat's file: it is how an expired seat is renewed. A second seat would share the one + // subscription's limits while looking like more, and keep the older, expiring copy in use. + Optional account = SignInFiles.accountOf(body); + if (account.isEmpty()) { + fail(c, id, "subscription_unidentified"); + return; + } + Optional existing = pool.seatFor(c, pending.get().harness(), account.orElseThrow()); + if (existing.isPresent()) { + pool.replaceSubscription(c, existing.orElseThrow(), body); + try (PreparedStatement ps = c.prepareStatement(""" + UPDATE harness_sign_in SET state='COMPLETE', credential_id=?, updated_at=now() WHERE id=? + """)) { + ps.setObject(1, existing.orElseThrow()); ps.setObject(2, id); ps.executeUpdate(); + } + return; + } if (pool.hasLabel(c, pending.get().label())) { // Taken by an ordinary key while this person was approving. Refusing by name beats // letting the unique constraint throw, because that throw used to be swallowed: diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/RunCharges.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/RunCharges.java index 35e8d398..f0a74e11 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/RunCharges.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/RunCharges.java @@ -14,6 +14,7 @@ import org.jboss.logging.Logger; import java.util.List; +import java.util.Optional; import java.util.UUID; /** @@ -99,10 +100,16 @@ public void record(RunResult result) { // IllegalArgumentException out of valueOf — which would reach the messaging layer and // produce exactly the redelivery loop the comment below says this catch prevents. ModelUsage usage = RunTokenUsage.of(result, model, maxReportedTokens); - List lines = pricer.priceCall(model, usage); // Which key paid, read from the run's own row like the model beside it. Empty for a run // dispatched before the pool existed; the column is nullable for exactly that. - String credentialRef = runs.harnessCredentialOf(runId).map(UUID::toString).orElse(null); + Optional credential = runs.harnessCredentialOf(runId); + // Priced by how the run PAID, not by its model: a subscription run's tokens cost nothing per + // token, and the same model's API-key run must still be priced (M3.5 part F, design §5.7). + // Read from the run, not through its credential: a re-armed dispatch clears the credential. + boolean subscription = runs.paidByOf(runId) + .filter(dev.codespire.contract.work.PayWith.SUBSCRIPTION::equals).isPresent(); + List lines = subscription ? pricer.priceUnmetered(usage) : pricer.priceCall(model, usage); + String credentialRef = credential.map(UUID::toString).orElse(null); ledger.recordCharges(ChargeCall.forRun(runId, CallRefs.forRun(runId, AGENT_CALL), model, lines, credentialRef)); // The terminal-status push precedes charging. FIX runs and runs that delivered nothing diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/SignInFiles.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/SignInFiles.java new file mode 100644 index 00000000..e8a417f8 --- /dev/null +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/SignInFiles.java @@ -0,0 +1,71 @@ +package dev.codespire.orchestrator.factory; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import java.util.Iterator; +import java.util.Map; + +/** + * The copy of a stored sign-in that an agent may hold (M3.5 part F). + * + *

The agent runs untrusted ticket text at full shell access, so whatever it is given, it can read. An + * access token expires on its own; a refresh token does not, and an agent that reads one holds the seat + * until a person revokes it. So every refresh token in the file is EMPTIED before the file leaves the + * orchestrator — emptied rather than removed, because the vendor's CLI refuses a file whose + * {@code refresh_token} field is missing but accepts an empty one and runs on the access token (measured + * 2026-09-25, codex-cli 0.156.1: design §5.3). + */ +final class SignInFiles { + + private static final ObjectMapper JSON = new ObjectMapper(); + private static final String REFRESH_TOKEN = "refresh_token"; + + private SignInFiles() { + } + + /** + * @throws IllegalStateException named {@code subscription_unreadable} when the stored bytes are not a + * JSON object — never with the bytes in the message, which are a credential + */ + static String forAgent(String storedFile) { + try { + JsonNode file = JSON.readTree(storedFile); + if (!(file instanceof ObjectNode object)) throw new IllegalStateException("subscription_unreadable"); + emptyRefreshTokens(object); + return JSON.writeValueAsString(object); + } catch (JsonProcessingException unreadable) { + throw new IllegalStateException("subscription_unreadable"); + } + } + + /** + * The vendor account a sign-in belongs to: {@code tokens.account_id}, measured on codex-cli 0.156.1 + * (design §5.3). Empty when the file names none, which the caller refuses rather than guesses. + */ + static java.util.Optional accountOf(String storedFile) { + try { + String account = JSON.readTree(storedFile).path("tokens").path("account_id").asText(""); + return account.isBlank() ? java.util.Optional.empty() : java.util.Optional.of(account); + } catch (JsonProcessingException unreadable) { + return java.util.Optional.empty(); + } + } + + /** Every level, arrays included: the vendor may add a list of sessions, each with its own token. */ + private static void emptyRefreshTokens(JsonNode node) { + if (node instanceof ArrayNode array) { + array.forEach(SignInFiles::emptyRefreshTokens); + return; + } + if (!(node instanceof ObjectNode object)) return; + for (Iterator> fields = object.properties().iterator(); fields.hasNext(); ) { + Map.Entry field = fields.next(); + if (field.getKey().equals(REFRESH_TOKEN)) field.setValue(JSON.getNodeFactory().textNode("")); + else emptyRefreshTokens(field.getValue()); + } + } +} diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/WorkRunAssembly.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/WorkRunAssembly.java index 14ab1660..752cf5d1 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/WorkRunAssembly.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/WorkRunAssembly.java @@ -45,32 +45,52 @@ public Prepared assemble(Connection c,WorkSourceRegistry.Source source,WorkItemE ps.setObject(1,source.repositoryId());try(ResultSet rs=ps.executeQuery()) { if(!rs.next())throw new IllegalStateException("factory_account_unavailable"); } } var account=accounts.resolve(source.repositoryId()).orElseThrow(()->new IllegalStateException("factory_account_unavailable")); + // How the APPROVED preparation pays — not the build setup now, which may have changed since. + boolean subscription=dev.codespire.contract.work.PayWith.SUBSCRIPTION.equals(item.preparation().payWith()); // Every type the chosen harness can report must have a rate or a not-billed assertion. Asking // only about INPUT and OUTPUT is what let item 36 start, spend, report CACHED_INPUT and - // REASONING, and stop with a cost nobody could account for. - try { if(models.isDisabled(in.model()))throw new IllegalStateException("model_disabled"); } + // REASONING, and stop with a cost nobody could account for. A subscription is not priced per + // token, so the price list neither enables nor blocks it. + try { if(!subscription && models.isDisabled(in.model()))throw new IllegalStateException("model_disabled"); } catch(dev.codespire.orchestrator.llm.LlmModelRegistry.CatalogueUnavailable unreadable) { throw new IllegalStateException("catalogue_unavailable"); } // Approved against the image the harness ran then; it may run another by now (§6A.4b). var admission=catalogues.admit(in.harness(),in.model(),item.preparation().effort()); if(admission.refusal()!=null)throw new IllegalStateException(admission.refusal()); - var unpriced=pricer.unpricedTypes(in.model(),in.harness()); - if(!unpriced.isEmpty())throw new IllegalStateException("model_pricing_incomplete:" - +unpriced.stream().map(Enum::name).collect(java.util.stream.Collectors.joining(","))); + if(!subscription) { + var unpriced=pricer.unpricedTypes(in.model(),in.harness()); + if(!unpriced.isEmpty())throw new IllegalStateException("model_pricing_incomplete:" + +unpriced.stream().map(Enum::name).collect(java.util.stream.Collectors.joining(","))); + } + // Still consulted for a subscription run: it adds no money, but the deployment's own gate is the + // operator's policy, and part F does not quietly change it (design §5.7). if(spend.decide().refused())throw new IllegalStateException("deployment_spend_cap_reached"); - if(!(pool.select() instanceof HarnessCredentialPool.Selection.Chosen chosen))throw new IllegalStateException("harness_credential_unavailable"); String id=RunIds.of(source.scm(),in.workspace(),in.slug(),subject,1),branch=DispatchRequestParser.RUN_BRANCH_PREFIX+subject; long wall=Math.min(config.wallClockSeconds(),Math.subtractExact(item.policy().limits().maxWallClockSeconds(),item.progress().wallSeconds())); - var command=new RunCommand.ExecuteRun(id,source.repository(),FactoryCloneUrls.cloneUrl(source.scm(),account.baseUrl(),source.repository()), + HarnessCredentialPool.PoolMember member=subscription ? pickSeat(in.harness()) : pickKey(); + // The agent gets the sign-in with its refresh token emptied; the whole file stays here, encrypted. + String handedOver=subscription ? SignInFiles.forAgent(member.apiKey()) : member.apiKey(); + RunCommand.ExecuteRun command=new RunCommand.ExecuteRun(id,source.repository(),FactoryCloneUrls.cloneUrl(source.scm(),account.baseUrl(),source.repository()), // The image the checked list was read from, not the tag, which may name another by now. in.baseBranch(),in.baseCommit(),branch,in.prompt(),in.harness(),in.model(),admission.image(), item.policy().limits().protectedPaths().stream().sorted().toList(),wall, - credentials.packScm(id,account.botUsername(),account.secret()),credentials.packHarness(id,chosen.member().apiKey())) + credentials.packScm(id,account.botUsername(),account.secret()),credentials.packHarness(id,handedOver)) // The level the approved binding hashed, so the build runs at what was approved (M3.5 part M). .atEffort(item.preparation().effort()); + if(subscription)command=command.paidBySignIn(); var held=new RunCommand.ExecuteWorkRun(command,new dev.codespire.contract.work.WorkRunBinding( item.workItemId(),item.generation(),item.progress().attemptId(),item.preparation().binding())); - return new Prepared(held,new FactoryRunProjection.QueuedRun(id,in.harness(),in.model(),in.baseBranch(),in.baseCommit(),branch,account.botUsername(),chosen.member().id())); + var row=new FactoryRunProjection.QueuedRun(id,in.harness(),in.model(),in.baseBranch(),in.baseCommit(),branch,account.botUsername(),member.id()); + return new Prepared(held,subscription?row.paidBySubscription():row); + } + + private HarnessCredentialPool.PoolMember pickSeat(String harness) { + return pool.selectSubscription(harness).orElseThrow(()->new IllegalStateException("subscription_unavailable")); + } + + private HarnessCredentialPool.PoolMember pickKey() { + if(!(pool.select() instanceof HarnessCredentialPool.Selection.Chosen chosen))throw new IllegalStateException("harness_credential_unavailable"); + return chosen.member(); } } diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/llm/LlmModelPricer.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/llm/LlmModelPricer.java index 25b162f5..fc403c24 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/llm/LlmModelPricer.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/llm/LlmModelPricer.java @@ -75,6 +75,21 @@ public List priceCall(String model, ModelUsage usage) { return counts.stream().map(count -> line(pricing, count)).toList(); } + /** + * A call paid for by a subscription rather than per token (M3.5 part F): rate 0 and cost 0, with the + * real token counts, whatever the model's own pricing says. Priced by how the call PAID rather than by + * model, so one model can serve an API-key run and a subscription run in the same deployment. + * + *

Missing usage stays UNKNOWN, exactly as in {@link #priceCall}: a subscription makes a reported + * call cost zero; it does not make an unreported one free. + */ + public List priceUnmetered(ModelUsage usage) { + List counts = usage == null ? List.of() : usage.counts(); + if (counts.isEmpty()) return List.of(ChargeLine.unknown(TokenType.TOTAL, 0)); + if (!usage.reconciled()) return List.of(ChargeLine.unmetered(TokenType.TOTAL, usage.reportedTotal())); + return counts.stream().map(count -> ChargeLine.unmetered(count.type(), count.tokens())).toList(); + } + /** Whether a review may be started against this model: priceable, or explicitly unbilled. */ public boolean isPriceable(String model) { Pricing pricing = pricingFor(model); diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkPreparationSweep.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkPreparationSweep.java index 082198fd..69c5e849 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkPreparationSweep.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkPreparationSweep.java @@ -203,9 +203,12 @@ private Result attempt(String id, boolean again, String actor, if (!setup.set()) return refuse(id, generation, expectedRevision, "build_defaults_missing"); // What the dispatch will ask, asked before an approval is opened on it. Without this, disabling // a model after the setup was saved still produced a decision whose build was already refused. + boolean subscription = dev.codespire.contract.work.PayWith.SUBSCRIPTION.equals(setup.payWith()); try { - if (models.isDisabled(setup.model())) return refuse(id, generation, expectedRevision, "model_disabled"); - var unpriced = pricer.unpricedTypes(setup.model(), setup.harness()); + // A subscription is not priced per token, so the price list neither enables nor blocks it. + if (!subscription && models.isDisabled(setup.model())) return refuse(id, generation, expectedRevision, "model_disabled"); + var unpriced = subscription ? java.util.List.of() + : pricer.unpricedTypes(setup.model(), setup.harness()); if (!unpriced.isEmpty()) return refuse(id, generation, expectedRevision, "model_pricing_incomplete:" + unpriced.stream().map(Enum::name).collect(java.util.stream.Collectors.joining(","))); // The setup was checked against the image it was saved for; the harness may run another now. @@ -247,7 +250,7 @@ private Result attempt(String id, boolean again, String actor, WorkPreparation.Origin.STORED, planId), setup.baseBranch(), head, setup.harness(), setup.model(), actor == null ? "system:build-defaults@" + setup.revision() : actor, - WorkPreparation.EFFORT_BINDING, setup.effort()); + WorkPreparation.PAY_WITH_BINDING, setup.effort(), setup.payWith()); // The saved setup is compared again INSIDE the registration's own transaction: it was read // before a forge call this waited on, and a repository whose branch, harness or model changed diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkRunAssemblyRefusals.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkRunAssemblyRefusals.java index a3991151..72d77018 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkRunAssemblyRefusals.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/work/WorkRunAssemblyRefusals.java @@ -21,7 +21,8 @@ final class WorkRunAssemblyRefusals { private static final Set NAMED = Set.of( "factory_account_unavailable", "harness_credential_unavailable", "deployment_spend_cap_reached", "model_pricing_unavailable", "model_disabled", "catalogue_unavailable", - "model_not_run_by_harness", "effort_not_offered", "effort_unverifiable"); + "model_not_run_by_harness", "effort_not_offered", "effort_unverifiable", + "subscription_unavailable", "subscription_unreadable"); /** The one reason that carries a payload: "model_pricing_incomplete:CACHED_INPUT,REASONING". */ private static final String PRICING = "model_pricing_incomplete"; diff --git a/spire-orchestrator/src/main/resources/db/migration/V83__pay_with_subscription.sql b/spire-orchestrator/src/main/resources/db/migration/V83__pay_with_subscription.sql new file mode 100644 index 00000000..eac79eb6 --- /dev/null +++ b/spire-orchestrator/src/main/resources/db/migration/V83__pay_with_subscription.sql @@ -0,0 +1,7 @@ +-- Paying for a build with a Codex subscription (M3.5 part F). + +-- How a repository's builds pay. Every existing setup pays with an API key, because until now that was +-- the only way a build could. +ALTER TABLE repository_build_defaults ADD COLUMN pay_with VARCHAR(16) NOT NULL DEFAULT 'API_KEY'; +ALTER TABLE repository_build_defaults ADD CONSTRAINT repository_build_defaults_known_pay_with + CHECK (pay_with IN ('API_KEY', 'SUBSCRIPTION')); diff --git a/spire-orchestrator/src/main/resources/db/migration/V84__run_paid_by.sql b/spire-orchestrator/src/main/resources/db/migration/V84__run_paid_by.sql new file mode 100644 index 00000000..3b33b3a0 --- /dev/null +++ b/spire-orchestrator/src/main/resources/db/migration/V84__run_paid_by.sql @@ -0,0 +1,8 @@ +-- How a run paid, on the run itself (M3.5 part F, design section 5.7). +-- +-- Charging used to read it through harness_credential_id, but a re-armed dispatch NULLS that column on +-- purpose (FactoryRunProjection.queued explains why), so a subscription retry was priced as if an API +-- key had paid. How a run pays is fixed by its approval and does not change on a retry, so it lives +-- here, where a re-arm cannot erase it. Every existing run paid with an API key. +ALTER TABLE factory_run ADD COLUMN paid_by VARCHAR(16) NOT NULL DEFAULT 'API_KEY'; +ALTER TABLE factory_run ADD CONSTRAINT factory_run_paid_by_known CHECK (paid_by IN ('API_KEY', 'SUBSCRIPTION')); diff --git a/spire-orchestrator/src/main/resources/db/migration/V85__one_seat_per_account.sql b/spire-orchestrator/src/main/resources/db/migration/V85__one_seat_per_account.sql new file mode 100644 index 00000000..e43290d3 --- /dev/null +++ b/spire-orchestrator/src/main/resources/db/migration/V85__one_seat_per_account.sql @@ -0,0 +1,14 @@ +-- One signed-in seat per vendor account (M3.5 part F, review of PR #178). +-- +-- V77 left account_ref empty until a real sign-in had been measured. It has been (design section 5.3): +-- the file carries tokens.account_id. Two seats on one account would share one subscription's limits +-- while looking like two, and keep an older, expiring copy in use, so an enabled seat's account is +-- unique per harness. Signing in to the same account again +-- replaces that seat's file instead of adding a second seat. +-- +-- Existing seats have no account_ref yet. The orchestrator fills it at startup from each stored file, +-- switching off all but the newest seat of an account; a seat whose file names no account is never +-- used and shows "Sign in again". +CREATE UNIQUE INDEX harness_credential_one_seat_per_account + ON harness_credential (type, account_ref) + WHERE auth_mode = 'SUBSCRIPTION' AND enabled AND account_ref IS NOT NULL; diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/BuildDefaultsTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/BuildDefaultsTest.java index b3e9629e..3c4f95d9 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/BuildDefaultsTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/BuildDefaultsTest.java @@ -156,6 +156,64 @@ void noThinkingLevelMeansTheModelsOwnDefault() { assertNull(defaults.save(repository, input("main", "codex", model, 0), "TEST-operator").effort()); } + @Inject HarnessCredentialPool pool; + + /** A signed-in codex seat, switched off by the caller when done: a seat a run points at cannot be deleted. */ + private UUID seat() throws java.sql.SQLException { + try (var c = dataSource.getConnection()) { + return pool.addSubscription(c, "TEST-build-seat-" + UUID.randomUUID(), "codex", "{\"auth_mode\":\"TEST\",\"tokens\":{\"account_id\":\"TEST-account-" + UUID.randomUUID() + "\"}}"); + } + } + + /** + * A subscription is not priced per token, so a model with no price at all may be saved — the harness's + * own list is what says it can run (M3.5 part F). + */ + @Test + void aSubscriptionSetupNeedsASeatButNoPrice() throws java.sql.SQLException { + String unpriced = "TEST-unpriced-" + UUID.randomUUID().toString().substring(0, 8); + UUID seat = seat(); + try { + var saved = defaults.save(repository, new BuildDefaults.Input(0, "main", "codex", unpriced, null, "SUBSCRIPTION"), + "TEST-operator"); + + assertEquals("SUBSCRIPTION", saved.payWith()); + assertEquals("SUBSCRIPTION", defaults.get(repository).payWith()); + } finally { + pool.remove(seat); + } + } + + /** Paying with a subscription nobody has signed in would refuse every build it prepares. */ + @Test + void aSubscriptionSetupIsRefusedWhileNoSeatIsSignedIn() { + assumeNoCodexSeat(); + assertEquals("subscription_not_signed_in", refusal(new BuildDefaults.Input(0, "main", "codex", model, null, "SUBSCRIPTION"))); + } + + /** Other suites switch their seats off; one left on would make the refusal above untestable, not wrong. */ + private void assumeNoCodexSeat() { + org.junit.jupiter.api.Assumptions.assumeFalse(pool.hasSubscription("codex"), "a codex seat is signed in on this database"); + } + + /** The same model paid with an API key is still refused without its prices. */ + @Test + void theSameModelPaidWithAKeyStillNeedsItsPrices() { + String unpriced = "TEST-unpriced-" + UUID.randomUUID().toString().substring(0, 8); + + assertEquals("model_unknown", refusal(new BuildDefaults.Input(0, "main", "codex", unpriced, null, "API_KEY"))); + } + + @Test + void anUnknownWayToPayIsRefused() { + assertEquals("pay_with_invalid", refusal(new BuildDefaults.Input(0, "main", "codex", model, null, "TEST-free"))); + } + + @Test + void aSetupThatSaysNothingPaysWithAKey() { + assertEquals("API_KEY", defaults.save(repository, input("main", "codex", model, 0), "TEST-operator").payWith()); + } + private String refusal(BuildDefaults.Input input) { return assertThrows(BuildDefaults.Refused.class, () -> defaults.save(repository, input, "TEST-operator")).reason(); } diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/FactoryRunProjectionTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/FactoryRunProjectionTest.java index 3d220cb3..13235628 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/FactoryRunProjectionTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/FactoryRunProjectionTest.java @@ -373,6 +373,56 @@ void aRunNamesTheCredentialThatPaidForIt() { FactoryRunProjection.RunView view = projection.find(runId).orElseThrow(); assertEquals(member.label(), view.credentialLabel()); assertEquals("openai", view.credentialType()); + assertEquals("API_KEY", view.credentialAuthMode()); + } + + /** + * A re-arm clears the credential on purpose, but not how the run paid: charging reads that, so a + * subscription retry is never priced as an API-key run (review of PR #178). + */ + @Test + void aReArmKeepsHowTheRunPaid() { + String runId = "run::github:TEST-acme/app:subject-" + UUID.randomUUID() + ":1"; + var row = new FactoryRunProjection.QueuedRun(runId, "codex", "gpt-5.6", "main", "abc1234", + "spire/TEST-paid", null, null).paidBySubscription(); + assertTrue(projection.queued(row, "TEST summary", null)); + projection.dispatchFailed(runId, "TEST: the broker did not answer"); + + assertTrue(projection.queued(row, "TEST summary", null), "the same request re-arms"); + + assertEquals(Optional.of("SUBSCRIPTION"), projection.paidByOf(runId)); + } + + /** A retry that pays another way is a different request, refused like any other differing component. */ + @Test + void aReArmThatPaysAnotherWayIsRefused() { + String runId = "run::github:TEST-acme/app:subject-" + UUID.randomUUID() + ":1"; + var row = new FactoryRunProjection.QueuedRun(runId, "codex", "gpt-5.6", "main", "abc1234", + "spire/TEST-paid", null, null); + assertTrue(projection.queued(row, "TEST summary", null)); + projection.dispatchFailed(runId, "TEST: the broker did not answer"); + + assertFalse(projection.queued(row.paidBySubscription(), "TEST summary", null)); + assertEquals(Optional.of("API_KEY"), projection.paidByOf(runId)); + } + + /** A run a signed-in seat paid for says so; the screen must not call it an API key billed per token. */ + @Test + void aRunPaidByASubscriptionSaysSo() throws SQLException { + UUID seat; + try (Connection c = dataSource.getConnection()) { + seat = pool.addSubscription(c, "TEST-frp-seat-" + UUID.randomUUID(), "codex", "{\"auth_mode\":\"TEST\",\"tokens\":{\"account_id\":\"TEST-account-" + UUID.randomUUID() + "\"}}"); + } + try { + String runId = "run::github:TEST-acme/app:subject-" + UUID.randomUUID() + ":1"; + assertTrue(queueWith(runId, seat)); + + FactoryRunProjection.RunView view = projection.find(runId).orElseThrow(); + assertEquals("SUBSCRIPTION", view.credentialAuthMode()); + assertEquals("codex", view.credentialType()); + } finally { + pool.remove(seat); + } } /** A run dispatched with no pool member names none, rather than inventing one. */ @@ -381,6 +431,7 @@ void aRunWithNoCredentialNamesNone() { FactoryRunProjection.RunView view = projection.find(queuedRun()).orElseThrow(); assertNull(view.credentialLabel()); assertNull(view.credentialType()); + assertNull(view.credentialAuthMode()); } private String queuedRun() { diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSignInsTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSignInsTest.java index e4cbedad..ab1c6ea7 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSignInsTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSignInsTest.java @@ -37,8 +37,11 @@ class HarnessSignInsTest { private static final String HARNESS = "codex"; /** A subscription-shaped file. The mode is NOT the measured API-key one, which is all that matters. */ - private static final String SUBSCRIPTION_FILE = - "{\"auth_mode\":\"TEST-chatgpt\",\"tokens\":{\"access\":\"TEST-not-a-real-token\"}}"; + private static final String SUBSCRIPTION_FILE = subscriptionFile("TEST-account-" + UUID.randomUUID(), "TEST-not-a-real-token"); + + private static String subscriptionFile(String account, String token) { + return "{\"auth_mode\":\"TEST-chatgpt\",\"tokens\":{\"access\":\"" + token + "\",\"account_id\":\"" + account + "\"}}"; + } /** The measured API-key file, with an obviously fake key. */ private static final String API_KEY_FILE = "{\"auth_mode\":\"apikey\",\"OPENAI_API_KEY\":\"sk-TEST-not-real\"}"; @@ -119,6 +122,46 @@ void anApprovedSignInBecomesASubscriptionMemberUnderTheNameTheOperatorGaveIt() { assertEquals("", member.baseUrl()); } + /** + * Signing in again to an account that has a seat renews that seat's file instead of adding a second + * seat — the way an expired seat is renewed (review of PR #178). + */ + @Test + void aSecondSignInToTheSameAccountRenewsItsSeat() throws SQLException { + String account = "TEST-account-" + UUID.randomUUID(); + HarnessSignIns.View first = start("TEST-seat-renew-first"); + signIns.completed(completion(first.id(), subscriptionFile(account, "TEST-old-token"))); + UUID seat = signIns.get(first.id()).orElseThrow().credentialId(); + + HarnessSignIns.View second = start("TEST-seat-renew-second"); + signIns.completed(completion(second.id(), subscriptionFile(account, "TEST-new-token"))); + + HarnessSignIns.View renewed = signIns.get(second.id()).orElseThrow(); + assertEquals("COMPLETE", renewed.state()); + assertEquals(seat, renewed.credentialId(), "the same seat, not a second one"); + assertTrue(pool.list().stream().noneMatch(member -> member.label().equals("TEST-seat-renew-second"))); + try (Connection c = dataSource.getConnection(); + java.sql.PreparedStatement ps = c.prepareStatement("SELECT api_key FROM harness_credential WHERE id = ?")) { + ps.setObject(1, seat); + try (java.sql.ResultSet rs = ps.executeQuery()) { + assertTrue(rs.next()); + assertTrue(encryption.decryptString(rs.getString(1), "harness-credential:" + seat).contains("TEST-new-token")); + } + } + } + + @Test + void aSignInThatNamesNoAccountIsRefused() { + HarnessSignIns.View view = start("TEST-seat-no-account"); + + signIns.completed(completion(view.id(), "{\"auth_mode\":\"TEST-chatgpt\",\"tokens\":{\"access\":\"TEST-token\"}}")); + + HarnessSignIns.View after = signIns.get(view.id()).orElseThrow(); + assertEquals("FAILED", after.state()); + assertEquals("subscription_unidentified", after.reason()); + assertTrue(pool.list().stream().noneMatch(member -> member.label().equals("TEST-seat-no-account"))); + } + /** * A sign-in that came back as an API key is refused rather than stored as a subscription. * diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSubscriptionSeatsTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSubscriptionSeatsTest.java new file mode 100644 index 00000000..d17874a7 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessSubscriptionSeatsTest.java @@ -0,0 +1,225 @@ +package dev.codespire.orchestrator.factory; + +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Signed-in seats are shared by builds, one seat per account (M3.5 part F; operator decision 2026-09-26). + * + *

Uses its own harness name so no other suite's seats are in range, and switches every seat it made + * off afterwards: a seat a finished run points at cannot be deleted, by design. + */ +@QuarkusTest +class HarnessSubscriptionSeatsTest { + + @Inject HarnessCredentialPool pool; + @Inject DataSource dataSource; + + private static final String HARNESS = "TEST-seat-harness"; + private final List seats = new ArrayList<>(); + + /** A sign-in file of its own account: one account has one enabled seat. */ + private static String file(String account) { + return "{\"auth_mode\":\"TEST\",\"tokens\":{\"refresh_token\":\"TEST-refresh\",\"account_id\":\"" + account + "\"}}"; + } + + private static String anAccount() { + return "TEST-account-" + UUID.randomUUID(); + } + + @AfterEach + void switchSeatsOff() { + seats.forEach(pool::remove); + } + + private UUID seat(String label) throws SQLException { + return seatOn(label, anAccount()); + } + + private UUID seatOn(String label, String account) throws SQLException { + try (Connection c = dataSource.getConnection()) { + UUID id = pool.addSubscription(c, label + "-" + UUID.randomUUID(), HARNESS, file(account)); + seats.add(id); + return id; + } + } + + private void sql(String statement, Object... params) throws SQLException { + try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(statement)) { + for (int i = 0; i < params.length; i++) ps.setObject(i + 1, params[i]); + ps.executeUpdate(); + } + } + + private HarnessCredentialPool.MemberView view(UUID seat) { + return pool.list().stream().filter(member -> member.id().equals(seat)).findFirst().orElseThrow(); + } + + /** Shared like a key: an agent's copy has no refresh token, so two agents cannot log each other out. */ + @Test + void aSeatServesSeveralBuildsAtOnce() throws SQLException { + UUID seat = seat("TEST-seat-shared"); + + assertEquals(seat, pool.selectSubscription(HARNESS).orElseThrow().id()); + assertEquals(seat, pool.selectSubscription(HARNESS).orElseThrow().id()); + } + + @Test + void theSelectedSeatCarriesItsWholeStoredFile() throws SQLException { + seat("TEST-seat-file"); + + assertTrue(pool.selectSubscription(HARNESS).orElseThrow().apiKey().contains("TEST-refresh"), + "the whole stored file; the dispatch empties its refresh token"); + } + + /** + * A dispatch picking the only seat at the same moment as another waits for it rather than being + * refused: the seat is shared (review of PR #178). + */ + @Test + void aSeatBeingPickedByAnotherDispatchIsWaitedForNotSkipped() throws Exception { + UUID seat = seat("TEST-seat-contended"); + try (Connection other = dataSource.getConnection()) { + other.setAutoCommit(false); + try (PreparedStatement lock = other.prepareStatement("SELECT id FROM harness_credential WHERE id = ? FOR UPDATE")) { + lock.setObject(1, seat); + lock.executeQuery().close(); + } + CompletableFuture> picking = + CompletableFuture.supplyAsync(() -> pool.selectSubscription(HARNESS)); + + assertThrows(TimeoutException.class, () -> picking.get(1, TimeUnit.SECONDS), "it waits, it does not skip"); + other.commit(); + assertEquals(seat, picking.get(30, TimeUnit.SECONDS).orElseThrow().id()); + } + } + + /** A seat switched off while a pick waits for it is not handed out when the wait ends. */ + @Test + void aSeatSwitchedOffDuringAWaitIsNotHandedOut() throws Exception { + UUID seat = seat("TEST-seat-switched-off"); + try (Connection other = dataSource.getConnection()) { + other.setAutoCommit(false); + try (PreparedStatement off = other.prepareStatement("UPDATE harness_credential SET enabled = FALSE WHERE id = ?")) { + off.setObject(1, seat); + off.executeUpdate(); + } + CompletableFuture> picking = + CompletableFuture.supplyAsync(() -> pool.selectSubscription(HARNESS)); + + assertThrows(TimeoutException.class, () -> picking.get(1, TimeUnit.SECONDS)); + other.commit(); + assertTrue(picking.get(30, TimeUnit.SECONDS).isEmpty(), "a switched-off seat pays for nothing"); + } + } + + /** Two seats take turns, least recently used first, like the key pool. */ + @Test + void seatsTakeTurns() throws SQLException { + UUID first = seat("TEST-seat-first"); + UUID second = seat("TEST-seat-second"); + + UUID a = pool.selectSubscription(HARNESS).orElseThrow().id(); + UUID b = pool.selectSubscription(HARNESS).orElseThrow().id(); + + assertNotEquals(a, b); + assertEquals(Set.of(first, second), Set.of(a, b)); + } + + /** A seat whose account is unknown could be a second seat on one account, so it is never used. */ + @Test + void aSeatWithNoKnownAccountIsNeverUsed() throws SQLException { + UUID seat = seat("TEST-seat-unidentified"); + sql("UPDATE harness_credential SET account_ref = NULL WHERE id = ?", seat); + + assertTrue(pool.selectSubscription(HARNESS).isEmpty()); + assertFalse(pool.hasSubscription(HARNESS), "a setup cannot be saved on a seat no build may use"); + assertFalse(view(seat).identified()); + } + + /** A sign-in that names no account is not stored as a seat at all. */ + @Test + void aSignInWithNoAccountIsRefused() { + IllegalArgumentException refused = assertThrows(IllegalArgumentException.class, () -> { + try (Connection c = dataSource.getConnection()) { + pool.addSubscription(c, "TEST-seat-no-account-" + UUID.randomUUID(), HARNESS, "{\"auth_mode\":\"TEST\"}"); + } + }); + assertEquals("subscription_unidentified", refused.getMessage()); + } + + /** + * Seats stored before accounts were recorded are identified from their own files. The newest seat of + * an account keeps it; an older one is switched off, and cannot be switched back on beside it. + */ + @Test + void identifyingOldSeatsKeepsTheNewestSeatOfAnAccount() throws SQLException { + String account = anAccount(); + UUID older = seatOn("TEST-seat-older", account); + sql("UPDATE harness_credential SET account_ref = NULL, updated_at = now() - interval '2 days' WHERE id = ?", older); + UUID newer = seatOn("TEST-seat-newer", account); + sql("UPDATE harness_credential SET account_ref = NULL, updated_at = now() - interval '1 day' WHERE id = ?", newer); + + assertTrue(pool.identifySeats() >= 1); + + assertTrue(view(newer).enabled() && view(newer).identified()); + assertFalse(view(older).enabled(), "one account has one seat"); + assertThrows(HarnessCredentialPool.SeatTakenException.class, () -> pool.enable(older)); + } + + /** + * Two orchestrators starting together identify one after the other, never side by side: side by side, + * the second would find the first's work and switch the surviving seat off (review of PR #178). + */ + @Test + void identificationWaitsForAnotherOrchestratorDoingTheSame() throws Exception { + try (Connection other = dataSource.getConnection()) { + other.setAutoCommit(false); + try (PreparedStatement lock = other.prepareStatement("SELECT pg_advisory_xact_lock(?)")) { + lock.setLong(1, HarnessCredentialPool.IDENTIFY_LOCK); + lock.execute(); + } + CompletableFuture identifying = CompletableFuture.supplyAsync(pool::identifySeats); + + assertThrows(TimeoutException.class, () -> identifying.get(1, TimeUnit.SECONDS), + "it must wait while another orchestrator holds the lock"); + other.commit(); + assertNotNull(identifying.get(30, TimeUnit.SECONDS)); + } + } + + /** API-key selection never reaches a seat: that path hands its credential out whole, to anyone. */ + @Test + void theKeySelectorNeverHandsOutASeat() throws SQLException { + UUID seat = seat("TEST-seat-containment"); + + HarnessCredentialPool.Selection selection = pool.select(); + + assertFalse(selection instanceof HarnessCredentialPool.Selection.Chosen chosen && chosen.member().id().equals(seat)); + } + + @Test + void aSeatOfAnotherHarnessIsNotOffered() throws SQLException { + seat("TEST-seat-other"); + + assertTrue(pool.selectSubscription("TEST-no-such-harness").isEmpty()); + assertTrue(pool.hasSubscription(HARNESS)); + assertFalse(pool.hasSubscription("TEST-no-such-harness")); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunChargesTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunChargesTest.java index c1940071..5cb5b31d 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunChargesTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/RunChargesTest.java @@ -75,6 +75,14 @@ public Optional modelOf(String runId) { public Optional harnessCredentialOf(String runId) { return Optional.ofNullable(credential); } + + String paidBy = "API_KEY"; + + /** How the run paid, answered here for the same reason as the credential above. */ + @Override + public Optional paidByOf(String runId) { + return Optional.ofNullable(paidBy); + } } /** Prices everything at a flat metered rate, so a line's presence is the thing under test. */ @@ -110,6 +118,35 @@ private static RunResult.RunFinished finished(String runId, Map us return new RunResult.RunFinished(runId, "refs/heads/spire/s", List.of(), List.of(), usage, false); } + /** + * A run paid for by a subscription costs nothing per token, whatever its model's rates — and keeps its + * real token counts, so what a subscription run used is still visible (M3.5 part F, design §5.7). + */ + @Test + void aSubscriptionRunIsChargedNothingPerTokenWithItsRealCounts() { + // No credential at all: a re-armed dispatch clears it, and the run must still be charged as paid. + runs.credential = null; + runs.paidBy = "SUBSCRIPTION"; + + charges.record(finished(RUN_ID, Map.of("INPUT", 1200L, "OUTPUT", 340L))); + + var lines = ledger.calls.getFirst().lines(); + assertEquals(2, lines.size()); + assertTrue(lines.stream().allMatch(line -> line.mode() == dev.codespire.orchestrator.llm.PricingMode.UNMETERED && Long.valueOf(0L).equals(line.costMillicents())), lines.toString()); + assertEquals(1540, lines.stream().mapToInt(ChargeLine::tokens).sum()); + } + + /** The same model paid with an API key is still priced: pricing follows the payment, not the model. */ + @Test + void anApiKeyRunOfTheSameModelIsStillPriced() { + runs.credential = java.util.UUID.fromString("00000000-0000-4000-8000-00000000c0de"); + runs.paidBy = "API_KEY"; + + charges.record(finished(RUN_ID, Map.of("INPUT", 1200L))); + + assertEquals(dev.codespire.orchestrator.llm.PricingMode.METERED, ledger.calls.getFirst().lines().getFirst().mode()); + } + @Test void aFinishedRunWritesOneChargeLinePerTokenType() { charges.record(finished(RUN_ID, Map.of("INPUT", 1200L, "OUTPUT", 340L))); diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/SignInFilesTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/SignInFilesTest.java new file mode 100644 index 00000000..b557328c --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/SignInFilesTest.java @@ -0,0 +1,55 @@ +package dev.codespire.orchestrator.factory; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The agent may hold the short-lived half of a sign-in, never the refresh token (M3.5 part F, design §5.6). + * Placeholder values: this is about what reaches the agent. + */ +class SignInFilesTest { + + private static final ObjectMapper JSON = new ObjectMapper(); + private static final String STORED = "{\"auth_mode\":\"chatgpt\",\"tokens\":{\"access_token\":\"TEST-access\"," + + "\"id_token\":\"TEST-id\",\"refresh_token\":\"TEST-refresh-must-not-leave\",\"account_id\":\"TEST-account\"}," + + "\"last_refresh\":\"1970-01-01T00:00:00Z\"}"; + + @Test + void theRefreshTokenIsEmptiedAndEverythingElseKept() throws Exception { + String forAgent = SignInFiles.forAgent(STORED); + JsonNode file = JSON.readTree(forAgent); + + assertFalse(forAgent.contains("TEST-refresh-must-not-leave")); + // Emptied, not removed: the vendor's CLI refuses a file whose refresh_token field is missing. + assertTrue(file.path("tokens").has("refresh_token")); + assertEquals("", file.path("tokens").path("refresh_token").asText()); + assertEquals("TEST-access", file.path("tokens").path("access_token").asText()); + assertEquals("TEST-id", file.path("tokens").path("id_token").asText()); + assertEquals("chatgpt", file.path("auth_mode").asText()); + } + + /** A token inside a list is still a refresh token (review of PR #178). */ + @Test + void aRefreshTokenInsideAListIsEmptiedToo() throws Exception { + String stored = "{\"sessions\":[{\"refresh_token\":\"TEST-listed-refresh\",\"access_token\":\"TEST-listed-access\"}]}"; + + String forAgent = SignInFiles.forAgent(stored); + + assertFalse(forAgent.contains("TEST-listed-refresh")); + assertEquals("", JSON.readTree(forAgent).path("sessions").path(0).path("refresh_token").asText()); + assertTrue(forAgent.contains("TEST-listed-access")); + } + + @Test + void storedBytesThatAreNotAJsonObjectAreRefusedWithoutQuotingThem() { + IllegalStateException refused = assertThrows(IllegalStateException.class, + () -> SignInFiles.forAgent("TEST-not-a-sign-in")); + assertEquals("subscription_unreadable", refused.getMessage()); + } +} diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/WorkRunChargeProof.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/WorkRunChargeProof.java index dfd38049..d8c511df 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/WorkRunChargeProof.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/WorkRunChargeProof.java @@ -42,6 +42,8 @@ public static void main(String[] args) throws Exception { charges.runs=new FactoryRunProjection(){ @Override public Optional modelOf(String id){return Optional.of("TEST-metered-model");} @Override public Optional harnessCredentialOf(String id){return Optional.empty();} + // Overridden for the trap RunChargesTest names: the real one reads a row this proof never wrote. + @Override public Optional paidByOf(String id){return Optional.of("API_KEY");} @Override protected void push(String id){ /* TEST: no dashboard websocket */ } }; charges.pricer=new LlmModelPricer(){@Override public List priceCall(String model,ModelUsage usage){ diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkPreparationSweepTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkPreparationSweepTest.java index d0e52bc7..f1d45414 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkPreparationSweepTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkPreparationSweepTest.java @@ -105,6 +105,30 @@ void aSetupTheHarnessNoLongerRunsIsNotPrepared() throws Exception { } } + @Inject dev.codespire.orchestrator.factory.HarnessCredentialPool pool; + + /** How the setup pays is what the prepared task binds, so the approval covers it (M3.5 part F). */ + @Test + void theSavedWayToPayIsCopiedIntoThePreparedTask() throws Exception { + java.util.UUID seat; + try (Connection c = dataSource.getConnection()) { + seat = pool.addSubscription(c, "TEST-sweep-seat-" + java.util.UUID.randomUUID(), "codex", "{\"auth_mode\":\"TEST\",\"tokens\":{\"account_id\":\"TEST-account-" + java.util.UUID.randomUUID() + "\"}}"); + } + try { + defaults.save(repository, new BuildDefaults.Input(defaults.get(repository).revision(), "main", "codex", model, + null, "SUBSCRIPTION"), "TEST-prepared-admin"); + String id = admit("assisted", 83); + + sweep.sweep(); + + var prepared = store.load(id).preparation(); + assertNotNull(prepared); + assertEquals(dev.codespire.contract.work.PayWith.SUBSCRIPTION, prepared.payWith()); + } finally { + pool.remove(seat); + } + } + /** The level saved with the build setup is the level the prepared task binds (M3.5 part M). */ @Test void theSavedThinkingLevelIsCopiedIntoThePreparedTask() throws Exception { @@ -119,7 +143,7 @@ void theSavedThinkingLevelIsCopiedIntoThePreparedTask() throws Exception { var prepared = store.load(id).preparation(); assertNotNull(prepared); assertEquals("high", prepared.effort()); - assertEquals(WorkPreparation.EFFORT_BINDING, prepared.bindingVersion()); + assertEquals(WorkPreparation.PAY_WITH_BINDING, prepared.bindingVersion()); } finally { executeWith("DELETE FROM harness_catalogue"); } @@ -134,8 +158,9 @@ void oneTicketBecomesAPreparedTaskWithNoTypingAtAll() throws Exception { var prepared = store.load(id).preparation(); assertNotNull(prepared, "the ticket alone must be enough"); - assertEquals(WorkPreparation.EFFORT_BINDING, prepared.bindingVersion()); + assertEquals(WorkPreparation.PAY_WITH_BINDING, prepared.bindingVersion()); assertNull(prepared.effort(), "no level was saved, so the model's own default applies"); + assertEquals(dev.codespire.contract.work.PayWith.API_KEY, prepared.payWith(), "a setup that says nothing pays with a key"); assertEquals(WorkPreparation.Origin.STORED, prepared.specification().origin()); assertEquals(WorkPreparation.Origin.STORED, prepared.plan().origin()); // The build coordinates come from the repository's saved setup, and the commit from the forge. diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkRunDispatchTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkRunDispatchTest.java index 31d65e5a..7db3855d 100644 --- a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkRunDispatchTest.java +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/work/WorkRunDispatchTest.java @@ -62,6 +62,63 @@ private void codexRuns(String slug,String... levels) { "TEST-registry.invalid/agent@sha256:0000000000000000000000000000000000000000000000000000000000000000")); } @org.junit.jupiter.api.AfterEach void forgetTheCatalogue() throws Exception {executeWith("DELETE FROM harness_catalogue");} + @Inject dev.codespire.encryption.EncryptionService encryption; + + /** + * A build approved to pay with a subscription leases a seat and hands the agent the sign-in with its + * refresh token emptied — never the whole stored file (M3.5 part F, design §5.6). + */ + @Test void aSubscriptionBuildLeasesASeatAndHandsOverNoRefreshToken() throws Exception { + UUID seat; + try(var c=dataSource.getConnection()) { + seat=pool.addSubscription(c,"TEST-dispatch-seat-"+UUID.randomUUID(),"codex", + "{\"auth_mode\":\"chatgpt\",\"tokens\":{\"access_token\":\"TEST-access\",\"refresh_token\":\"TEST-refresh-must-not-leave\"," + +"\"account_id\":\"TEST-account-"+UUID.randomUUID()+"\"}}"); + } + try { + String id=admit("autonomous",58);var plain=preparation("TEST-prepared-admin"); + var paying=new WorkPreparation(plain.specification(),plain.plan(),plain.baseBranch(),plain.baseCommit(),plain.harness(), + plain.model(),plain.registeredBy(),WorkPreparation.PAY_WITH_BINDING,null,PayWith.SUBSCRIPTION); + var outcome=transitions.prepare(id,store.history(id).size(),paying);assertEquals(200,outcome.status(),outcome.reason()); + + dispatcher.drain(); + + var command=heldCommands.getLast().execution(); + assertTrue(command.harnessSignIn(),"the worker must write a file, not pipe a key"); + String handedOver=encryption.decryptString(command.harnessCredential(),dev.codespire.contract.command.RunCommand.harnessCredentialAad(command.runId())); + assertFalse(handedOver.contains("TEST-refresh-must-not-leave"),"the refresh token never leaves the orchestrator"); + assertTrue(handedOver.contains("TEST-access")); + assertEquals(1,count("SELECT count(*) FROM factory_run WHERE run_id=? AND harness_credential_id=?",command.runId(),seat), + "the run names the seat that paid"); + assertEquals(1,count("SELECT count(*) FROM factory_run WHERE run_id=? AND paid_by='SUBSCRIPTION'",command.runId()), + "the run records how it paid, where a re-arm cannot erase it"); + } finally { pool.remove(seat); } + } + + /** A stored sign-in that cannot be read is refused by name, and nothing reaches a worker. */ + @Test void aSubscriptionBuildWhoseSignInCannotBeReadDispatchesNothing() throws Exception { + UUID seat; + try(var c=dataSource.getConnection()) { + seat=pool.addSubscription(c,"TEST-dispatch-unreadable-"+UUID.randomUUID(),"codex", + "{\"auth_mode\":\"TEST\",\"tokens\":{\"account_id\":\"TEST-account-"+UUID.randomUUID()+"\"}}"); + // The stored file becomes unreadable after it was identified: the hand-over fails at assembly. + try(var ps=c.prepareStatement("UPDATE harness_credential SET api_key=? WHERE id=?")) { + ps.setString(1,encryption.encryptString("TEST-not-a-sign-in","harness-credential:"+seat)); + ps.setObject(2,seat);ps.executeUpdate(); + } + } + try { + String id=admit("autonomous",59);var plain=preparation("TEST-prepared-admin"); + var paying=new WorkPreparation(plain.specification(),plain.plan(),plain.baseBranch(),plain.baseCommit(),plain.harness(), + plain.model(),plain.registeredBy(),WorkPreparation.PAY_WITH_BINDING,null,PayWith.SUBSCRIPTION); + var outcome=transitions.prepare(id,store.history(id).size(),paying);assertEquals(200,outcome.status(),outcome.reason()); + int before=heldCommands.size(); + + dispatcher.drain(); + + assertEquals(before,heldCommands.size(),"nothing was dispatched"); + } finally { pool.remove(seat); } + } @Inject RunResultSaga saga; @Inject dev.codespire.orchestrator.factory.WorkRunAssembly assembly; String ready() throws Exception {String id=admit("autonomous",56);register(id);return id;} diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/Credentials.java b/spire-run-worker/src/main/java/dev/codespire/runworker/Credentials.java index 7192bedb..6d1a351e 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/Credentials.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/Credentials.java @@ -91,4 +91,43 @@ public Map harnessEnv(String runId, String packed) { return Map.of(HarnessInvocation.CREDENTIAL, encryption.decryptString(packed, RunCommand.harnessCredentialAad(runId))); } + + /** + * The same credential, handed over under {@link HarnessInvocation#SIGN_IN} when it is a sign-in file. + * It goes through {@link #harnessEnv(String, String)} rather than decrypting again, so that method + * stays the one a test fake overrides: a second decrypting overload let the scrub path bypass every + * fake and leave the key unscrubbed. + */ + public Map harnessEnv(String runId, String packed, boolean signIn) { + Map env = harnessEnv(runId, packed); + if (!signIn || env.isEmpty()) return env; + return Map.of(HarnessInvocation.SIGN_IN, env.values().iterator().next()); + } + + /** + * Everything in a sign-in file that must never reach a log: the file itself, and every text value in + * it long enough to be a token or an account id. A tool that echoes one token prints that token, not + * the whole file, so scrubbing the file as one string would miss exactly what leaks. + */ + public static java.util.List signInSecrets(String file) { + java.util.List secrets = new java.util.ArrayList<>(); + secrets.add(file); + try { + collect(JSON.readTree(file), secrets); + } catch (JsonProcessingException unreadable) { + // The whole file is still scrubbed; nothing inside it can be named. + } + return secrets; + } + + private static void collect(com.fasterxml.jackson.databind.JsonNode node, java.util.List secrets) { + if (node.isTextual() && node.asText().length() >= SHORTEST_SCRUBBED_VALUE) secrets.add(node.asText()); + node.forEach(child -> collect(child, secrets)); + } + + /** Short enough to cover an account id, long enough to leave a word like "chatgpt" alone. */ + private static final int SHORTEST_SCRUBBED_VALUE = 16; + + /** For the static helper above; it reads a document and never serialises one. */ + private static final ObjectMapper JSON = new ObjectMapper(); } diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/HarnessSignInWorker.java b/spire-run-worker/src/main/java/dev/codespire/runworker/HarnessSignInWorker.java index 4bbf4bf3..b46ba828 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/HarnessSignInWorker.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/HarnessSignInWorker.java @@ -59,6 +59,9 @@ public class HarnessSignInWorker { @Inject SignInRuntime runtime; @Inject EncryptionService encryption; + + /** Keys a sign-in's entry in LiveSecrets apart from any run id. */ + private static final String SIGN_IN_SECRETS = "sign-in:"; @Inject HarnessRegistry harnesses; /** Keyed and AWAITABLE: a Record send answers a stage, which a bare Message send does not. */ @@ -243,6 +246,7 @@ private void start(HarnessSignInCommand.Start command) { } } finally { running.remove(command.signInId()); + LiveSecrets.forget(SIGN_IN_SECRETS + command.signInId()); // ALWAYS, including after a failure. What this removes is a credential the operator just // created; leaving it in a stopped container leaves it readable by anything that can reach // the daemon, for as long as nobody notices. @@ -270,6 +274,10 @@ private HarnessSignInResult collect(String signInId, SignInRuntime.Handle handle "the sign-in ended without a credential (exit " + exit + ")"); } String body = new String(written.get(), StandardCharsets.UTF_8); + // A person's account credential, in this process until the unit is destroyed; see LiveSecrets. + LiveSecrets.register(SIGN_IN_SECRETS + signInId, () -> dev.codespire.secrets.SecretScrub.of( + Credentials.signInSecrets(body).stream() + .map(secret -> new dev.codespire.secrets.SecretScrub.Credential(null, secret)).toList())); String mode = SignInAuthMode.of(body); if (mode == null) { return new HarnessSignInResult.Failed(signInId, HarnessSignInResult.Failed.UNIT_FAILED, diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/LiveSecrets.java b/spire-run-worker/src/main/java/dev/codespire/runworker/LiveSecrets.java new file mode 100644 index 00000000..491f4a03 --- /dev/null +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/LiveSecrets.java @@ -0,0 +1,64 @@ +package dev.codespire.runworker; + +import dev.codespire.secrets.SecretScrub; +import org.jboss.logging.Logger; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + +/** + * The secrets of every run and sign-in this worker holds right now, for {@link SecretLogFilter}. + * + *

A failure detail was always scrubbed before it left the worker, but a log line was not: about + * twenty-five places log an exception whose message can quote a container's create request, and with it + * a model key, a forge token or a sign-in (review of PR #178). Scrubbing each site would miss the next + * one written. So every log record passes one filter, and the filter scrubs against what is held here. + * + *

Static because Quarkus builds logging filters before CDI exists. An entry lives while its unit may + * still produce output: from the start of a run until its unit is gone. A unit that outlives this + * process is logged about by a later process that never decrypted its secrets, so that process cannot + * scrub them — the same limit {@code RunFailures} states for the watchdog. + */ +final class LiveSecrets { + + private static final Logger LOG = Logger.getLogger(LiveSecrets.class); + + private static final Map HELD = new ConcurrentHashMap<>(); + + private LiveSecrets() { + } + + /** + * Holds a run's secrets for the log filter. Never throws: building the scrub decrypts, and a run must + * not fail because its log lines could not be protected — it says so instead. + */ + static void register(String key, Supplier scrub) { + try { + HELD.put(key, scrub.get()); + } catch (RuntimeException unavailable) { + LOG.warnf("%s: its secrets could not be read for log scrubbing (%s); its log lines are not scrubbed", + key, unavailable.getClass().getSimpleName()); + } + } + + static void forget(String key) { + HELD.remove(key); + } + + static boolean isEmpty() { + return HELD.isEmpty(); + } + + static boolean holds(String key) { + return HELD.containsKey(key); + } + + /** The text with every held secret replaced. */ + static String clean(String text) { + if (text == null) return null; + String cleaned = text; + for (SecretScrub scrub : HELD.values()) cleaned = scrub.clean(cleaned); + return cleaned; + } +} diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/OrphanWatchdog.java b/spire-run-worker/src/main/java/dev/codespire/runworker/OrphanWatchdog.java index 38e79ba9..87cee981 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/OrphanWatchdog.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/OrphanWatchdog.java @@ -259,6 +259,7 @@ private void reap(RunHandle unit, boolean alreadyReported) { } if (destroy(unit)) { leases.release(unit.runId()); + LiveSecrets.forget(unit.runId()); } } diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/RunDispatcher.java b/spire-run-worker/src/main/java/dev/codespire/runworker/RunDispatcher.java index 95c904b6..59bdd208 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/RunDispatcher.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/RunDispatcher.java @@ -214,6 +214,8 @@ private CompletionStage handle(Message message, RunCommand com return DONE; } + // From here the run's credentials are in a container that may log them back; see LiveSecrets. + LiveSecrets.register(execute.runId(), () -> failures.scrubFor(execute)); LeaseKeeper keeper = new LeaseKeeper(execute.runId(), execute.harness()); RunResult result; try { @@ -240,7 +242,8 @@ private CompletionStage handle(Message message, RunCommand com // reclaim it. emit is guarded, but asCancellationIfCancelled reaches SecretScrub, which // is not, so "nothing after the ack may throw" was a rule this line did not enforce. registry.forget(execute.runId()); - keeper.settle(); + // A preserved unit may still log; the watchdog forgets it when it reaps the unit. + if (keeper.settle()) LiveSecrets.forget(execute.runId()); } return DONE; } @@ -360,13 +363,15 @@ public void unitReleased() { * find — but it is STAMPED rather than merely left alone, which stops the heartbeat from * refreshing it forever and so lets it become findable at all. */ - private void settle() { + /** @return whether the unit is gone */ + private boolean settle() { if (!unitExists || unitGone) { leases.release(runId); - return; + return true; } LOG.infof("run %s: its unit was not destroyed, so the lease is kept for the watchdog", runId); leases.preserve(runId); + return false; } } diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/RunFailures.java b/spire-run-worker/src/main/java/dev/codespire/runworker/RunFailures.java index e7502799..1977b44b 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/RunFailures.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/RunFailures.java @@ -64,13 +64,20 @@ public RunResult.RunFailed of(RunCommand.ExecuteRun command, String cause, Strin /** Publication may use a rotated identity. Redact it as well as the original build secrets. */ public RunResult.RunFailed ofPublication(RunCommand.ExecuteRun command, RunCommand.PublishWorkRun publication, String cause, String detail) { + // A failed current-credential read propagates to retained publication recovery. It must + // never turn potentially credential-bearing publisher text into an emitted failure. + return of(command, cause, scrubForPublication(command, publication).clean(detail)); + } + + /** + * The forge credential a publication runs with — the CURRENT one its permit carries, which may have + * been rotated since the build and so be absent from {@link #scrubFor}. + */ + SecretScrub scrubForPublication(RunCommand.ExecuteRun command, RunCommand.PublishWorkRun publication) { Credentials.Scm scm = credentials.scm(command.runId(), publication.scmCredential()); - SecretScrub current = SecretScrub.of(List.of( + return SecretScrub.of(List.of( new SecretScrub.Credential(scm.readUsername(), scm.readSecret()), new SecretScrub.Credential(scm.writeUsername(), scm.writeSecret()))); - // A failed current-credential read propagates to retained publication recovery. It must - // never turn potentially credential-bearing publisher text into an emitted failure. - return of(command, cause, current.clean(detail)); } /** @@ -113,8 +120,12 @@ SecretScrub scrubFor(RunCommand.ExecuteRun command) { try { // No username: an API key rides a Bearer header, not a Basic pair, so a base64 form // built for it would match nothing. - credentials.harnessEnv(command.runId(), command.harnessCredential()).values() - .forEach(secret -> forms.add(new SecretScrub.Credential(null, secret))); + for (String secret : credentials.harnessEnv(command.runId(), command.harnessCredential(), + command.harnessSignIn()).values()) { + // A sign-in file is scrubbed token by token as well as whole (M3.5 part F). + for (String form : command.harnessSignIn() ? Credentials.signInSecrets(secret) : java.util.List.of(secret)) + forms.add(new SecretScrub.Credential(null, form)); + } } catch (RuntimeException undecryptable) { LOG.warnf("run %s: the harness credential could not be decrypted to redact it; this " + "run's failure details are unscrubbed for it", command.runId()); diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/RunUnitBuilder.java b/spire-run-worker/src/main/java/dev/codespire/runworker/RunUnitBuilder.java index 84dfbb40..7328e5bd 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/RunUnitBuilder.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/RunUnitBuilder.java @@ -126,7 +126,8 @@ private RunUnitSpec build(RunCommand.ExecuteRun command, HarnessAdapter adapter, + "); the channel's ack budget is sized to the latter"); } Credentials.Scm scm = credentials.scm(command.runId(), command.scmCredential()); - Map harnessEnv = credentials.harnessEnv(command.runId(), command.harnessCredential()); + Map harnessEnv = credentials.harnessEnv(command.runId(), command.harnessCredential(), + command.harnessSignIn()); ContainerSpec init = new ContainerSpec( publisherImage, diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/SecretLogFilter.java b/spire-run-worker/src/main/java/dev/codespire/runworker/SecretLogFilter.java new file mode 100644 index 00000000..f3686cbe --- /dev/null +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/SecretLogFilter.java @@ -0,0 +1,95 @@ +package dev.codespire.runworker; + +import io.quarkus.logging.LoggingFilter; +import org.jboss.logmanager.ExtLogRecord; + +import java.util.IdentityHashMap; +import java.util.Map; +import java.util.logging.Filter; +import java.util.logging.LogRecord; +import java.util.logging.SimpleFormatter; + +/** + * Scrubs every held run secret out of every log record: the message, and every exception a formatter + * prints — the cause chain and each suppressed exception (review of PR #178). Configured on the console + * handler as {@code run-secrets}. + * + *

An exception is replaced, not edited — a {@link Throwable}'s message cannot be changed — by a copy + * that keeps the original class name in its message and the original stack frames, so the trace still + * points where it did. When anything in the graph quotes a secret, the WHOLE graph is copied: a copy + * that kept an original branch would print that branch as it was. + */ +@LoggingFilter(name = "run-secrets") +public final class SecretLogFilter implements Filter { + + /** + * How many exceptions one graph may hold before the rest is cut off. A real one has a handful; the + * bound exists so a pathological graph cannot stall logging, and what is cut is dropped, never + * printed unscrubbed. + */ + private static final int MAX_EXCEPTIONS = 64; + + @Override + public boolean isLoggable(LogRecord record) { + if (LiveSecrets.isEmpty()) return true; + String message = record instanceof ExtLogRecord ext ? ext.getFormattedMessage() : new SimpleFormatter().formatMessage(record); + String cleaned = LiveSecrets.clean(message); + if (cleaned != null && !cleaned.equals(message)) { + if (record instanceof ExtLogRecord ext) ext.setMessage(cleaned, ExtLogRecord.FormatStyle.NO_FORMAT); + else record.setMessage(cleaned); + record.setParameters(null); + } + Throwable thrown = record.getThrown(); + if (thrown != null && quotesASecret(thrown, new IdentityHashMap<>())) record.setThrown(copy(thrown, new Budget())); + return true; + } + + /** + * Whether the graph may quote a held secret. A graph too large to scan counts as one that does: the + * copy is bounded too, and dropping what it cannot hold is safe where trusting it is not. + */ + private static boolean quotesASecret(Throwable thrown, Map seen) { + if (thrown == null) return false; + if (seen.size() >= MAX_EXCEPTIONS) return true; + if (seen.put(thrown, Boolean.TRUE) != null) return false; + String message = thrown.getMessage(); + if (message != null && !message.equals(LiveSecrets.clean(message))) return true; + if (quotesASecret(thrown.getCause(), seen)) return true; + for (Throwable suppressed : thrown.getSuppressed()) if (quotesASecret(suppressed, seen)) return true; + return false; + } + + /** Counts every exception copied, so a cycle or a huge graph ends in a cut, not a stall. */ + private static final class Budget { + private final Map seen = new IdentityHashMap<>(); + + boolean admit(Throwable thrown) { + return seen.size() < MAX_EXCEPTIONS && seen.put(thrown, Boolean.TRUE) == null; + } + } + + private static Throwable copy(Throwable original, Budget budget) { + if (original == null || !budget.admit(original)) return null; + Scrubbed copy = new Scrubbed(original, copy(original.getCause(), budget)); + for (Throwable suppressed : original.getSuppressed()) { + Throwable scrubbed = copy(suppressed, budget); + if (scrubbed != null) copy.addSuppressed(scrubbed); + } + return copy; + } + + /** A stand-in carrying the original's class name, scrubbed message and stack frames. */ + static final class Scrubbed extends RuntimeException { + Scrubbed(Throwable original, Throwable cause) { + super(original.getClass().getName() + ": " + LiveSecrets.clean(String.valueOf(original.getMessage())), + cause, true, true); + setStackTrace(original.getStackTrace()); + } + + /** Printed as the original would be — its class and message — not as this wrapper's name. */ + @Override + public String toString() { + return getMessage(); + } + } +} diff --git a/spire-run-worker/src/main/java/dev/codespire/runworker/WorkRunWorker.java b/spire-run-worker/src/main/java/dev/codespire/runworker/WorkRunWorker.java index ecaceda7..08cd2474 100644 --- a/spire-run-worker/src/main/java/dev/codespire/runworker/WorkRunWorker.java +++ b/spire-run-worker/src/main/java/dev/codespire/runworker/WorkRunWorker.java @@ -36,6 +36,8 @@ public class WorkRunWorker { @Inject RunTranscript transcript; @ConfigProperty(name="spire.run.orphan-stale-after-seconds") long staleAfterSeconds; private final Set active=ConcurrentHashMap.newKeySet(); + /** Keys a publication's own credential apart from the build's, under the same run id. */ + static final String PUBLICATION_SECRETS=":publication"; public CompletionStage execute(Message message,RunCommand.ExecuteWorkRun command) { boolean claimed=store.claim(command); // No ack until the command and shared M2 execute slot commit together. @@ -50,7 +52,16 @@ public CompletionStage execute(Message message,RunCommand.Exec } else if(!leases.take(id)) { result=failures.of(command.execution(),"WORKER_FAILED","No lease was taken; the held build was not started"); } else { - result=launcher.launchHeld(command,new HeldObserver(command),unit->store.saveUnit(id,unit)); + // Held until the retained workspace is released: a held unit's publisher runs later. Not + // before the refusals above, which create nothing and would leave the entry held for ever. + LiveSecrets.register(id,()->failures.scrubFor(command.execution())); + // The topology is saved immediately before creation is attempted. Only a launch that + // never got that far is proven to have created nothing: a create that fails part-way can + // leave credential-bearing containers behind with no unit reported (review of PR #178). + boolean[] creationAttempted={false}; + // After the save: a save that throws stops the launch before anything is created. + result=launcher.launchHeld(command,new HeldObserver(command),unit->{store.saveUnit(id,unit);creationAttempted[0]=true;}); + if(!creationAttempted[0])LiveSecrets.forget(id); } if(cancelled(id) || store.revoked(command)) result=cancelledResult(command,result); store.buildResult(result); @@ -83,6 +94,10 @@ public void publish(RunCommand.PublishWorkRun request) { private void publishClaimed(WorkRunStore.Held held,RunHandle handle,PublicationRuntime publication) { String id=held.execution().runId(); + // A publisher may run in a later process than the build did, so its secrets are held again here — + // and the permit's forge credential with them, which may have been rotated since the build. + LiveSecrets.register(id,()->failures.scrubFor(held.execution().execution())); + LiveSecrets.register(id+PUBLICATION_SECRETS,()->failures.scrubForPublication(held.execution().execution(),held.permit())); // Register the retained handle before publisher creation so control can find this run again. registry.register(id,held.execution().execution().harness(),handle,RunNotes.IGNORING); try { @@ -173,6 +188,8 @@ private void releasePublished(WorkRunStore.Held held,PublicationRuntime publicat else if(localUnit(id).isPresent())throw new IllegalStateException("Published resources no longer carry their expected hold"); store.released(id); leases.release(id); + LiveSecrets.forget(id); + LiveSecrets.forget(id+PUBLICATION_SECRETS); } private Optional localUnit(String id) { diff --git a/spire-run-worker/src/main/resources/application.yml b/spire-run-worker/src/main/resources/application.yml index ab0cdc17..fe7eeb9d 100644 --- a/spire-run-worker/src/main/resources/application.yml +++ b/spire-run-worker/src/main/resources/application.yml @@ -68,6 +68,9 @@ quarkus: create-schemas: true log: console: + # Scrubs every held run secret out of every record, exceptions included (SecretLogFilter). + # Some exceptions quote a container's create request, and with it the run's credentials. + filter: run-secrets json: enabled: true # The same two fields the other deployables carry, so one log pipeline can tell the diff --git a/spire-run-worker/src/test/java/dev/codespire/runworker/RunDispatcherTest.java b/spire-run-worker/src/test/java/dev/codespire/runworker/RunDispatcherTest.java index 38e9405a..c92666e3 100644 --- a/spire-run-worker/src/test/java/dev/codespire/runworker/RunDispatcherTest.java +++ b/spire-run-worker/src/test/java/dev/codespire/runworker/RunDispatcherTest.java @@ -562,6 +562,7 @@ void anOrdinaryFailureReleasesItsLease() { dispatcher.onCommand(new Delivery(order).of(EXECUTE)).toCompletableFuture().join(); assertTrue(leases.released); + assertFalse(LiveSecrets.holds(EXECUTE.runId()), "a gone unit logs nothing more, so its secrets are let go"); } @Test @@ -577,6 +578,9 @@ void aFailureWhoseUnitSurvivedKeepsItsLeaseWhateverItsCause() { assertFalse(leases.released); assertTrue(leases.preserved); + // A surviving unit can still log; its secrets stay scrubbed until the watchdog reaps it. + assertTrue(LiveSecrets.holds(EXECUTE.runId())); + LiveSecrets.forget(EXECUTE.runId()); } @Test diff --git a/spire-run-worker/src/test/java/dev/codespire/runworker/RunLauncherTest.java b/spire-run-worker/src/test/java/dev/codespire/runworker/RunLauncherTest.java index c6fc6fc6..03e1d8d2 100644 --- a/spire-run-worker/src/test/java/dev/codespire/runworker/RunLauncherTest.java +++ b/spire-run-worker/src/test/java/dev/codespire/runworker/RunLauncherTest.java @@ -670,6 +670,31 @@ void aFailureDetailCarriesNoCredential() { assertTrue(failed.detail().contains("create failed"), "the diagnosis itself must survive"); } + /** A sign-in is scrubbed token by token: an echoed access token is not the whole file (M3.5 part F). */ + @Test + void aSignInRunScrubsEachTokenOfTheFile() { + String accessToken = "TEST-access-token-0123456789"; + launcher.failures = failuresWith(new Credentials() { + @Override + public Scm scm(String runId, String packed) { + return new Scm(SCM_USERNAME, READ_SECRET, SCM_USERNAME, WRITE_SECRET); + } + + @Override + public Map harnessEnv(String runId, String packed) { + return Map.of(dev.codespire.harness.HarnessInvocation.CREDENTIAL, + "{\"tokens\":{\"access_token\":\"" + accessToken + "\"}}"); + } + }); + runtime.salvageFails = new IllegalStateException("codex said: bearer " + accessToken); + + RunResult.RunFailed failed = assertInstanceOf(RunResult.RunFailed.class, + launcher.launch(COMMAND.paidBySignIn(), RunObserver.IGNORING)); + + assertFalse(failed.detail().contains(accessToken), "one echoed token is still a leaked token"); + assertTrue(failed.detail().contains("codex said"), "the diagnosis itself must survive"); + } + @Test void aFailureIsRetryableOnlyWhenItsCauseIs() { // Every publisher failure used to be reported retryable. A push the forge rejected refuses diff --git a/spire-run-worker/src/test/java/dev/codespire/runworker/RunUnitBuilderTest.java b/spire-run-worker/src/test/java/dev/codespire/runworker/RunUnitBuilderTest.java index 88265690..55a2f646 100644 --- a/spire-run-worker/src/test/java/dev/codespire/runworker/RunUnitBuilderTest.java +++ b/spire-run-worker/src/test/java/dev/codespire/runworker/RunUnitBuilderTest.java @@ -192,6 +192,15 @@ void aProtectedBranchIsNotDroppedJustBecauseTheModeIsTheDefault() { assertFalse(env.containsKey("SPIRE_BRANCH_MODE"), env.keySet().toString()); } + /** A run paid by a sign-in hands the agent the file, never as a key (M3.5 part F). */ + @Test + void aRunPaidBySignInHandsTheAgentTheFileNotAKey() { + Map env = builder.build(command().paidBySignIn(), new CodexAdapter()).agent().environment(); + + assertEquals(HARNESS_KEY, env.get("CODEX_SIGN_IN_FILE")); + assertFalse(env.containsKey("OPENAI_API_KEY")); + } + /** The level on the command is the level in the agent's command line, not lost on the way. */ @Test void theThinkingLevelReachesTheAgentsCommand() { diff --git a/spire-run-worker/src/test/java/dev/codespire/runworker/SecretLogFilterTest.java b/spire-run-worker/src/test/java/dev/codespire/runworker/SecretLogFilterTest.java new file mode 100644 index 00000000..9657f280 --- /dev/null +++ b/spire-run-worker/src/test/java/dev/codespire/runworker/SecretLogFilterTest.java @@ -0,0 +1,119 @@ +package dev.codespire.runworker; + +import dev.codespire.secrets.SecretScrub; +import org.jboss.logmanager.ExtLogRecord; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.logging.Level; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * No held run secret reaches a log line, in the message or anywhere in the exception chain (review of + * PR #178: raw exceptions quoting a container's create request were logged at about 25 places). + */ +class SecretLogFilterTest { + + private static final String RUN = "TEST-log-filter-run"; + private static final String SECRET = "TEST-secret-0123456789"; + + @AfterEach + void forget() { + LiveSecrets.forget(RUN); + } + + private static void hold() { + LiveSecrets.register(RUN, () -> SecretScrub.of(List.of(new SecretScrub.Credential(null, SECRET)))); + } + + private static ExtLogRecord record() { + ExtLogRecord record = new ExtLogRecord(Level.SEVERE, "run %s failed: %s", ExtLogRecord.FormatStyle.PRINTF, + SecretLogFilterTest.class.getName()); + record.setParameters(new Object[] {RUN, "env=[OPENAI_API_KEY=" + SECRET + "]"}); + record.setThrown(new IllegalStateException("create failed: " + SECRET, + new RuntimeException("caused by " + SECRET))); + return record; + } + + @Test + void aHeldSecretLeavesNeitherTheMessageNorTheExceptionChain() { + hold(); + ExtLogRecord record = record(); + StackTraceElement[] frames = record.getThrown().getStackTrace(); + + assertTrue(new SecretLogFilter().isLoggable(record), "scrubbed, never dropped"); + + assertFalse(record.getFormattedMessage().contains(SECRET), record.getFormattedMessage()); + assertTrue(record.getFormattedMessage().contains("run " + RUN + " failed"), "the diagnosis survives"); + for (Throwable at = record.getThrown(); at != null; at = at.getCause()) { + assertFalse(String.valueOf(at.getMessage()).contains(SECRET), at.getMessage()); + } + assertTrue(record.getThrown().toString().startsWith("java.lang.IllegalStateException: create failed"), + "the original class still names the failure"); + assertEquals(frames.length, record.getThrown().getStackTrace().length, "the trace still points where it did"); + } + + /** A formatter prints suppressed exceptions too; a secret there is still a secret in the log. */ + @Test + void aSecretInASuppressedExceptionIsScrubbed() { + hold(); + ExtLogRecord record = new ExtLogRecord(Level.SEVERE, "cleanup failed", ExtLogRecord.FormatStyle.NO_FORMAT, + SecretLogFilterTest.class.getName()); + IllegalStateException outer = new IllegalStateException("harmless"); + outer.addSuppressed(new RuntimeException("close failed: " + SECRET)); + record.setThrown(outer); + + new SecretLogFilter().isLoggable(record); + + Throwable shown = record.getThrown(); + assertEquals(1, shown.getSuppressed().length, "the suppressed exception is kept, scrubbed"); + assertFalse(shown.getSuppressed()[0].getMessage().contains(SECRET), shown.getSuppressed()[0].getMessage()); + java.io.StringWriter printed = new java.io.StringWriter(); + shown.printStackTrace(new java.io.PrintWriter(printed)); + assertFalse(printed.toString().contains(SECRET), "nothing a formatter prints quotes the secret"); + } + + /** A secret beyond what the filter scans is not trusted to be absent (review of PR #178). */ + @Test + void aSecretBeyondTheScanLimitDoesNotReachTheLog() { + hold(); + Throwable deepest = new RuntimeException("deep: " + SECRET); + Throwable chain = deepest; + for (int i = 0; i < 100; i++) chain = new RuntimeException("layer " + i, chain); + ExtLogRecord record = new ExtLogRecord(Level.SEVERE, "failed", ExtLogRecord.FormatStyle.NO_FORMAT, + SecretLogFilterTest.class.getName()); + record.setThrown(chain); + + new SecretLogFilter().isLoggable(record); + + java.io.StringWriter printed = new java.io.StringWriter(); + record.getThrown().printStackTrace(new java.io.PrintWriter(printed)); + assertFalse(printed.toString().contains(SECRET), "cut off, never printed as it was"); + } + + @Test + void aForgottenRunIsNoLongerScrubbed() { + hold(); + LiveSecrets.forget(RUN); + ExtLogRecord record = record(); + + new SecretLogFilter().isLoggable(record); + + assertTrue(record.getFormattedMessage().contains(SECRET), "nothing is held, so nothing is changed"); + } + + /** The filter does nothing unless the console handler names it; the name is the contract. */ + @Test + void theConsoleHandlerUsesTheFilter() throws Exception { + String config = Files.readString(Path.of("src/main/resources/application.yml")); + String name = SecretLogFilter.class.getAnnotation(io.quarkus.logging.LoggingFilter.class).name(); + + assertTrue(config.contains("filter: " + name), "quarkus.log.console.filter must name " + name); + } +} diff --git a/spire-run-worker/src/test/java/dev/codespire/runworker/SignInSecretsTest.java b/spire-run-worker/src/test/java/dev/codespire/runworker/SignInSecretsTest.java new file mode 100644 index 00000000..c2545e40 --- /dev/null +++ b/spire-run-worker/src/test/java/dev/codespire/runworker/SignInSecretsTest.java @@ -0,0 +1,40 @@ +package dev.codespire.runworker; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * A tool that echoes one token prints that token, not the whole file, so a sign-in is scrubbed token by + * token as well as whole (M3.5 part F). Placeholder values: this is about which strings are listed. + */ +class SignInSecretsTest { + + private static final String FILE = "{\"auth_mode\":\"chatgpt\",\"tokens\":{\"access_token\":\"TEST-access-token-0123456789\"," + + "\"id_token\":\"TEST-id-token-0123456789\",\"refresh_token\":\"\",\"account_id\":\"TEST-account-0123456789\"}}"; + + @Test + void everyTokenInTheFileIsScrubbedAsWellAsTheWholeFile() { + var secrets = Credentials.signInSecrets(FILE); + + assertTrue(secrets.contains(FILE)); + assertTrue(secrets.contains("TEST-access-token-0123456789")); + assertTrue(secrets.contains("TEST-id-token-0123456789")); + assertTrue(secrets.contains("TEST-account-0123456789")); + } + + /** A short word is not a secret, and scrubbing it would mangle every log line that says it. */ + @Test + void shortWordsAreLeftAlone() { + var secrets = Credentials.signInSecrets(FILE); + + assertFalse(secrets.contains("chatgpt")); + assertFalse(secrets.contains("")); + } + + @Test + void anUnreadableFileIsStillScrubbedWhole() { + assertTrue(Credentials.signInSecrets("not json").contains("not json")); + } +} diff --git a/spire-run-worker/src/test/java/dev/codespire/runworker/WorkRunWorkerTest.java b/spire-run-worker/src/test/java/dev/codespire/runworker/WorkRunWorkerTest.java index b1d1f3e4..e3a30c63 100644 --- a/spire-run-worker/src/test/java/dev/codespire/runworker/WorkRunWorkerTest.java +++ b/spire-run-worker/src/test/java/dev/codespire/runworker/WorkRunWorkerTest.java @@ -22,6 +22,10 @@ class WorkRunWorkerTest { RunResult terminal; String state="ready"; boolean claim=true,permitAllowed=true,cancelled,claimFails,leaseAvailable=true,reportAccepted=true,present=true,held=true,lateCancel; + /** Whether the fake launch reaches creation, and whether creation reports a unit. */ + boolean attemptsCreation=true,createsUnit=true,saveFails; + /** What the log filter did to the rotated publisher secret while the publisher ran. */ + String cleanedDuringPublish; int launches,publicationClaims,publications,deletions; boolean revoked; List publisherLines=List.of("{\"event\":\"pushed\",\"ref\":\"refs/heads/spire/TEST-held\"}"); @@ -41,6 +45,10 @@ class WorkRunWorkerTest { @Override public List awaitingRelease(){return List.of();} @Override public boolean claimPublicationRecovery(String id,Instant stale){return permitAllowed;} @Override public void abandonBuild(RunResult.RunFailed result){terminal(result);} + // Answered here: a launch that creates a unit records it, and the parent would open a database. + @Override public void recordUnit(String id,String unitId){events.add("record-unit");} + // Answered for the same reason: a launch saves its topology just before creating. + @Override public void saveUnit(String id,RunUnitSpec unit){if(saveFails)throw new IllegalStateException("TEST save failed");events.add("save-unit");} }; final WorkspaceLeases leases=new WorkspaceLeases(){ @Override public boolean take(String id){return leaseAvailable;} @@ -48,6 +56,8 @@ class WorkRunWorkerTest { @Override public void release(String id){events.add("release-lease");} @Override public Optional staleBefore(Duration duration){return Optional.of(Instant.now().minusSeconds(60));} @Override public Optional find(String id){return Optional.empty();} + // Answered for the same reason as the store's recordUnit. + @Override public void recordUnit(String id,String unitId){events.add("lease-unit");} }; final class Runtime extends RunLauncherTest.FakeRuntime implements PublicationRuntime { @Override public List discoverUnits(){return present?List.of(new RunHandle(command.runId(),"TEST-unit")):List.of();} @@ -56,6 +66,7 @@ final class Runtime extends RunLauncherTest.FakeRuntime implements PublicationRu @Override public Finalization publishHeld(RunHandle run,PublicationKey key,UUID attempt,RunUnitSpec spec,Consumer lines,BooleanSupplier allowed){ publications++;assertEquals("publishing",state,"The durable claim must precede publisher IO"); assertEquals(command.work().publicationKey(),key.value());assertEquals(permit.permit().deliveryAttemptId(),attempt); + cleanedDuringPublish=LiveSecrets.clean("token=TEST-current-publisher-secret"); if(!allowed.getAsBoolean())lines.accept("{\"event\":\"failed\",\"cause\":\"PUBLICATION_CANCELLED\"}");else WorkRunWorkerTest.this.publisherLines.forEach(lines); if(lateCancel)WorkRunWorkerTest.this.cancelled=true; return WorkRunWorkerTest.this.finalization; @@ -68,17 +79,70 @@ final class Runtime extends RunLauncherTest.FakeRuntime implements PublicationRu @BeforeEach void wire(){ worker.store=store;worker.leases=leases;worker.registry=new RunRegistry();worker.runtime=runtime;worker.staleAfterSeconds=60; worker.claims=new RunClaimStore(){@Override public boolean taken(String id,String slot){return cancelled;}}; - worker.launcher=new RunLauncher(){@Override public RunResult launchHeld(RunCommand.ExecuteWorkRun execution,RunObserver observer,Consumer saved){launches++;if(lateCancel)cancelled=true;return buildResult;}}; + worker.launcher=new RunLauncher(){@Override public RunResult launchHeld(RunCommand.ExecuteWorkRun execution,RunObserver observer,Consumer saved){ + launches++; + // As the real launcher does: a failed topology save ends the launch before creation. + if(attemptsCreation){try{saved.accept(null);}catch(RuntimeException saveFailed){return new RunResult.RunFailed(execution.runId(),"RUNTIME_UNAVAILABLE","TEST save failed",true,null);}} + if(createsUnit)observer.unitCreated("TEST-unit",RunNotes.IGNORING); + if(lateCancel)cancelled=true;return buildResult;}}; worker.builder=new RunUnitBuilder(){@Override public RunUnitSpec publication(RunUnitSpec original,RunCommand.ExecuteWorkRun execution,RunCommand.PublishWorkRun request){return null; /* TEST runtime does not consume topology. */}}; worker.failures=new RunFailures(){ @Override public RunResult.RunFailed of(RunCommand.ExecuteRun execution,String cause,String detail){return new RunResult.RunFailed(execution.runId(),cause,detail,false,null);} // This decision fixture does not decrypt credentials; the focused publication tests below do. @Override public RunResult.RunFailed ofPublication(RunCommand.ExecuteRun execution,RunCommand.PublishWorkRun publication,String cause,String detail){return of(execution,cause,detail);} + // Answered here rather than left to the parent, whose credentials are not injected: the held + // path holds these for the log filter, and a throw would be swallowed there, unseen. + @Override dev.codespire.secrets.SecretScrub scrubFor(RunCommand.ExecuteRun execution){ + return dev.codespire.secrets.SecretScrub.of(List.of(new dev.codespire.secrets.SecretScrub.Credential(null,"TEST-held-secret-0123456789"))); + } }; worker.results=new RunResultReporter(){@Override public boolean report(RunResult result){reported.add(result);return reportAccepted;}}; } - @AfterEach void close(){worker.launcher.stopStreams();} + // LiveSecrets is static and every case uses one run id: a leftover entry would let a case pass for another. + @BeforeEach void noHeldSecrets(){LiveSecrets.forget(command.runId());} + @AfterEach void close(){worker.launcher.stopStreams();LiveSecrets.forget(command.runId());LiveSecrets.forget(command.runId()+WorkRunWorker.PUBLICATION_SECRETS);} void execute(){worker.execute(Message.of((RunCommand)command,()->{events.add("ack-command");return CompletableFuture.completedFuture(null);}),command).toCompletableFuture().join();} + /** A held build's secrets are scrubbed from logs until its workspace is released (review of PR #178). */ + @Test void aHeldBuildsSecretsAreScrubbedUntilItsWorkspaceIsReleased(){ + try { + assertFalse(LiveSecrets.holds(command.runId())); + execute(); + assertTrue(LiveSecrets.holds(command.runId()),"the retained unit's publisher can still log"); + worker.publish(permit); + assertFalse(LiveSecrets.holds(command.runId()),"a released workspace logs nothing more"); + } finally {LiveSecrets.forget(command.runId());} + } + /** A build refused before it starts creates nothing, so nothing holds its secrets (review of PR #178). */ + @Test void aBuildCancelledBeforeItStartsHoldsNoSecrets(){ + cancelled=true;execute(); + assertFalse(LiveSecrets.holds(command.runId())); + } + /** A launch refused before creation was attempted created nothing, so its secrets are let go. */ + @Test void aLaunchRefusedBeforeCreationLetsItsSecretsGo(){ + attemptsCreation=false;createsUnit=false;execute(); + assertEquals(1,launches); + assertFalse(LiveSecrets.holds(command.runId())); + } + /** A topology save that failed stopped the launch before creation, so nothing holds the secrets. */ + @Test void aFailedTopologySaveLetsItsSecretsGo(){ + saveFails=true;createsUnit=false;execute(); + assertFalse(events.contains("save-unit")); + assertFalse(LiveSecrets.holds(command.runId())); + } + /** A creation that failed part-way may have left containers, so their secrets stay scrubbed. */ + @Test void aCreationThatFailedPartWayKeepsItsSecretsScrubbed(){ + attemptsCreation=true;createsUnit=false;execute(); + assertTrue(LiveSecrets.holds(command.runId())); + } + /** A publisher runs with its permit's forge credential, which may be a rotated one the build never saw. */ + @Test void aRotatedPublisherCredentialIsScrubbedFromLogsWhileItPublishes(){ + publicationFailureCredentials(false); + try { + worker.publish(permit); + assertNotNull(cleanedDuringPublish); + assertFalse(cleanedDuringPublish.contains("TEST-current-publisher-secret"),cleanedDuringPublish); + } finally {LiveSecrets.forget(command.runId()+WorkRunWorker.PUBLICATION_SECRETS);} + } @Test void aFailedClaimCannotAcknowledgeTheCommand(){claimFails=true;assertThrows(IllegalStateException.class,this::execute);assertEquals(List.of("claim"),events);} @Test void anExecuteRedeliveryCannotRunTheHarnessAgain(){claim=false;execute();assertEquals(0,launches);assertTrue(reported.isEmpty());assertEquals(List.of("claim","ack-command"),events);} @Test void cancellationBeforeBuildBuysNoHarnessCall(){cancelled=true;execute();assertEquals(0,launches);assertEquals("CANCELLED",assertInstanceOf(RunResult.RunFailed.class,terminal).cause());} diff --git a/spire-ui/src/api.ts b/spire-ui/src/api.ts index 4ea1e14a..3e858f54 100644 --- a/spire-ui/src/api.ts +++ b/spire-ui/src/api.ts @@ -1103,6 +1103,8 @@ export interface RunView extends RunListEntry { // Which pool member paid for this run. Null on a run dispatched before a credential was recorded. credentialLabel: string | null; credentialType: string | null; + // API_KEY or SUBSCRIPTION; absent from a server older than M3.5 part F, which only held API keys. + credentialAuthMode?: string | null; providerType: string; workspace: string; slug: string; @@ -1696,11 +1698,10 @@ export interface HarnessCredentialView { /** When the vendor refused the key. Only an operator clears this. */ rejectedAt: string | null; lastUsedAt: string | null; - /** - * How this member pays. A SUBSCRIPTION is deliberately unreachable by any run until selection, - * injection and zero-cost charging exist, so the screen must not render it as ready to use. - */ + /** How this member pays: an API key, or a signed-in subscription seat. Both are shared by builds. */ authMode?: 'API_KEY' | 'SUBSCRIPTION'; + /** False for a seat whose account is unknown: it is never used until it is signed in again. */ + identified?: boolean; } export interface NewHarnessCredential { label: string; type: string; baseUrl: string; apiKey: string } diff --git a/spire-ui/src/components/HarnessSubscriptionSignIn.tsx b/spire-ui/src/components/HarnessSubscriptionSignIn.tsx index 8453d143..13381b25 100644 --- a/spire-ui/src/components/HarnessSubscriptionSignIn.tsx +++ b/spire-ui/src/components/HarnessSubscriptionSignIn.tsx @@ -15,6 +15,7 @@ const REASONS: Record = { sign_in_cancelled: 'The sign-in was cancelled.', sign_in_unit_failed: 'The sign-in tool did not start, or printed something this version cannot read. Nothing was stored.', sign_in_wrong_mode: 'That signed in as an API key, not a subscription. Add it as an API key instead.', + subscription_unidentified: 'The sign-in did not say which account it belongs to, so it was not stored. Start again.', sign_in_not_started: 'No run worker showed a code within six minutes. Check that the run worker is running and has the agent image, then start again.', }; diff --git a/spire-ui/src/components/RunDefinitionCard.tsx b/spire-ui/src/components/RunDefinitionCard.tsx index 5349c3c6..5cc0deaa 100644 --- a/spire-ui/src/components/RunDefinitionCard.tsx +++ b/spire-ui/src/components/RunDefinitionCard.tsx @@ -5,6 +5,15 @@ import { formatEventTime } from '../format'; import RunCard, { RunFields } from './RunCard'; import { isRunUnfinished, reviewPath, runDuration } from './Runs'; +// Which credential paid, in the operator's words. A subscription seat is not billed per token, so it +// must never read as an API key: that is the one question this row exists to answer (M3.5 part F). +function billedTo(run: RunView): string | null { + if (!run.credentialLabel) return null; + const type = run.credentialType ? ` · ${run.credentialType}` : ''; + if (run.credentialAuthMode === 'SUBSCRIPTION') return `Subscription ${run.credentialLabel}${type}, no per-token price`; + return `API key ${run.credentialLabel}${type ? `${type}, billed per token` : ''}`; +} + export default function RunDefinitionCard({ run }: { run: RunView }) { const [now, setNow] = useState(Date.now); const ticking = isRunUnfinished(run.status) && !run.endedAt; @@ -15,9 +24,7 @@ export default function RunDefinitionCard({ run }: { run: RunView }) { }, [ticking]); const link = run.reviewId ? reviewPath(run.reviewId) : null; const elapsed = runDuration({ ...run, endedAt: run.endedAt ?? (ticking ? new Date(now).toISOString() : null) }); - // Which key paid, in the operator's words. Every harness credential here is an API key, billed per - // token — a subscription is not a credential this deployment holds, so the card never implies one. - const billing = run.credentialLabel ? `API key ${run.credentialLabel}${run.credentialType ? ` · ${run.credentialType}, billed per token` : ''}` : null; + const billing = billedTo(run); return {run.reviewId} : run.reviewId], diff --git a/spire-ui/src/components/RunDetail.test.tsx b/spire-ui/src/components/RunDetail.test.tsx index 1e107c8b..7efc1e74 100644 --- a/spire-ui/src/components/RunDetail.test.tsx +++ b/spire-ui/src/components/RunDetail.test.tsx @@ -196,6 +196,13 @@ it('names the key a run was billed to', async () => { expect(screen.getByText('API key TEST-factory-key · openai, billed per token')).toBeInTheDocument(); }); +// A seat's run is not billed per token; calling it an API key would answer the question wrongly. +it('names a subscription seat as a subscription, not as a key billed per token', async () => { + show(runView({ credentialLabel: 'TEST-factory-seat', credentialType: 'codex', credentialAuthMode: 'SUBSCRIPTION' })); + expect(await screen.findByText('Subscription TEST-factory-seat · codex, no per-token price')).toBeInTheDocument(); + expect(screen.queryByText(/API key/)).toBeNull(); +}); + // An unrecorded key reads as unknown, never as a run nobody paid for. it('shows no key rather than inventing one when a run recorded none', async () => { show(runView({ credentialLabel: null, credentialType: null })); diff --git a/spire-ui/src/components/SettingsHarnessCredentials.test.tsx b/spire-ui/src/components/SettingsHarnessCredentials.test.tsx index a6537bd3..e6db698e 100644 --- a/spire-ui/src/components/SettingsHarnessCredentials.test.tsx +++ b/spire-ui/src/components/SettingsHarnessCredentials.test.tsx @@ -50,6 +50,16 @@ it('tells a resting key apart from a refused one, and offers the action each nee expect(within(rejected).queryByRole('button', { name: 'Rest longer' })).toBeNull(); }); +// A signed-in seat reads as a subscription, so it is never mistaken for a key billed per token (M3.5 part F). +it('shows a signed-in seat as a ready subscription', async () => { + vi.mocked(api.fetchHarnessCredentials).mockResolvedValue([ + member({ id: 'TEST-seat-free', label: 'TEST-seat-free', type: 'codex', authMode: 'SUBSCRIPTION', identified: true }), + ]); + render(); + + expect(within(await row('TEST-seat-free')).getByText('Ready · subscription')).toBeInTheDocument(); +}); + it('switches a member off and back on, and says which one changed', async () => { // The list is read again after the change, so the second answer is the switched-off row. vi.mocked(api.fetchHarnessCredentials).mockResolvedValueOnce([member()]).mockResolvedValue([member({ enabled: false })]); @@ -99,3 +109,25 @@ it('shows the server refusal rather than a status line', async () => { render(); expect(await screen.findByRole('alert')).toHaveTextContent('Failed to load the harness credential pool'); }); + +// A seat whose account is unknown could be a second seat on one account; no build uses it. +it('asks for a new sign-in on a seat whose account is unknown', async () => { + vi.mocked(api.fetchHarnessCredentials).mockResolvedValue([ + member({ id: 'TEST-seat-old', label: 'TEST-seat-old', type: 'codex', authMode: 'SUBSCRIPTION', identified: false }), + ]); + render(); + + expect(within(await row('TEST-seat-old')).getByText('Sign in again')).toBeInTheDocument(); +}); + +it('says why a second seat of one account cannot be switched back on', async () => { + vi.mocked(api.enableHarnessCredential).mockRejectedValue(new Error('Failed to switch the credential on: subscription_account_taken')); + vi.mocked(api.fetchHarnessCredentials).mockResolvedValue([ + member({ id: 'TEST-seat-off', label: 'TEST-seat-off', type: 'codex', authMode: 'SUBSCRIPTION', enabled: false, identified: true }), + ]); + render(); + + fireEvent.click(within(await row('TEST-seat-off')).getByRole('button', { name: 'Switch on' })); + + expect(await screen.findByText('Another seat is already signed in to this account. Switch that one off first.')).toBeInTheDocument(); +}); diff --git a/spire-ui/src/components/SettingsHarnessCredentials.tsx b/spire-ui/src/components/SettingsHarnessCredentials.tsx index 23ada2a2..fc18ecaf 100644 --- a/spire-ui/src/components/SettingsHarnessCredentials.tsx +++ b/spire-ui/src/components/SettingsHarnessCredentials.tsx @@ -21,15 +21,21 @@ function state(member: HarnessCredentialView): { label: string; tone: string } { if (member.rejectedAt) return { label: 'Rejected', tone: 'chip danger' }; if (member.rateLimitedUntil && new Date(member.rateLimitedUntil) > new Date()) return { label: 'Resting', tone: 'chip warn' }; if (!member.enabled) return { label: 'Switched off', tone: 'chip' }; - // Before "Available", because it is not. A subscription is stored and safe, and no run can use it - // until the factory can select, inject and bill one. Rendering it as ready told the operator the - // factory was finished and left them to discover otherwise at the first build. - if (member.authMode === 'SUBSCRIPTION') return { label: 'Not usable yet', tone: 'chip warn' }; + // A seat whose account is unknown could be a second seat on one account, so no build uses it. + if (member.authMode === 'SUBSCRIPTION' && member.identified === false) return { label: 'Sign in again', tone: 'chip warn' }; + if (member.authMode === 'SUBSCRIPTION') return { label: 'Ready · subscription', tone: 'chip ok' }; return { label: 'Available', tone: 'chip ok' }; } const when = (value: string | null) => (value ? new Date(value).toLocaleString() : '—'); +/** Server refusals that have a sentence; anything else is shown as the server said it. */ +function sentence(message: string): string { + if (message.includes('subscription_account_taken')) + return 'Another seat is already signed in to this account. Switch that one off first.'; + return message; +} + /** * The keys a factory run may call the model with (FR-F12, ADR-031). * @@ -61,7 +67,7 @@ export default function SettingsHarnessCredentials() { async function act(action: () => Promise, message: string) { setBusy(true); setError(''); try { await action(); reload(message); } - catch (failure) { setError(String(failure instanceof Error ? failure.message : failure)); } + catch (failure) { setError(sentence(String(failure instanceof Error ? failure.message : failure))); } finally { setBusy(false); } } @@ -98,8 +104,9 @@ export default function SettingsHarnessCredentials() { load rather than burning one window.

- A Codex subscription can be signed in and is kept safely, but no run can use one yet: choosing - it, handing it to a run and recording its cost as zero are not built. Every run uses an API key. + A signed-in Codex subscription pays for builds whose setup says Pay with: subscription. Builds share + it, within the subscription's own limits. A build on it costs nothing per token; its token counts + are still recorded.

{signingIn && ( @@ -110,8 +117,7 @@ export default function SettingsHarnessCredentials() { // Says what actually happened. "Runs can now be paid by the subscription" was false: // nothing selects, injects or bills one yet, and the credential is deliberately // unreachable by every run until all three exist. - reload(`Saved ${label}. It is kept safely, and no run can use it yet — paying with a` - + ' subscription is not built. Runs keep using an API key.'); + reload(`Saved ${label}. Builds whose setup pays with a subscription can use it now.`); }} /> )} diff --git a/spire-ui/src/components/repositories/factory/BuildModelFields.tsx b/spire-ui/src/components/repositories/factory/BuildModelFields.tsx index d556d2e0..bac89f8b 100644 --- a/spire-ui/src/components/repositories/factory/BuildModelFields.tsx +++ b/spire-ui/src/components/repositories/factory/BuildModelFields.tsx @@ -1,7 +1,7 @@ import type { LlmModelView } from '../../../api'; import { TOKEN_TYPE_LABEL, unpricedTypesFor } from '../../../llmPricing'; import SettingField from '../../SettingField'; -import type { HarnessModels } from './buildDefaultsApi'; +import type { HarnessModels, PayWith } from './buildDefaultsApi'; /** Why a harness has no model list, as a sentence. The empty dropdown alone could mean any of these. */ const NO_LIST: Record = { @@ -24,8 +24,10 @@ export interface ModelChoice { value: string; label: string; blocked: string | n * before, and the screen says so rather than presenting it as the harness's own list. */ export function modelChoices(harness: string, known: HarnessModels | undefined, priced: LlmModelView[], - reported: Record): ModelChoice[] { - const unpriced = (model: LlmModelView) => (harness ? unpricedTypesFor(model, reported[harness]) : []); + reported: Record, payWith: PayWith = 'API_KEY'): ModelChoice[] { + // A subscription is not priced per token (M3.5 part F): no model is held back for a missing rate. + const subscription = payWith === 'SUBSCRIPTION'; + const unpriced = (model: LlmModelView) => (harness && !subscription ? unpricedTypesFor(model, reported[harness]) : []); if (known?.status === 'OK') { return known.offered.map(model => { const price = priced.find(entry => entry.name === model.slug); @@ -35,7 +37,7 @@ export function modelChoices(harness: string, known: HarnessModels | undefined, label: model.displayName === model.slug ? model.slug : `${model.displayName} (${model.slug})`, // Not offered as runnable until it can be paid for: an API-key run needs a rate for every type // the harness reports, and the save would refuse it anyway. - blocked: !price ? 'no price yet — add it in Settings → LLM' + blocked: subscription ? null : !price ? 'no price yet — add it in Settings → LLM' : missing.length ? `no price for ${missingLabel(missing)}` : null, }; }); diff --git a/spire-ui/src/components/repositories/factory/BuildStep.tsx b/spire-ui/src/components/repositories/factory/BuildStep.tsx index 7ec90ea4..bb26e417 100644 --- a/spire-ui/src/components/repositories/factory/BuildStep.tsx +++ b/spire-ui/src/components/repositories/factory/BuildStep.tsx @@ -3,7 +3,7 @@ import { fetchLlmModels, type LlmModelView } from '../../../api'; import SettingField from '../../SettingField'; import BuildModelFields, { modelChoices } from './BuildModelFields'; import FactoryStep from './FactoryStep'; -import { buildOptions, repositoryBranchHead, saveBuildDefaults, type BuildDefaults, type HarnessModels } from './buildDefaultsApi'; +import { buildOptions, repositoryBranchHead, saveBuildDefaults, type BuildDefaults, type HarnessModels, type PayWith } from './buildDefaultsApi'; interface Props { repositoryId: string; @@ -23,12 +23,15 @@ interface Props { * here once, they are offered as what this deployment can actually run, and the same refusals arrive * where they can be fixed. */ +/** How a build pays, in the words the setup shows. */ +const PAY_WITH_LABEL: Record = { API_KEY: 'an API key', SUBSCRIPTION: 'a Codex subscription' }; + export default function BuildStep({ repositoryId, defaults, open, setOpen, changed, reload }: Props) { const live = useRef(true); useEffect(() => { live.current = true; return () => { live.current = false; }; }, []); const [form, setForm] = useState({ baseBranch: defaults.baseBranch ?? '', harness: defaults.harness ?? '', model: defaults.model ?? '', - effort: defaults.effort ?? '', + effort: defaults.effort ?? '', payWith: (defaults.payWith ?? 'API_KEY') as PayWith, }); const [choices, setChoices] = useState<{ harnesses: string[]; models: LlmModelView[]; reportedTypes: Record; harnessModels: Record }>({ harnesses: [], models: [], reportedTypes: {}, harnessModels: {} }); @@ -62,7 +65,7 @@ export default function BuildStep({ repositoryId, defaults, open, setOpen, chang try { // An empty level is the model's own default, sent as null rather than as a level called "". await saveBuildDefaults(repositoryId, { expectedRevision: defaults.revision, ...form, effort: form.effort || null }); - if (live.current) changed(`Build setup saved: ${form.harness} on ${form.model}${form.effort ? ` (${form.effort})` : ''}, starting from ${form.baseBranch}.`); + if (live.current) changed(`Build setup saved: ${form.harness} on ${form.model}${form.effort ? ` (${form.effort})` : ''}, paid with ${PAY_WITH_LABEL[form.payWith]}, starting from ${form.baseBranch}.`); } catch (failure) { if (live.current) setError(String(failure instanceof Error ? failure.message : failure)); } finally { if (live.current) setBusy(null); } } @@ -70,7 +73,7 @@ export default function BuildStep({ repositoryId, defaults, open, setOpen, chang // The chosen PAIR, not just three non-empty fields. Choosing a model and then changing the harness // can leave a selection the dispatch refuses; the option goes grey, and Save used to stay live. const known = choices.harnessModels[form.harness]; - const offered = modelChoices(form.harness, known, choices.models, choices.reportedTypes); + const offered = modelChoices(form.harness, known, choices.models, choices.reportedTypes, form.payWith); const picked = offered.find(choice => choice.value === form.model); // An unresolved model is UNKNOWN, not complete: before the catalogue answers, and for a saved name the // catalogue no longer offers, there is nothing to judge — so Save waits rather than guessing. @@ -83,7 +86,7 @@ export default function BuildStep({ repositoryId, defaults, open, setOpen, chang actions={!editing && }> {defaults.revision > 0 - ?
{defaults.harness} · {defaults.model}{defaults.effort ? ` · ${defaults.effort}` : ''}starts from {defaults.baseBranch}
+ ?
{defaults.harness} · {defaults.model}{defaults.effort ? ` · ${defaults.effort}` : ''}starts from {defaults.baseBranch} · paid with {PAY_WITH_LABEL[(defaults.payWith ?? 'API_KEY') as PayWith]}
:

Not set, so every ticket has to be given a branch, a harness and a model by hand.

}

A prepared task copies these. Changing them here never changes a decision that is already open.

{editing &&
@@ -102,6 +105,12 @@ export default function BuildStep({ repositoryId, defaults, open, setOpen, chang {choices.harnesses.map(harness => )} {form.harness && !choices.harnesses.includes(form.harness) && } + + setForm(previous => ({ ...previous, model }))} setEffort={effort => setForm(previous => ({ ...previous, effort }))} /> diff --git a/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx b/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx index a19a134f..b7cf3c21 100644 --- a/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx +++ b/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx @@ -238,11 +238,31 @@ describe('build setup', () => { fireEvent.click(screen.getByRole('button', { name: 'Save build setup' })); await waitFor(() => expect(build.saveBuildDefaults).toHaveBeenCalledWith(repository.id, // No level chosen is the model's own default, sent as null rather than as a level called ''. - { expectedRevision: 0, baseBranch: 'main', harness: 'codex', model: 'TEST-model', effort: null })); + { expectedRevision: 0, baseBranch: 'main', harness: 'codex', model: 'TEST-model', effort: null, payWith: 'API_KEY' })); expect(await screen.findByText(/Build setup saved: codex on TEST-model/)).toBeInTheDocument(); }); /** A model with no output price is refused at dispatch, so it cannot be chosen here either. */ + // A subscription is not priced per token (M3.5 part F): an unpriced model can be picked and saved. + it('pays with a subscription without needing a price for the model', async () => { + const types = ['INPUT', 'CACHED_INPUT', 'CACHE_WRITE', 'OUTPUT', 'REASONING']; + vi.mocked(build.buildOptions).mockResolvedValue({ harnesses: ['codex'], reportedTypes: { codex: types }, + models: { codex: { status: 'OK', offered: [{ slug: 'TEST-unpriced-only', displayName: 'TEST unpriced only', + defaultEffort: 'medium', efforts: ['medium'], visible: true, priority: 1 }] } } }); + renderFactory(); + await open(); + fireEvent.change(await screen.findByLabelText('Base branch', field), { target: { value: 'main' } }); + fireEvent.change(await screen.findByLabelText('Harness', field), { target: { value: 'codex' } }); + fireEvent.change(screen.getByRole('combobox', { name: 'Pay with' }), { target: { value: 'SUBSCRIPTION' } }); + + expect(await screen.findByRole('option', { name: 'TEST unpriced only (TEST-unpriced-only)' })).toBeEnabled(); + fireEvent.change(screen.getByRole('combobox', { name: 'Model' }), { target: { value: 'TEST-unpriced-only' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save build setup' })); + + await waitFor(() => expect(build.saveBuildDefaults).toHaveBeenCalledWith(repository.id, expect.objectContaining({ + model: 'TEST-unpriced-only', payWith: 'SUBSCRIPTION' }))); + }); + // Every option disabled: the select ignores clicks and keys, which the operator read as broken. it('says why no model can be picked when every one lacks a price', async () => { const types = ['INPUT', 'CACHED_INPUT', 'CACHE_WRITE', 'OUTPUT', 'REASONING']; @@ -356,7 +376,7 @@ describe('build setup', () => { fireEvent.click(screen.getByRole('button', { name: 'Use the model default' })); fireEvent.click(screen.getByRole('button', { name: 'Save build setup' })); await waitFor(() => expect(build.saveBuildDefaults).toHaveBeenCalledWith(repository.id, - { expectedRevision: 2, baseBranch: 'main', harness: 'codex', model: 'TEST-model', effort: null })); + { expectedRevision: 2, baseBranch: 'main', harness: 'codex', model: 'TEST-model', effort: null, payWith: 'API_KEY' })); }); // A level belongs to one harness's list: switching away and back must not bring it back unseen. diff --git a/spire-ui/src/components/repositories/factory/buildDefaultsApi.ts b/spire-ui/src/components/repositories/factory/buildDefaultsApi.ts index 624fe260..965067d2 100644 --- a/spire-ui/src/components/repositories/factory/buildDefaultsApi.ts +++ b/spire-ui/src/components/repositories/factory/buildDefaultsApi.ts @@ -9,9 +9,13 @@ export interface BuildDefaults { model: string | null; /** The thinking level, or null for the model's own default. Absent from an older server. */ effort?: string | null; + /** How builds pay: per token with an API key, or on a signed-in subscription. Absent from an older server. */ + payWith?: PayWith | null; updatedBy: string | null; updatedAt: string | null; } +export type PayWith = 'API_KEY' | 'SUBSCRIPTION'; + /** One model a harness can run, as its agent image declares it. */ export interface HarnessModel { slug: string; @@ -61,7 +65,7 @@ const base = (repository: string) => `/api/repositories/${encodeURIComponent(rep export const buildDefaults = (repository: string) => read(`${base(repository)}/build`); export const buildOptions = (repository: string) => read(`${base(repository)}/build/options`); -export const saveBuildDefaults = (repository: string, input: { expectedRevision: number; baseBranch: string; harness: string; model: string; effort: string | null }) => +export const saveBuildDefaults = (repository: string, input: { expectedRevision: number; baseBranch: string; harness: string; model: string; effort: string | null; payWith: PayWith }) => read(`${base(repository)}/build`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(input) }); export const repositoryBranchHead = (repository: string, branch: string) => read(`${base(repository)}/branch-head?branch=${encodeURIComponent(branch)}`); diff --git a/spire-ui/src/components/work-items/workReasons.ts b/spire-ui/src/components/work-items/workReasons.ts index a5281ffa..3670eccb 100644 --- a/spire-ui/src/components/work-items/workReasons.ts +++ b/spire-ui/src/components/work-items/workReasons.ts @@ -22,6 +22,10 @@ const REASONS = new Map(Object.entries({ model_name_invalid: 'That model name has characters a run cannot pass to the agent.', catalogue_unavailable: 'The model catalogue could not be read, so whether that model may run is unknown. The build was not started.', model_disabled: 'That model is switched off in the catalogue, so a run cannot call it.', + subscription_not_signed_in: 'No Codex subscription is signed in for this harness. Sign one in under Settings → Harness credentials, or pay with an API key.', + subscription_unavailable: 'No signed-in subscription can be used: each is switched off, refused, resting, or needs to be signed in again. The build was not started.', + subscription_unreadable: 'The stored subscription sign-in could not be read. Sign in again under Settings → Harness credentials.', + pay_with_invalid: 'Choose how builds pay: an API key or a subscription.', model_not_run_by_harness: 'The agent image for that harness does not run this model. Choose one of the models it offers in the build setup.', effort_not_offered: 'That model does not offer this thinking level. Choose one of its own levels in the build setup.', effort_unverifiable: 'The models this harness runs are not known yet, so the thinking level cannot be checked. Use the model default, or wait for the list.',