Treat an unclear SHADOW_MODE as on, instead of posting for real

[jerelvelarde]

Recording: the flag under nine different spellings — captured from this branch's isShadowMode() actually running, ending on the log line it emits. Linked rather than embedded: outpost is private, so raw.githubusercontent.com returns 404 to GitHub's own image proxy and an inline ![]() renders broken. See What is not covered.

The problem

Shadow mode is the flag that lets Outpost run alongside an incumbent without double-posting at real reporters. With it on, answers are generated, scored and recorded, but never posted to Discord or GitHub. It is the one switch standing between a parallel-run validation window and machine-generated text arriving in a stranger's support thread.

Every place that read it tested process.env.SHADOW_MODE === 'true'. So an operator who sets SHADOW_MODE=TRUE, or =1, or =yes — three spellings any of us would write without thinking — gets a worker that reads "not shadow mode" and posts for real. There is no error, no warning, and nothing in the logs to distinguish it from a deliberate live run. The first evidence is a reply on a community thread.

The codebase already conceded the ambiguity in its own docblock, which describes the behaviour as "When SHADOW_MODE=true" while the comparison it documents accepts exactly that one spelling and silently rejects the rest. And there were three separate copies of the predicate — in the queue's AI handler, in the onboarding digest, and in the Discord bot's helper — all three with the same comparison. A safety flag failing open, in triplicate.

The approach

One isShadowMode() in shared, and it fails closed. Recognized off values (false, 0, no, off, empty) are off. Recognized on values (true, 1, yes, on) are on. Anything else that is set is treated as on, and logged with the value it rejected.

The asymmetry is the whole design, and it is not free. A false positive costs a parallel-run window where nothing gets posted and somebody works it out from a warning in the logs. A false negative posts generated text at real people under the flag that existed to prevent that. Those are not comparable, so the tie is broken toward silence — which does mean a typo in SHADOW_MODE now quietly stops the bot answering, and you find that out by reading logs rather than by watching behaviour. That is the trade, and it is the right way round.

Unset still means off. Shadow mode is opt-in; defaulting an absent variable to on would make a fresh deployment silently answer nobody, which is a worse failure than the one being fixed. The fail-closed rule applies only to values that are set — those represent an operator trying to say something, and the safe reading of an unclear instruction is the one that posts nothing.

Central, not per-call-site, as #157 suggested. Three copies existed and all three had the identical bug, which is precisely why it survived this long; the Discord bot's helper now re-exports the shared one instead of keeping its own. Values are trimmed and case-folded before comparison, so " True " behaves like true.

What is not covered

• Only SHADOW_MODE. The === 'true' pattern appears on other boolean env vars in this repo. Those are not safety flags, so they are not urgent, and folding them in would make this diff about something else.
• No startup assertion and no dashboard surface. Nothing announces the resolved mode at boot or shows it in the UI, so an operator still has to read worker logs to confirm which mode is live. That is the real remaining gap and it wants its own change.
• Two test suites mock @copilotkit/outpost/shared wholesale, so the mock had to grow an isShadowMode entry. It delegates to the same env check rather than returning a constant — a hardcoded false would have left those files' existing SHADOW_MODE tests asserting nothing. That is duplicated logic in test scaffolding, and worth knowing about.
• The recording is a link, not an inline image, for the private-repo reason above. The GIF lives on the orphan pr-media branch so the diff here stays pure code.
• The other two items in AI response handler: three small correctness gaps (escalation reporting, null description, SHADOW_MODE compare) #157 are already fixed and this PR does not touch them. I verified both against merged main rather than assuming: the null-description-stringifies-as-"null" concatenation and the escalated: true-when-the-enqueue-threw report were both closed by feat(queue): one AI answer per ticket, arbitrated by the database #191. AI response handler: three small correctness gaps (escalation reporting, null description, SHADOW_MODE compare) #157 can be closed when this lands.
• Withheld drafts are indistinguishable from delivered replies (replayed as assistant history, no DB marker) #148 is untouched. Also verified while I was here, and worth recording: feat(queue): one AI answer per ticket, arbitrated by the database #191's responseState does not supply the marker Withheld drafts are indistinguishable from delivered replies (replayed as assistant history, no DB marker) #148 needs. A delivered low-confidence answer also ends up ESCALATED, so ESCALATED cannot mean "the reporter never saw this". Withheld drafts are indistinguishable from delivered replies (replayed as assistant history, no DB marker) #148 still needs its own suppressed column.

