Conversation
MAPCO-11434. Adds a second authentication mode so a deployment can run against a Claude subscription token rather than a metered Anthropic API key, and wires the Secret and the docs that were the unmet half of the first acceptance criterion. Which mode is in use is explicit configuration, `MODEL_AUTH`, and is never inferred from whichever credential happens to be set. Both credentials look alike to the SDK and bill completely differently, so inferring would make the billed party a property of the pod's environment rather than of a decision — and the failure is silent, because a run that quietly spends someone's personal quota looks exactly like a working one. One mode's credential is never used for the other; the worker refuses to start and names the one it found, since setting a token and forgetting the mode is the mistake an operator actually makes. An unrecognised mode also refuses rather than falling back to the default. `modelEnv` now scrubs every credential and injects exactly one, the configured mode's. Previously it injected ANTHROPIC_API_KEY over a partially-scrubbed environment; with two modes reading different variables, leaving the unused one in place would let the SDK pick the other. Chart: MODEL_AUTH plus a secretKeyRef for whichever credential the mode needs, from worker.modelSecretName. README documents both variables and the mode. subscription mode is reachable, not blessed. Anthropic's Agent SDK documentation states that claude.ai login and its rate limits may not be used for products built on the Agent SDK unless previously approved, so setting the mode asserts this deployment has that approval — code cannot check it. Three consequences no code can fix are recorded in README.md and credential.ts: the quota is shared with that person's own interactive use, runs are attributed to them rather than to the worker, and the pod crash-loops when the token expires. api-key remains the default for those reasons. Renames apiKey.ts to credential.ts, since it no longer only reads a key.
|
🎫 Related Jira Issue: MAPCO-11434 |
…fault MODEL_AUTH takes 'sdk' rather than 'subscription', and a deployment that sets nothing now gets it. This worker is meant to run on a Claude subscription, and a default of api-key meant every manifest had to remember to say so. The cost is that the safe direction is no longer the default. A deployment or a laptop carrying only ANTHROPIC_API_KEY used to work and now refuses to start, naming the credential it found — which is the loud half of the trade, and better than billing an account nobody chose.
Three things, all of which had to happen before the MODEL_AUTH block could be trusted. **The chart could not render at all.** `helm/values.yaml` set `mclabels.component: worker`, and the subchart's values.schema.json restricts that field to frontend | backend | database | proxy-server | cache-server | infrastructure. Every `helm template` and `helm lint` failed on it, with or without the dependency present — this would have failed CI and blocked a deploy regardless of the auth work. Set to `backend`, with the allowed set named in a comment. That label feeds org-wide dashboards and ownership tooling, so change it if MapColonies conventionally tags pollers as `infrastructure`. **The MODEL_AUTH block is now verified rather than assumed.** Rendered in both modes against the real mclabels chart and parsed with a YAML parser: the expected variable is present at the right depth in containers[0].env and the other branch's variable is absent. `helm lint` passes. **Trailing whitespace in deployment.yaml** leaked into rendered output, landing on the new block's last line as `key: apiKey `. Stripped. The chart now defaults to `modelAuth: sdk` to match DEFAULT_AUTH_MODE, and the secretKeyRef key follows the mode — `oauthToken` for sdk, `apiKey` for api-key. README records how to fetch mclabels. Anonymous pull from the OCI registry is refused (401), so a developer needs `az acr login --name acrarolibotnonprod` or `helm registry login` first; that is documented rather than presented as solved.
Compared against mapproxy-api, cleaner, exporter-trigger, geojson-viewer and the rest. Two things were wrong and one is now documented. **prometheus.enabled is false.** Almost every chart in the org sets it true, and this one must not: the worker is outbound-only — no Service, no Route, no container port — and although containerConfig.ts builds a prom-client Registry, nothing ever serves it over HTTP. Left true, mclabels stamped `prometheus.io/scrape: "true"` with port 8080 and /metrics onto the pod, giving Prometheus a target that could only ever fail. geojson-viewer, the other chart with nothing to scrape, does the same. **component: backend is confirmed, not guessed.** Every non-frontend chart in the org uses it, and `worker` appears in no schema. The earlier open question about `infrastructure` is settled: nothing uses it. **The environment label is documented.** Only `mclabels.environment` feeds `mapcolonies.io/environment` — `global.environment` and a top-level `environment` are both ignored by the subchart — and leaving it unset stamps the literal string `undefined`. Added the commented `#environment:` line the other charts carry, so a deploy-time values file supplies it. `gisDomain` stays absent: the raster-owned charts carry it, the common-owned tooling charts do not, and this is one of those.
A `node_modules` symlink pointing at the main checkout was committed with
this branch. `.gitignore` listed `node_modules/` with a trailing slash, which
matches a directory but not a symlink, so `git add -A` picked it up.
On CI the symlink dangles, which broke two jobs:
- eslint: `npm ci` could not populate it, so lint-action fell through to
`npx eslint` and failed with 'npx canceled due to missing packages'.
- build_docker_image: 'cannot replace to directory .../node_modules with file'.
Drop the entry and drop the trailing slash so the ignore rule covers both.
The README still said the current slice was MAPCO-11431 and described a worker that writes no code. It now covers what is actually on this branch. New section on implementing a ticket: why the model's tool surface is the security control rather than the prompt, why a write counts only when its tool_result comes back (reading the attempt alone certified diffs that did not exist), and why the test command is taken off the pristine clone before the first hand-off so the model cannot rewrite what grades it. Two chart decisions added to "Things that look wrong but aren't": this chart sets prometheus.enabled false where almost every other one sets it true, because there is nothing listening to scrape; and mclabels.environment is the only key that feeds the environment label, with global.environment and a top-level environment both ignored and the unset case stamping the literal string "undefined". Two known gaps recorded rather than left for a reader to discover: nothing is wired into runCycle, so this is a library with tests and not a behaviour the deployed worker has; and DescriptionPort has no implementation, so every ticket is refused before the first model turn.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #7 — review that first.
Adds a second authentication mode so the worker can bill a Claude subscription token instead of a metered Anthropic API key, and wires the Secret and README rows that were the unmet half of #7's first acceptance criterion.
Anthropic's Agent SDK documentation states:
This PR makes the mode reachable, not permitted. Setting
MODEL_AUTH=subscriptionasserts that this deployment has that approval; no code can check it.api-keystays the default.Three consequences no code can fix, recorded in both
README.mdandcredential.ts:The mode is explicit, never inferred
The design decision worth reviewing.
MODEL_AUTHis read from configuration; the worker does not pick whichever credential happens to be present.Both credentials look alike to the SDK and bill completely differently. Inferring would make the billed party a property of the pod's environment rather than of a decision, and the failure mode is silent — a run that quietly spends someone's personal quota looks exactly like a working one. So:
modelEnvnow scrubs then injectsIt previously injected
ANTHROPIC_API_KEYover a partially-scrubbed environment. With two modes reading different variables, leaving the unused one in place would let the SDK pick the other — so every credential is scrubbed and exactly one goes back in, making the choice singular by construction. Tested in both modes.What this closes on #7
helm/templates/deployment.yamlgetsMODEL_AUTHplus asecretKeyReffor whichever credential the mode needs (apiKeyoroauthTokenfromworker.modelSecretName), and the README documents all three variables.Note the criterion's wording is now strained:
subscriptionmode is an interactive-login credential, obtained withclaude setup-token. It is opt-in, off by default, and the reason is in the file — but if you'd rather the ticket's wording hold literally, this PR is the thing to drop.Verification
tsc --noEmitclean,eslintclean, 202 tests pass (17 files) on this branch.Not verified: the chart change.
helm templatecannot run here —Error: found in Chart.yaml, but missing in charts/ directory: mclabels, the known gap the README already records. The template edit is unrendered. Worth a second pair of eyes on the{{- if eq .Values.worker.modelAuth "subscription" }}branch.apiKey.tsis renamed tocredential.ts, since it no longer reads only a key.Refs: MAPCO-11434
Update: renamed to
sdk, now the default, and the chart is verifiedThree commits since the description above was written.
MODEL_AUTHtakessdk, notsubscription, and is the default. A deployment that sets nothing now authenticates with a Claude subscription token. The trade is explicit: the safe direction is no longer the default, so a deployment or laptop carrying onlyANTHROPIC_API_KEYrefuses to start and names the credential it found, rather than quietly billing an account nobody chose.api-keyis one variable away.Since
sdkis the default, the approval caveat at the top now applies to running this worker at all, not to an opt-in.The chart could not render — and not because of this PR.
helm/values.yamlsetmclabels.component: worker, which the subchart'svalues.schema.jsonrejects (allowed:frontend | backend | database | proxy-server | cache-server | infrastructure). Everyhelm templateandhelm lintfailed on it regardless of the auth work; this would have failed CI and blocked a deploy. Set tobackend.The
MODEL_AUTHblock is now verified, not assumed. Rendered against the realmclabelschart in both modes and parsed with a YAML parser — the expected variable is present at the right depth incontainers[0].env, the other branch's variable is absent, and indentation matches the surrounding entries.helm lintpasses with only theicon is recommendedinfo.Also fixed: trailing whitespace in
deployment.yamlleaked into rendered output, landing on the new block's last line askey: apiKey.mclabels is documented, not solved. Anonymous pull from
oci://acrarolibotnonprod.azurecr.io/helm/infrais refused with a 401. A developer needsaz acr login --name acrarolibotnonprod(orhelm registry login) first; the README now says so, replacing the "fails without registry access" known-gap line. Nothing vendored is committed —helm/charts/is already covered by the root.gitignore.Gate:
tsc --noEmitclean,eslintclean, 204 tests pass,helm lintpasses.