Verification

packages/outpost: 63 files, 1099 tests passing, against 1075 on main — the delta is exactly the new file and nothing changed status. apps/discord-bot unchanged at 66. Typecheck: 48 errors in queue, 0 in shared, identical to main (the 48 are the known unbuilt-workspace resolution errors, which CI's pnpm build step resolves). Prettier clean on every changed file.

shared/src/__tests__/shadow-mode.test.ts — 24 tests. Holds down the nine previously-fail-open spellings, the explicit off values, unset meaning off, and that an unrecognized value is both honoured as on and reported with the value in the message. Reverting the function body to === 'true' fails 12 of them.

Merge notes

shared/src/index.ts gains one export line, and queue/src/handlers/ai-response.ts gains one symbol on an existing import. #150 (feat/slack-ticket-mirror-impl) also edits ai-response.ts and was just brought up to date with main, so expect a one-line import conflict there depending on which lands first.

[linear-code]

CPK-7929 2. #148 + #157 — record what was posted, not the draft (blocked by PR #191)

Both live in packages/outpost/queue/src/handlers/ai-response.ts, which is touched by PRs #187, #191 and #150. Start after #191 merges or the work gets rewritten under you.

#148 — the DB records the draft, not what was posted

Message.content holds the raw draft while the thread receives formatted.text. Under suppression the two diverge completely, so a withheld draft is indistinguishable from a delivered reply and an audit reads the wrong answer. This is the one that makes every other quality metric untrustworthy — if we cannot tell what the reporter saw, we cannot measure whether it was good.

Note that #191 introduces a delivery state machine (PENDING → DELIVERED | ESCALATED) plus a deliveryConfirmed column. Check whether that already provides the marker #148 needs before designing a second mechanism.

#157 — three correctness gaps in the same handler

• Reports escalated: true even when the escalation enqueue threw — same class of "the record disagrees with reality" as Withheld drafts are indistinguishable from delivered replies (replayed as assistant history, no DB marker) #148
• A null description stringifies as the literal "null" into the classifier prompt
• SHADOW_MODE === 'true' fails open, so TRUE / 1 / yes post to real community surfaces

The third is the urgent one and is arguably safety rather than quality. #191's description claims the null-description concatenation is already fixed there — verify against the merged diff before re-fixing it.

---

UNBLOCKED 2026-08-19 — PR #191 merged at 17:45Z, so the ai-response.ts contention that gated this ticket is gone. main is at b11aafb.

Start with the check this ticket already flagged rather than designing fresh: #191 introduced the PENDING → DELIVERED | ESCALATED state machine plus a deliveryConfirmed column, and one or both may already supply the marker #148 needs to distinguish a withheld draft from a delivered reply. Read what landed before adding a second mechanism.

Same for #157: #191's description claimed the null-description-stringifies-as-"null" concatenation was fixed there. Verify against the merged diff before re-fixing it. The SHADOW_MODE === 'true' fail-open is the part most likely still outstanding, and it is the urgent one — TRUE/1/yes post to real community surfaces.

Note that #150 also still touches ai-response.ts (CPK-7936), so coordinate if both move at once.

Review in Linear

[jerelvelarde]

Re-verified against current main rather than as-opened, since this has been sitting for 19 days and is now 28 commits behind. GitHub will not let me approve my own PR, so this is a status comment — it still needs a reviewer, and it is the only thing in the queue that is a posting-safety switch.

It has not rotted. Merges cleanly onto current main with no conflicts, including the ai-response.ts import line the merge notes warned about. Full package suite on the merge result: 1222 tests, 66 files, all passing — against 1198/65 on main, so the delta is exactly the new file and nothing changed status. shadow-mode.test.ts is 24 of those.

One correction to my own PR body. It claims reverting isShadowMode to === 'true' fails 12 tests. Ran it: 15 fail, all in shadow-mode.test.ts. Understated rather than overstated, but the count is wrong and I have been holding other PRs to theirs.

What a reviewer should push on, so this is not a rubber stamp:

• The asymmetry is the whole design and it is not free: a typo in the value now silently stops the bot answering, and you find out by reading logs. The PR argues that trade is right way round. That is the thing to agree or disagree with; everything else is mechanical.
• Unset still means off, deliberately — worth confirming that is what you want, because it means a fresh deployment posts for real unless someone sets the flag.
• The two test suites that mock @copilotkit/outpost/shared wholesale now carry an isShadowMode entry that delegates to the same env check rather than returning a constant. That is duplicated logic in scaffolding, and the alternative (a hardcoded false) would have left those files' existing SHADOW_MODE tests asserting nothing. Worth a look.

Still not covered, unchanged: no startup assertion and no dashboard surface, so an operator confirms the live mode by reading worker logs. That is the real remaining gap and it wants its own change.

@NathanTarbert — this is the one I would put ahead of #248 in your queue if you have a slot. It is 9 files, 156 lines, verified green on current main, and it is the switch between a parallel-run window and generated text landing in a stranger's support thread.

[NathanTarbert]

Hey @jerelvelarde,

Read through this one properly and ran it rather than reading it. The asymmetry call is right, and the docblock in shared/src/shadow-mode.ts explains the trade instead of just asserting it, which is the part that will keep this decision legible in six months. shadow-mode.test.ts is also a real test: reverting the function body to === 'true' fails 15 of its 24, not the 12 the description claims.

Three things I'd want before this lands, then some smaller ones.

The gates the fix protects aren't actually covered. I reverted queue/src/handlers/ai-response.ts:824 and queue/src/handlers/onboarding-digest.ts:119 back to process.env.SHADOW_MODE === 'true' and the full suite still passed 1099/1099. Both test files vi.mock('@copilotkit/outpost/shared', ...) with a hand-rolled copy of the predicate (queue/src/__tests__/ai-response.test.ts:76-84, queue/src/__tests__/onboarding-digest.test.ts:15-22), so the handlers never execute the real helper. Same at apps/discord-bot/src/lib/shadow-mode.ts:16-19 — swapping the re-export back for the inline comparison keeps all 66 discord-bot tests green. Spreading await vi.importActual('@copilotkit/outpost/shared') into the mocks fixes it, plus one test per handler that sets SHADOW_MODE='TRUE' and asserts nothing was posted.

The cutover script is a fourth read path, and this change makes it disagree with the workers. scripts/cutover/execute-cutover.ts:171-178 still evaluates currentValue !== 'true'. With SHADOW_MODE=TRUE, the workers are in shadow mode and post nothing, while disableShadowMode() returns PASS with "Shadow mode already disabled (SHADOW_MODE=TRUE)" and never prints the ACTION REQUIRED line — so a cutover run reads green while Outpost answers nobody. Before this PR the script and the gates were wrong in the same direction and agreed; now they diverge. Importing isShadowMode there would give the checker and the gate one definition.

The docker-compose.override.yml looks like it rode along. Compose auto-merges that filename, so it isn't opt-in: ports: !override ['5437:5432'] replaces 5432:5432, and .env.example:2's connection string then can't reach the container, so prisma/db:push/db:studio all fail with ECONNREFUSED on a fresh clone. Worth dropping from the PR and adding to .gitignore, which is presumably why it got staged in the first place.

Smaller ones:

• shared/src/shadow-mode.ts:24 keeps '' in EXPLICITLY_OFF, and since :46 trims, ' ' lands there too. A var cleared rather than deleted, or a ${SHADOW_MODE:-} passthrough, arrives as '', reads as off, and posts for real. Nothing in the repo templates it that way today, so it's latent rather than live — but it's the one set-but-unclear value going the opposite way from the stated rule. Either drop '' from the set or give the docblock a line on why absence and empty string are the same thing.
• isShadowMode() runs per AI_RESPONSE job and per Discord event, so SHADOW_MODE=maybe reprints the same warning forever. Per-call resolution is right (caching would break the env-mutating tests), but memoizing the last-warned raw value would keep the signal. shadow-mode.test.ts:50 asserts toHaveBeenCalledTimes(1) and would move with it.
• docs/deployment.md:193-213 and .env.example:43-44 still describe the old semantics. .env.example is where someone writing TRUE would look, and deployment.md:212 is the natural spot to point at isShadowMode() so the next outbound path doesn't add a fifth copy.
• The numbers in the description are behind main — origin/main is at 65 files / 1198 tests after ci: run eslint and a format check on every pull request #250 landed the ESLint flat config. I ran that config over your changed files and it's clean apart from a pre-existing unused threadId at apps/discord-bot/src/lib/shadow-mode.ts:130, so the merge-forward looks mechanical, just worth re-measuring.

I also grepped for a fifth production read path and there isn't one — the three you convert plus the cutover script is the whole set. And the barrel addition is fine against the browser-safety convention in shared/src/index.ts:16-20: pure function, env read at call time, nothing in apps/web calls it.

Happy to be wrong on the empty-string one if you'd rather treat it as absence deliberately. The mock and cutover items are the two I'd call blocking.

[NathanTarbert]

Jerel, this is the right fix and the right default. The asymmetry argument in the description is the part I'd want kept verbatim somewhere permanent: naming what each failure direction actually costs, rather than asserting that one is "safer", is what makes the unusual default defensible six months from now. The unset carve-out is also correct, and I'm glad you argued it rather than quietly picking one.

A few things before it goes in. Most of them are the same shape, which is worth saying up front: the predicate is now right, but very little outside shadow-mode.test.ts would notice if it stopped being right.

The three that need doing

docker-compose.override.yml looks like local scaffolding that rode along. Compose auto-loads that exact filename, and ports: !override replaces the base list instead of adding to it, so docker-compose.yml's 5432:5432 goes away and postgres publishes only 5437. The README's setup path (docker compose up -d → cp .env.example .env → pnpm db:push) points at localhost:5432, so a fresh clone of this branch gets connection refused. Worth deleting from the diff and adding to .gitignore while we're here, since the override file is per-developer by convention and the next one would otherwise get committed too. (!override also wants Compose ≥ 2.24.4, and the README lists the prerequisite as just "Docker".)

The two vi.mock factories hand-roll the predicate. This is the one I'd most want changed, because the diff removes three copies of the comparison and the mocks add two back — and they've already drifted, with no EXPLICITLY_ON and no warn. The comments describe them as delegating to the real env check, which is the behaviour we want; a partial mock gets you that literally:

vi.mock('@copilotkit/outpost/shared', async (importOriginal) => ({
    ...(await importOriginal<typeof import('@copilotkit/outpost/shared')>()),
    computeFunnelMetrics: vi.fn().mockReturnValue({ /* unchanged fixture */ }),
}));

Not a guess about whether it works here — apps/discord-bot/src/__tests__/shadow-mode.test.ts:13-19 already does it against this same specifier, as do apps/web/src/__tests__/postmark-webhook.test.ts:45 and two others, and packages/outpost/vitest.config.ts:14 aliases the specifier to source so nothing needs building first. I tried it on onboarding-digest.test.ts keeping only computeFunnelMetrics stubbed and it came out 15/15. ai-response.test.ts's mock is bigger, but the spread makes the override list strictly narrower — its AI_CONFIDENCE and MAX_JOB_ATTEMPTS stubs look like they already match the real constants, worth confirming before dropping them.

The fix isn't fenced at the call sites yet. Every shadow test in both queue suites sets SHADOW_MODE='true', which is the one value that behaves identically before and after this change. I reverted shadow-mode.ts to === 'true' locally to see what would catch it: 121 of 145 tests stayed green, and the 24 that failed were all in the new shared file. So ai-response.ts:824 could go back to a raw comparison tomorrow and CI would pass.

One case per handler with a non-canonical spelling closes it, and it falls out of the mock change for free (right now such a test would exercise the mock's private copy, not the shipped function):

it.each(['1', 'TRUE', 'yes', 'on', ' true '])(
    'does not post to Discord when SHADOW_MODE=%j',
    async (value) => { /* … */ },
);

ai-response.ts is the higher-value one of the two, since that's the path that reaches real Discord and GitHub surfaces.

Two smaller ones in the same family

EXPLICITLY_ON isn't pinned by anything. I shrank it to new Set(['true']) and all 24 tests still passed, because a dropped member still comes back true through the fail-closed branch and the ON it.each only asserts the boolean. Adding expect(warn).not.toHaveBeenCalled() to that block makes the mutant fail, and it happens to pin the normalization at the same time — right now .trim() and .toLowerCase() are each held down by exactly one OFF case, so most of the ON list ( 'TRUE', 'tRuE', ' true ', 'YES', 'ON') passes for the wrong reason.

And there's a fourth reader the sweep missed, which changes the story slightly: scripts/cutover/execute-cutover.ts:171 still has its own currentValue !== 'true'. With SHADOW_MODE=TRUE the worker now correctly treats shadow mode as on, while the cutover script reports "Shadow mode already disabled (SHADOW_MODE=TRUE)" and passes the step. Same fail-open, moved to the cutover path. Either pull it onto the helper here or say so in the comment, because as written ("a shared one is the only version of this that stays fixed") a future maintainer won't go looking.

Things I'd like your read on rather than just changing

A boot log of the resolved mode. You call the missing startup assertion out yourself, and I agree it wants its own change, but there's a cheap piece of it that belongs here: right now a healthy worker gives an operator no way to answer "which mode am I in?" without waiting for a job and inferring it from which line got printed. apps/worker/src/index.ts:42-55 already establishes fail-fast-at-boot for the sync engine, so one console.log of the resolved mode next to it would fit the existing shape. The stronger version — throwing when the var is absent in production — is a real behaviour change and I'd rather discuss it than assume; the case for it is that staging is documented as SHADOW_MODE=true, so a variable that fails to carry over to a new replica is a silent fail-open with no output at all, which is both likelier and quieter than the TRUE case this PR fixes.

'' sitting in EXPLICITLY_OFF. A declared-but-cleared Railway variable, or a .env line with nothing after the =, currently reads as "post for real" with no warning. That's arguably against the module's own rule that anything set is an operator trying to say something. Dropping '' would send it to the unrecognized branch (on, plus the warn), and ' ' would follow. I can see the other side — nobody clears a value meaning "off", so maybe it's genuinely noise — happy to leave it if you'd rather.

The header's fail-closed claim. The module docblock states it unqualified, and the unset exception only appears in the function doc fifteen lines down. Someone skimming the header concludes that a missing variable is the safe case, which is the belief this module exists to kill. Scoping it to "values that ARE set fail closed" in the header would let the function doc shrink.

Before the check runs: the branch is 31 commits behind, and 1fedbc8 added the eslint and format checks to every PR after this one branched. Worth a rebase so those run against the current config. Relatedly, the two reformatted hunks in the discord-bot file aren't noise — I checked, and the base revision fails prettier --check while the branch passes, so touching that file makes them mandatory. Might be worth a line in the description so nobody spends time on them.

Separate, and not yours to fix here

Two things I found while checking whether the sweep was complete, both pre-existing. apps/teams-bot/src has no SHADOW_MODE reference anywhere, and handlers/message.ts:120 posts the acknowledgment card at ingest — so a staging Teams channel gets "an AI answer is coming" while the worker correctly withholds the answer. .env.example:41-42 says the flag "covers EVERY platform (GitHub, Slack, Teams)", which isn't true today. There are also three unreferenced postAiResponse helpers (apps/github-app/src/lib/github-poster.ts:36, slack-bot/src/lib/slack-poster.ts:16, teams-bot/src/lib/teams-poster.ts:26), each fully formed and one import away from bypassing the worker's gate. I'll open issues for both rather than grow this diff.

While the docs are open: docs/deployment.md:212 still says "check SHADOW_MODE before posting", which is the instruction that produced the three copies. Naming isShadowMode() there instead would be a one-line change worth folding in.

Checked and clean

For the record, so nobody re-does it: the subpath import resolves without any new dependency (apps/discord-bot already imported @copilotkit/outpost/shared before this PR, and turbo.json's build.dependsOn: ["^build"] covers the Dockerfile's prune-then-build). The barrel addition stays browser-safe — process.env is read lazily inside the function, and while 15 "use client" files in apps/web import that barrel, none calls this. The new test file is inside the tsc program and typechecks clean. And the env restore hygiene in the new test is the good version: capturing in beforeEach and delete-ing rather than assigning undefined is exactly the trap ai-response.test.ts:110-122 documents, and it's nice to see it not re-trodden.

One genuinely good thing worth calling out: the test asserts on the warning itself, both that it fires once and that the message carries the offending value. Most diagnostics get added and never tested, which is how they quietly disappear a year later.

[NathanTarbert]

Filed the two out-of-scope items from the review so they're linkable rather than living in a comment:

• Keep the Teams ack card out of shadow-mode runs #258 — the Teams ack card at apps/teams-bot/src/handlers/message.ts:120 posts at ingest with no shadow gate, and .env.example:41-42 claims Teams coverage the flag doesn't have. Depends on this PR for the shared helper.
• Retire the three unreachable postAiResponse helpers #259 — the three unreachable postAiResponse helpers in the github-app, slack-bot and teams-bot libs, each one import away from bypassing the worker's gate. Delete or guard; I lean delete.

Both are pre-existing and neither should grow this diff. The one doc line I'd still fold in here is docs/deployment.md:212, since it's the instruction that produced the three copies you're consolidating.

[jerelvelarde]

All five of the actionable ones are in, plus my read on the three you asked about. Rebased onto current main first, so the eslint and format checks from 1fedbc8 run against this. d4fdae0.

You were right that the shape was the problem rather than the predicate. The fix now has teeth at the call sites:

Mutation	Before	After
isShadowMode → === 'true'	24 failed, 1 file	29 failed, 3 files
EXPLICITLY_ON → ['true']	0 failed	5 failed
'' back into EXPLICITLY_OFF	n/a	2 failed
unset flipped to ON	(unpinned)	23 failed, 3 files

Baseline 1234 passing, up from 1222.

The three

docker-compose.override.yml — gone from the diff and added to .gitignore with the reason, so the next per-developer one does not get committed either. You were right about !override: it replaced the base list rather than adding to it, so 5432 disappeared and the README's pnpm db:push path failed on a fresh clone.

Both vi.mock factories are partial now. The importOriginal spread works exactly as you said. One thing worth recording, because it is the answer to your "worth confirming before dropping them": AI_CONFIDENCE, MAX_JOB_ATTEMPTS, BACKOFF_BASE_MS and BACKOFF_MAX_MS are all byte-identical to shared/src/constants.ts, so the spread supplies them and I dropped the stubs. But calculateBackoff has to stay overridden — the real one at shared/src/utils.ts:40 adds Math.random() * BACKOFF_BASE_MS of jitter, so taking it from the spread would make any assertion on a retry delay nondeterministic. That is now said in the comment rather than left as a puzzle.

The fence. it.each(['1', 'TRUE', 'yes', 'on', ' true ', 'YES']) on both handlers, ai-response included since that is the path that reaches real surfaces. It only works because of the mock change — before it, such a test would have exercised the mock's private copy. Your 121-of-145 measurement was the most useful line in the review.

The two smaller

EXPLICITLY_ON is pinned by the absence of the warning, exactly as you suggested — and you were right that it pins the normalization for free. 'TRUE', 'tRuE', ' true ', 'YES' and 'ON' were all passing through the fail-closed branch for the wrong reason.

The fourth reader. execute-cutover.ts:171 now reads through the helper. Good catch — that one was the same fail-open sitting on the cutover path, which is arguably a worse place for it than the worker. Also took your docs/deployment.md:212 suggestion; "check SHADOW_MODE before posting" is the instruction that produced the three copies in the first place, so it now names isShadowMode().

Your three judgment calls

Boot log — yes, took the cheap half. One line next to the sync-engine fail-fast, printing the resolved mode and the raw value when it is off. I left the throw out and said why in the comment: staging is documented as SHADOW_MODE=true, so a variable that fails to carry to a new replica is a silent fail-open that this log makes visible without preventing. That is a real behaviour change and it belongs with the startup-assertion work, where the dashboard surface lives too.

'' — dropped it, your reading was right. A cleared variable is a variable that IS set, so keeping it in EXPLICITLY_OFF contradicted the module's own rule. It takes the unrecognized branch now: on, plus the warning carrying the raw value, and ' ' follows it. Stating the trade plainly since it is a behaviour change: a .env line reading SHADOW_MODE= now stops the bot answering rather than starting it. That is the side of the trade this module exists to take, but it is worth someone else agreeing with before it ships.

Header claim — scoped. It now says every value that IS set fails closed, and states the absent-variable exception in the header rather than fifteen lines down, on your exact reasoning: someone skimming it and concluding a missing variable is safe has formed the belief the module exists to kill.

Two notes on process

The reformatted hunks in the discord-bot file are mandatory rather than noise, as you found — I have added a line to the description. Same applies to a second prettier commit that is now folded into this one.

And I did not run a cr-loop on this, because the session I did it in is configured without subagents. Everything above is verified by hand instead: four mutations from a committed baseline, full suite, typecheck on both packages/outpost and apps/worker, pnpm lint 10/10, and the format check green on every changed file. Worth a cr-loop before merge if you would rather have one.

Your teams-bot and postAiResponse findings are the right call to keep out of this diff — the three unreferenced helpers one is genuinely alarming, and .env.example:41-42 promising Teams coverage that does not exist is the kind of claim that gets believed. Happy to take those issues if you have not filed them yet.

[NathanTarbert]

This is in good shape. I re-ran your mutation table from a clean checkout of d4fdae0 and got your numbers exactly — 1234 baseline, and 29 / 5 / 2 / 23 on the four mutations. The fence working across three files instead of one is the difference that matters; the predicate was never really the risk.

The calculateBackoff note is the useful one for whoever reads this next. I would have taken the spread for it without checking, and a nondeterministic retry delay is a miserable thing to debug later. Worth keeping that comment exactly where it is.

One thing to fix before merge

The cutover import does not resolve from scripts/, and it takes the whole suite with it:

FAIL  scripts/__tests__/cutover.test.ts
Error: Cannot find package '@copilotkit/outpost/shared' imported from scripts/cutover/execute-cutover.ts

scripts/ is not a workspace package — pnpm-workspace.yaml lists only apps/* and packages/* — and pnpm does not hoist, so there is no node_modules/@copilotkit for it to walk up to. All 15 tests in that file stop collecting, including the executeCutover test that exercises disableShadowMode. On main the file is fine, because @prisma/client is mocked with a factory and the shared barrel was not imported at all.

Measured with npx vitest run --config vitest.config.ts: main gives 4 files / 64 tests passing, this branch gives 1 failed file / 49 passing.

The reason CI is green is worth knowing on its own. pnpm test is turbo run test, and turbo only runs per-package tasks — the root vitest.config.ts is the only config whose include covers scripts/__tests__/**, and it is not wired to any turbo task. scripts/tsconfig.json is detached from pnpm typecheck the same way. So that suite is green-by-absence and only breaks for someone running vitest at the repo root.

Adding the dependency edge fixes it. I tried it rather than guessing:

// package.json (root)
"dependencies": {
    "@copilotkit/outpost": "workspace:*"
}

pnpm honours the workspace protocol in the root manifest, which creates the link the script needs. Back to 4 files / 64 tests, and turbo test still 10/10. Routing the script through the helper is right — it just needs that edge to exist.

Wiring scripts/ up as a real package with its own test and typecheck tasks would close the CI hole properly, but that is its own change and I would not put it in this diff. (While I was in there: npx tsx scripts/cutover/execute-cutover.ts, the invocation documented at the top of that file, already dies on @prisma/client for the same reason on main. So the script's runtime path was broken before this.)

Small, and I would fold it in

'' meaning ON is documented in the TSDoc and nowhere an operator looks. I swept for anything in the repo that could produce an empty value — all seven apps/*/Dockerfile, the eight railway.toml files, docker-compose.yml, the four workflows, both start.sh, scripts/ — and there is nothing, so this can only be reached by a hand-edited Railway variable or .env line. Which means documentation is the only place it can be mitigated, and "clearing this variable silences production" is not something anyone will infer:

Values. Unset means OFF. true / 1 / yes / on mean ON; false / 0 / no / off mean OFF. Anything else that is set — including an empty value — means ON and logs a warning. Clearing the variable therefore engages shadow mode; to turn it off, set false explicitly rather than blanking it.

One line in .env.example next to the staging/production note, and the same in the docs/deployment.md section you already touched.

Two I would rather you decide on

The warning fires per call rather than per process. isShadowMode() runs per AI_RESPONSE job, per digest, and per Discord thread and message, so SHADOW_MODE= in production is one stderr line per inbound message indefinitely. apps/worker/start.sh splits stdout and stderr specifically so "something needs attention" is machine-distinguishable, and its comment mentions an alert rule keyed on stderr — so a blank variable becomes a permanently-firing alarm, which is how a channel stops being read. A Set of already-warned raw values would do it, though the toHaveBeenCalledTimes(1) assertions would need a reset hook. The other shape is to leave the predicate silent and hoist the complaint into an assertShadowModeConfigured() called once per service at boot, which folds neatly into the next item.

The boot log covers the worker but not the bot. .env.example and docs/deployment.md:204 both document two gates, and the bot's is at ingest — earlier in the flow. So a blank variable on outpost-discord-bot alone diverts every new thread and Discord goes quiet, while the worker's boot line says responses will post. The one line built to answer "which mode am I in" points the wrong way in exactly that case. Same line in apps/discord-bot/src/index.ts before client.login() would cover it. Also worth putting the raw value in both branches — right now the ON branch omits it, and that is the branch unrecognized and blank values land in.

Not this diff

The cutover reporting is worse than the comparison we fixed, and it predates this PR on both paths, so I have written it up as #261 rather than growing this one. Short version: disableShadowMode returns PASS on the branch reached when shadow mode is still on, and step 4 does not gate on it, so --confirm prints "Cutover COMPLETED" and exits 0 with the flag still engaged. rollback.ts is the same shape but never reads the flag at all, which is worse for being the incident path. Your change is what made it visible — under !== 'true' the message was wrong too, and now only the status is.

The test gap goes with it: nothing in scripts/__tests__/cutover.test.ts sets SHADOW_MODE, so disableShadowMode is the one call site this PR touched that no fence holds down. Noted on #261 with the it.each shape.

And the teams-bot and postAiResponse ones are already filed — #258 and #259, linked in the comment above yours, so no need to pick those up.

Verified while I was in here

So nobody repeats it: the barrel export adds only isShadowMode with no collision, and stays safe for the 15 "use client" files importing it — shadow-mode.ts reads process.env inside the function, not at module scope, and web's 697 tests are green. The partial mocks pull in no import-time side effects; the shared subtree is pure re-exports, no PrismaClient, and those two suites plus the shared one run 143 tests in 365ms. Unset-means-off fails in both directions when mutated. The boot log has no throw path, sits above the buildSyncEngine() fail-fast so the mode prints before the DB can kill boot, and can only ever echo unset/false/0/no/off — never an arbitrary operator string. And the .gitignore entry is the complete fix for the compose file, since it was never tracked.

Happy to take the root package.json line myself if you would rather not respin for it.

[NathanTarbert]

Re-ran the three blockers against d4fdae0 rather than reading the diff, and two of them plus all the smaller items are closed.

Queue gates — closed. Both test files now partial-mock with importOriginal instead of hand-rolling the predicate, so the handlers execute the real helper. Reverting queue/src/handlers/ai-response.ts:824 to process.env.SHADOW_MODE === 'true' now fails 6 tests, and the same revert at onboarding-digest.ts:125 fails 6. Reverting the shared function body fails 29.

Cutover script — closed. execute-cutover.ts:172 is if (!isShadowMode()). Probed directly: SHADOW_MODE=TRUE now returns true, so the step no longer reports "already disabled" and no longer skips the ACTION REQUIRED line.

docker-compose.override.yml — closed. Gone from the tree, and .gitignore:78 ignores it with the port-remap reasoning written down.

Also closed: '' is out of EXPLICITLY_OFF (shared/src/shadow-mode.ts:40), the header claim is scoped to values that are set, and docs/deployment.md:212-216 names isShadowMode().

One still open: the Discord gate. apps/discord-bot/src/lib/shadow-mode.ts:19 re-exports the shared helper, which is right, but nothing in that app would notice if someone reintroduced a local comparison there — I replaced the re-export with return process.env.SHADOW_MODE === 'true' and all 66 tests still passed. src/__tests__/shadow-mode.test.ts:62-76 covers unset, 'false' and 'true', the three values where old and new agree. One it.each over 'TRUE', '1', 'yes', 'on' closes it. That app is the one that already had its own copy, so it's the likeliest place for a fourth to reappear.

Two smaller things:

The warning is still unbounded — SHADOW_MODE=maybe with five calls writes five console.warn lines, and ai-response.ts:824 runs per job, so a long shadow window with a typo'd value writes one line per job indefinitely. Memoizing the last-warned raw value at module scope keeps the signal; shadow-mode.test.ts:50 asserts toHaveBeenCalledTimes(1) and would move with it.

The body's numbers are behind the branch: it says 63 files / 1099 tests and "reverting the function body fails 12". At head it's 66 files / 1234 tests, and that revert fails 29 — the coverage is better than the description claims.

.env.example is untouched on this branch, so lines 40-42 still say the worker gate covers every platform including Teams. Pre-existing and separate from this PR, just noting it's still there.