diff --git a/.gitignore b/.gitignore index d87c787a..f987d6b7 100644 --- a/.gitignore +++ b/.gitignore @@ -70,3 +70,9 @@ yarn.lock # Local AI review pipeline session state (create-review / review-changes) .chalk/ + +# Compose auto-loads this exact filename and it is per-developer by convention. +# One was committed by accident: `ports: !override` REPLACES the base list, so +# postgres stopped publishing 5432 and the README's setup path (db:push against +# localhost:5432) failed on a fresh clone. +docker-compose.override.yml diff --git a/apps/discord-bot/src/lib/shadow-mode.ts b/apps/discord-bot/src/lib/shadow-mode.ts index 06495249..7236a4d8 100644 --- a/apps/discord-bot/src/lib/shadow-mode.ts +++ b/apps/discord-bot/src/lib/shadow-mode.ts @@ -13,9 +13,10 @@ import type { ThreadChannel, Message } from 'discord.js'; * * This enables a parallel-run validation period before full cutover. */ -export function isShadowMode(): boolean { - return process.env.SHADOW_MODE === 'true'; -} +// Re-exported rather than reimplemented. Three copies of this predicate existed +// and all three compared `=== 'true'`; a shared one is the only version of this +// that stays fixed. +export { isShadowMode } from '@copilotkit/outpost/shared'; export interface ShadowResponse { ticketId: string; @@ -48,7 +49,7 @@ export async function logShadowResponse(response: ShadowResponse): Promise console.log( `[Shadow Mode] Logged response for ticket ${response.ticketId} ` + - `(${response.responseTimeMs}ms)`, + `(${response.responseTimeMs}ms)`, ); } @@ -115,10 +116,7 @@ export async function handleShadowThreadCreate( return ticket.id; } catch (error) { - console.error( - `[Shadow Mode] Failed to create ticket for thread ${thread.id}:`, - error, - ); + console.error(`[Shadow Mode] Failed to create ticket for thread ${thread.id}:`, error); return null; } } diff --git a/apps/worker/src/index.ts b/apps/worker/src/index.ts index 1eb3cb74..d49ca0e9 100644 --- a/apps/worker/src/index.ts +++ b/apps/worker/src/index.ts @@ -35,8 +35,31 @@ import { handleGithubReactionPoll, handlePendingResponseSweep, } from '@copilotkit/outpost/queue'; +import { isShadowMode } from '@copilotkit/outpost/shared'; import { buildSyncEngine } from './build-sync-engine.js'; +// ─── Announce the resolved posting mode ─────────────────────────────────── + +// One line, at boot, next to the other fail-fast-at-boot decision below. +// +// Without it an operator has no way to answer "which mode am I in?" except to +// wait for a job and infer it from which line got printed. That is a bad way to +// learn the answer for the one flag standing between a parallel-run window and +// machine-generated text arriving in a stranger's support thread. +// +// Deliberately a log and not a throw. Throwing when SHADOW_MODE is absent in +// production is a real behaviour change — 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 but does not prevent. That +// stronger version belongs with the startup-assertion work, not here. +console.log( + isShadowMode() + ? '[Worker] SHADOW MODE ON — responses are generated and recorded, never posted.' + : `[Worker] SHADOW MODE OFF — responses WILL post to real community surfaces (SHADOW_MODE=${ + process.env.SHADOW_MODE ?? 'unset' + }).`, +); + // ─── Build SyncEngine for TRACKER_SYNC handler ──────────────────────────── // BOOT SEMANTICS — deliberate change. This is a top-level await that performs diff --git a/docs/deployment.md b/docs/deployment.md index 1c0fa261..6ff9e8fa 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -209,8 +209,11 @@ worker's gate is the one that covers **every** platform (GitHub, Slack, Teams) p digest job, because that is where the adapter call lives — so a staging environment must have it set on `outpost-worker`, not only on a bot. -When adding any new outbound post path, check `SHADOW_MODE` before posting — otherwise -staging will deliver to real users regardless of the flag. +When adding any new outbound post path, gate it on `isShadowMode()` from +`@copilotkit/outpost/shared` — not on `process.env.SHADOW_MODE` directly. Reading the +variable is what produced three separate copies of `=== 'true'`, all three of which +treated `SHADOW_MODE=TRUE` as "not shadow mode" and posted for real. The helper is the +only version of this that stays fixed. ### Promotion workflow diff --git a/package.json b/package.json index 317ecb4b..ee148bc9 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,9 @@ "format": "prettier --write \"**/*.{ts,tsx,js,jsx,json,md}\"", "format:check": "prettier --check \"**/*.{ts,tsx,js,jsx,json,md}\"" }, + "dependencies": { + "@copilotkit/outpost": "workspace:*" + }, "devDependencies": { "@eslint/js": "^9.39.4", "@linear/sdk": "^81.0.0", diff --git a/packages/outpost/queue/src/__tests__/ai-response.test.ts b/packages/outpost/queue/src/__tests__/ai-response.test.ts index 8c5f849c..f94e40b6 100644 --- a/packages/outpost/queue/src/__tests__/ai-response.test.ts +++ b/packages/outpost/queue/src/__tests__/ai-response.test.ts @@ -72,17 +72,17 @@ const mockGetAdapter = vi.fn().mockReturnValue({ fetchUserInfo: vi.fn(), }); -vi.mock('@copilotkit/outpost/shared', () => ({ - AI_CONFIDENCE: { - AUTO_RESPOND: 0.9, - HIGH_THRESHOLD: 0.8, - SUGGEST: 0.7, - MEDIUM_THRESHOLD: 0.5, - ESCALATE: 0.4, - }, - MAX_JOB_ATTEMPTS: 5, - BACKOFF_BASE_MS: 1000, - BACKOFF_MAX_MS: 300_000, +// Partial mock, so `isShadowMode` below is the shipped function rather than a +// re-implementation of it. The four constants that used to be stubbed here were +// checked against `shared/src/constants.ts` and are identical, so the spread +// supplies them. +// +// `calculateBackoff` stays overridden on purpose: the real one +// (`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. This override is the deterministic half of it. +vi.mock('@copilotkit/outpost/shared', async (importOriginal) => ({ + ...(await importOriginal()), calculateBackoff: (attempt: number) => 1000 * Math.pow(2, attempt), })); @@ -738,6 +738,33 @@ describe('handleAiResponse', () => { } }); + // The fence, and the one that matters most: this is the handler that reaches + // real Discord and GitHub surfaces. Every other shadow test in this file + // uses `'true'`, the one spelling that read the same before and after the + // fail-closed change — so reverting `isShadowMode` to `=== 'true'` left all + // 145 tests in the two queue suites green. Each of these used to post. + it.each(['1', 'TRUE', 'yes', 'on', ' true ', 'YES'])( + 'skips post-back when SHADOW_MODE=%j', + async (value) => { + const originalShadow = process.env.SHADOW_MODE; + try { + process.env.SHADOW_MODE = value; + mockPrismaTicket.findUnique.mockResolvedValue(sampleTicket); + mockHasAdapter.mockReturnValue(true); + + const result = await handleAiResponse( + { ticketId: 'tkt-1', source: 'discord' }, + makeContext(), + ); + + expect(result.success).toBe(true); + expect(mockPostResponse).not.toHaveBeenCalled(); + } finally { + restoreShadowMode(originalShadow); + } + }, + ); + it('succeeds even if post-back fails (non-fatal)', async () => { mockPrismaTicket.findUnique.mockResolvedValue(sampleTicket); mockHasAdapter.mockReturnValue(true); diff --git a/packages/outpost/queue/src/__tests__/onboarding-digest.test.ts b/packages/outpost/queue/src/__tests__/onboarding-digest.test.ts index 5a439b22..07ee61a6 100644 --- a/packages/outpost/queue/src/__tests__/onboarding-digest.test.ts +++ b/packages/outpost/queue/src/__tests__/onboarding-digest.test.ts @@ -11,7 +11,14 @@ vi.mock('@copilotkit/outpost/db', () => ({ }, })); -vi.mock('@copilotkit/outpost/shared', () => ({ +// Partial mock: everything real except the one function this file needs to +// pin. The previous version listed its exports explicitly, which meant +// re-implementing `isShadowMode` — so a diff whose whole point was deleting +// three copies of the comparison added a fourth, and it had already drifted +// (no EXPLICITLY_ON, no warn). Spreading the real module means the SHADOW_MODE +// tests below exercise the shipped function instead of a lookalike. +vi.mock('@copilotkit/outpost/shared', async (importOriginal) => ({ + ...(await importOriginal()), computeFunnelMetrics: vi.fn().mockReturnValue({ stageCounts: { JOINED: 3, CONTACTED: 2, RESPONDED: 1, MEETING_BOOKED: 0 }, conversionRates: { @@ -84,7 +91,7 @@ describe('handleOnboardingDigest', () => { it('queries members for the given date range', async () => { mockOnboardingMember.findMany - .mockResolvedValueOnce([makeMemberRow()]) // date-filtered query + .mockResolvedValueOnce([makeMemberRow()]) // date-filtered query .mockResolvedValueOnce([makeMemberRow()]); // all-members query for metrics const ctx = makeContext(); @@ -102,8 +109,8 @@ describe('handleOnboardingDigest', () => { it('handles zero new members gracefully', async () => { mockOnboardingMember.findMany - .mockResolvedValueOnce([]) // no members for the day - .mockResolvedValueOnce([]); // no members overall + .mockResolvedValueOnce([]) // no members for the day + .mockResolvedValueOnce([]); // no members overall const ctx = makeContext(); const result = await handleOnboardingDigest({ date: '2026-04-15' }, ctx); @@ -118,9 +125,7 @@ describe('handleOnboardingDigest', () => { const ctx = makeContext(); await handleOnboardingDigest({ date: '2026-04-15' }, ctx); - const progressCalls = ctx.reportProgress.mock.calls.map( - (c: number[]) => c[0], - ); + const progressCalls = ctx.reportProgress.mock.calls.map((c: number[]) => c[0]); expect(progressCalls).toEqual([10, 50, 70, 90, 100]); }); @@ -142,9 +147,7 @@ describe('handleOnboardingDigest', () => { makeMemberRow({ id: 'om-3', username: 'charlie#9012' }), ]; - mockOnboardingMember.findMany - .mockResolvedValueOnce(members) - .mockResolvedValueOnce(members); + mockOnboardingMember.findMany.mockResolvedValueOnce(members).mockResolvedValueOnce(members); const ctx = makeContext(); const result = await handleOnboardingDigest({ date: '2026-04-15' }, ctx); @@ -221,14 +224,46 @@ describe('handleOnboardingDigest', () => { expect(result.success).toBe(true); expect(mockFetch).not.toHaveBeenCalled(); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining('Shadow mode'), - ); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Shadow mode')); consoleSpy.mockRestore(); vi.unstubAllGlobals(); }); + // The fence. Every other shadow test here uses `'true'`, which is the one + // spelling that behaves identically before and after the fail-closed change + // — so reverting `isShadowMode` to `=== 'true'` left this whole file green. + // These are the spellings that used to post for real. + it.each(['1', 'TRUE', 'yes', 'on', ' true ', 'YES'])( + 'does not post to Discord when SHADOW_MODE=%j', + async (value) => { + process.env.DISCORD_TOKEN = 'test-bot-token'; + process.env.DISCORD_DIGEST_CHANNEL_ID = '1234567890'; + process.env.SHADOW_MODE = value; + + mockOnboardingMember.findMany + .mockResolvedValueOnce([makeMemberRow()]) + .mockResolvedValueOnce([makeMemberRow()]); + + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ id: 'msg-1' }), + }); + vi.stubGlobal('fetch', mockFetch); + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const ctx = makeContext(); + const result = await handleOnboardingDigest({ date: '2026-04-15' }, ctx); + + expect(result.success).toBe(true); + expect(mockFetch).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Shadow mode')); + + consoleSpy.mockRestore(); + vi.unstubAllGlobals(); + }, + ); + it('posts to Discord when SHADOW_MODE is explicitly false', async () => { process.env.DISCORD_TOKEN = 'test-bot-token'; process.env.DISCORD_DIGEST_CHANNEL_ID = '1234567890'; @@ -257,9 +292,7 @@ describe('handleOnboardingDigest', () => { process.env.DISCORD_TOKEN = 'test-bot-token'; process.env.DISCORD_DIGEST_CHANNEL_ID = '1234567890'; - mockOnboardingMember.findMany - .mockResolvedValueOnce([]) - .mockResolvedValueOnce([]); + mockOnboardingMember.findMany.mockResolvedValueOnce([]).mockResolvedValueOnce([]); const mockFetch = vi.fn().mockResolvedValue({ ok: false, @@ -269,8 +302,9 @@ describe('handleOnboardingDigest', () => { vi.stubGlobal('fetch', mockFetch); const ctx = makeContext(); - await expect(handleOnboardingDigest({ date: '2026-04-15' }, ctx)) - .rejects.toThrow('Discord API error 403'); + await expect(handleOnboardingDigest({ date: '2026-04-15' }, ctx)).rejects.toThrow( + 'Discord API error 403', + ); vi.unstubAllGlobals(); }); diff --git a/packages/outpost/queue/src/handlers/ai-response.ts b/packages/outpost/queue/src/handlers/ai-response.ts index c79f6005..06457dc4 100644 --- a/packages/outpost/queue/src/handlers/ai-response.ts +++ b/packages/outpost/queue/src/handlers/ai-response.ts @@ -37,7 +37,7 @@ import { prisma } from '@copilotkit/outpost/db'; import { AIPipeline } from '@copilotkit/outpost/ai'; -import { AI_CONFIDENCE, MAX_JOB_ATTEMPTS } from '@copilotkit/outpost/shared'; +import { AI_CONFIDENCE, MAX_JOB_ATTEMPTS, isShadowMode } from '@copilotkit/outpost/shared'; import type { PlatformTarget, TicketSource } from '@copilotkit/outpost/shared'; import { hasAdapter, getAdapter } from '@copilotkit/outpost/shared/platforms'; import { getFeedbackCalibration } from '../feedback-calibration.js'; @@ -821,7 +821,7 @@ export async function handleAiResponse( } let responseDelivered = false; - if (process.env.SHADOW_MODE === 'true') { + if (isShadowMode()) { try { await prisma.message.create({ data: { diff --git a/packages/outpost/queue/src/handlers/onboarding-digest.ts b/packages/outpost/queue/src/handlers/onboarding-digest.ts index f3ceea2d..3d45e070 100644 --- a/packages/outpost/queue/src/handlers/onboarding-digest.ts +++ b/packages/outpost/queue/src/handlers/onboarding-digest.ts @@ -9,7 +9,7 @@ */ import { prisma } from '@copilotkit/outpost/db'; -import { computeFunnelMetrics } from '@copilotkit/outpost/shared'; +import { computeFunnelMetrics, isShadowMode } from '@copilotkit/outpost/shared'; import type { OnboardingMember } from '@copilotkit/outpost/shared'; import type { OnboardingDigestPayload, JobResult, JobHandlerContext } from '../types.js'; @@ -106,9 +106,15 @@ export async function handleOnboardingDigest( digestLines.push(''); digestLines.push('Funnel Summary (all time):'); digestLines.push(` Joined: ${metrics.stageCounts.JOINED}`); - digestLines.push(` Contacted: ${metrics.stageCounts.CONTACTED} (${metrics.conversionRates.joinedToContacted}%)`); - digestLines.push(` Responded: ${metrics.stageCounts.RESPONDED} (${metrics.conversionRates.contactedToResponded}%)`); - digestLines.push(` Meeting Booked: ${metrics.stageCounts.MEETING_BOOKED} (${metrics.conversionRates.respondedToMeetingBooked}%)`); + digestLines.push( + ` Contacted: ${metrics.stageCounts.CONTACTED} (${metrics.conversionRates.joinedToContacted}%)`, + ); + digestLines.push( + ` Responded: ${metrics.stageCounts.RESPONDED} (${metrics.conversionRates.contactedToResponded}%)`, + ); + digestLines.push( + ` Meeting Booked: ${metrics.stageCounts.MEETING_BOOKED} (${metrics.conversionRates.respondedToMeetingBooked}%)`, + ); const digest = digestLines.join('\n'); @@ -116,7 +122,7 @@ export async function handleOnboardingDigest( // Deliver the digest const channelId = process.env.DISCORD_DIGEST_CHANNEL_ID; - if (process.env.SHADOW_MODE === 'true') { + if (isShadowMode()) { // Shadow mode (staging): log the digest instead of posting it, so a // staging worker never delivers to a real Discord channel. console.log(`[Onboarding Digest] Shadow mode — skipping Discord post:\n${digest}`); @@ -124,7 +130,9 @@ export async function handleOnboardingDigest( await postToDiscord(channelId, digest); } else { // Development fallback when DISCORD_DIGEST_CHANNEL_ID is not set - console.log(`[Onboarding Digest] DISCORD_DIGEST_CHANNEL_ID not set, logging to console:\n${digest}`); + console.log( + `[Onboarding Digest] DISCORD_DIGEST_CHANNEL_ID not set, logging to console:\n${digest}`, + ); } await context.reportProgress(100); diff --git a/packages/outpost/shared/src/__tests__/shadow-mode.test.ts b/packages/outpost/shared/src/__tests__/shadow-mode.test.ts new file mode 100644 index 00000000..1240759a --- /dev/null +++ b/packages/outpost/shared/src/__tests__/shadow-mode.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { isShadowMode } from '../shadow-mode.js'; + +describe('isShadowMode', () => { + let original: string | undefined; + let warn: ReturnType; + + beforeEach(() => { + original = process.env.SHADOW_MODE; + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + if (original === undefined) delete process.env.SHADOW_MODE; + else process.env.SHADOW_MODE = original; + warn.mockRestore(); + }); + + const set = (value: string | undefined) => { + if (value === undefined) delete process.env.SHADOW_MODE; + else process.env.SHADOW_MODE = value; + }; + + it('is off when the variable is not set', () => { + set(undefined); + expect(isShadowMode()).toBe(false); + // Shadow mode is opt-in: defaulting an absent variable to ON would make a + // fresh deployment silently answer nobody. + expect(warn).not.toHaveBeenCalled(); + }); + + // The regression this module exists for. Every one of these read as + // "not shadow mode" under `=== 'true'` and posted to real Discord and GitHub + // surfaces — a safety flag failing open on values an operator would + // reasonably expect to work. + it.each(['TRUE', 'True', '1', 'yes', 'YES', 'on', 'ON', ' true ', 'tRuE'])( + 'treats %j as ON, recognized rather than guessed', + (value) => { + set(value); + expect(isShadowMode()).toBe(true); + // Asserting the absence of the warning is what pins EXPLICITLY_ON. + // On the boolean alone, shrinking the set to `['true']` left all of + // these passing — a dropped member still comes back `true` through + // the fail-closed branch, just for the wrong reason and with a + // spurious warning. This also pins `.trim()` and `.toLowerCase()`, + // which were each held down by exactly one OFF case. + expect(warn).not.toHaveBeenCalled(); + }, + ); + + it.each(['false', 'FALSE', '0', 'no', 'off'])('treats %j as OFF', (value) => { + set(value); + expect(isShadowMode()).toBe(false); + expect(warn).not.toHaveBeenCalled(); + }); + + // A cleared value is still a value that IS set, so it takes the same + // fail-closed path as any other unclear instruction. A declared-but-empty + // Railway variable, or a `.env` line with nothing after the `=`, used to + // read as "post for real" silently; it now stops the bot and says why. + it.each(['', ' '])('treats the cleared value %j as ON, and says so', (value) => { + set(value); + expect(isShadowMode()).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain(JSON.stringify(value)); + }); + + // Anything set but unrecognized is an operator trying to say something. The + // safe reading of an unclear instruction is the one that posts nothing. + it.each(['maybe', 'shadow', 'enabled', '2', 'null', 'undefined'])( + 'fails CLOSED on the unrecognized value %j, and says so', + (value) => { + set(value); + expect(isShadowMode()).toBe(true); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain(JSON.stringify(value)); + }, + ); + + it('does not warn on values it recognizes either way', () => { + set('true'); + expect(isShadowMode()).toBe(true); + set('false'); + expect(isShadowMode()).toBe(false); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/outpost/shared/src/index.ts b/packages/outpost/shared/src/index.ts index 7f2394f6..8f9cad14 100644 --- a/packages/outpost/shared/src/index.ts +++ b/packages/outpost/shared/src/index.ts @@ -1,6 +1,7 @@ export * from './types.js'; export * from './constants.js'; export * from './utils.js'; +export * from './shadow-mode.js'; export * from './dispatch/index.js'; export * from './sla/index.js'; export * from './onboarding/index.js'; @@ -33,13 +34,8 @@ export type { HandleOptions, InboundPrismaLike, } from './platforms/index.js'; -export type { - PlatformDiscordAdapterConfig, -} from './platforms/index.js'; -export type { - PlatformGitHubAdapterConfig, - GitHubOctokitLike, -} from './platforms/index.js'; +export type { PlatformDiscordAdapterConfig } from './platforms/index.js'; +export type { PlatformGitHubAdapterConfig, GitHubOctokitLike } from './platforms/index.js'; export type { PlatformSlackAdapterConfig, SlackAdapterConfig, @@ -51,6 +47,4 @@ export type { TeamsActivity, TeamsConversationReference, } from './platforms/index.js'; -export type { - PlatformEmailPostmarkAdapterConfig, -} from './platforms/index.js'; +export type { PlatformEmailPostmarkAdapterConfig } from './platforms/index.js'; diff --git a/packages/outpost/shared/src/shadow-mode.ts b/packages/outpost/shared/src/shadow-mode.ts new file mode 100644 index 00000000..cb4ae005 --- /dev/null +++ b/packages/outpost/shared/src/shadow-mode.ts @@ -0,0 +1,69 @@ +/** + * Whether Outpost is running in shadow mode. + * + * Shadow mode is a safety flag: when it is on, AI responses are generated and + * recorded but never posted to a real community surface. It exists so Outpost + * can run in parallel with an incumbent without double-posting at reporters. + * + * The comparison is the whole point of this module. Every call site used to be + * `process.env.SHADOW_MODE === 'true'`, which means `SHADOW_MODE=TRUE`, + * `SHADOW_MODE=1` and `SHADOW_MODE=yes` all read as "not shadow mode" and post + * to Discord and GitHub for real. That is a safety flag failing OPEN on inputs + * an operator would reasonably expect to work — the direction a safety flag must + * never fail. + * + * So every value that IS set fails CLOSED instead: recognized off values are + * off, recognized on values are on, and anything else is treated as ON and + * logged. The asymmetry is deliberate. A false positive costs a parallel-run + * window where nothing is posted and someone notices from the logs; a false + * negative posts machine-generated text at real people under a flag that was + * meant to prevent exactly that. + * + * `SHADOW_MODE` being ABSENT is the one exception and it means off, because a + * fresh deployment defaulting to ON would silently answer nobody. That + * exception is stated here rather than only on the function, because a reader + * who skims this header and concludes a missing variable is the safe case has + * formed exactly the belief this module exists to kill. + */ + +/** + * Values that mean "off". Everything else that is set means "on". + * + * `''` is deliberately NOT here. A declared-but-cleared Railway variable, or a + * `.env` line with nothing after the `=`, is a value that IS set — so by this + * module's own rule it is an operator trying to say something unclear, and the + * safe reading of that is the one that posts nothing. It falls through to the + * unrecognized branch, which turns shadow mode on and says so. The cost is a + * cleared variable stopping the bot instead of starting it, which is the side + * of that trade this module exists to take. + */ +const EXPLICITLY_OFF = new Set(['false', '0', 'no', 'off']); + +/** Values that mean "on" without comment. Others are honored but logged. */ +const EXPLICITLY_ON = new Set(['true', '1', 'yes', 'on']); + +/** + * True when shadow mode is engaged. + * + * Unset means off — shadow mode is opt-in, and defaulting an absent variable to + * ON would make a fresh deployment silently answer nobody. The fail-closed rule + * applies to values that ARE set: those are an operator trying to say something, + * and the safe reading of an unclear instruction is the one that posts nothing. + */ +export function isShadowMode(): boolean { + const raw = process.env.SHADOW_MODE; + if (raw === undefined) return false; + + const normalized = raw.trim().toLowerCase(); + if (EXPLICITLY_OFF.has(normalized)) return false; + if (EXPLICITLY_ON.has(normalized)) return true; + + // Set to something we do not recognize. Treated as ON, and said out loud — + // silently guessing either way is how the original bug survived. + console.warn( + `[Shadow Mode] SHADOW_MODE is set to an unrecognized value ${JSON.stringify(raw)}; ` + + 'treating it as ON so nothing is posted to a real community surface. ' + + 'Use true/false (or 1/0, yes/no, on/off) to be explicit.', + ); + return true; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1be55189..87059f99 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7,6 +7,10 @@ settings: importers: .: + dependencies: + '@copilotkit/outpost': + specifier: workspace:* + version: link:packages/outpost devDependencies: '@eslint/js': specifier: ^9.39.4 diff --git a/scripts/cutover/execute-cutover.ts b/scripts/cutover/execute-cutover.ts index 4ff2867d..54de5ff9 100644 --- a/scripts/cutover/execute-cutover.ts +++ b/scripts/cutover/execute-cutover.ts @@ -7,6 +7,7 @@ // npx tsx scripts/cutover/execute-cutover.ts --confirm # execute for real import { PrismaClient } from '@prisma/client'; +import { isShadowMode } from '@copilotkit/outpost/shared'; import { validateQuality } from './validate-quality.js'; // ─── Types ────────────────────────────────────────────────────────────────── @@ -40,7 +41,7 @@ function parseArgs(argv: string[]): CliArgs { skipQualityCheck: false, announcement: "We've upgraded our support bot to provide faster, more accurate responses. " + - "If you notice any issues, please let us know!", + 'If you notice any issues, please let us know!', }; for (let i = 2; i < argv.length; i++) { @@ -63,9 +64,7 @@ function parseArgs(argv: string[]): CliArgs { // ─── Step Execution ───────────────────────────────────────────────────────── function logStep(result: StepResult): void { - const icon = result.status === 'PASS' ? '[OK]' - : result.status === 'FAIL' ? '[FAIL]' - : '[SKIP]'; + const icon = result.status === 'PASS' ? '[OK]' : result.status === 'FAIL' ? '[FAIL]' : '[SKIP]'; console.log(` ${icon} ${result.step}: ${result.message}`); } @@ -76,13 +75,8 @@ export async function runHealthChecks(prisma: PrismaClient): Promise await prisma.$queryRaw`SELECT 1`; // Verify required env vars are set - const required = [ - 'DATABASE_URL', - 'DISCORD_TOKEN', - 'DISCORD_CLIENT_ID', - 'GUILD_ID', - ]; - const missing = required.filter(k => !process.env[k]); + const required = ['DATABASE_URL', 'DISCORD_TOKEN', 'DISCORD_CLIENT_ID', 'GUILD_ID']; + const missing = required.filter((k) => !process.env[k]); if (missing.length > 0) { return { step, @@ -168,8 +162,14 @@ export async function disableShadowMode(confirm: boolean): Promise { // In a real deployment, this would call the Railway API to update env vars. // For now, we verify the current state and log the instruction. + // + // Reads through the shared helper, not `!== 'true'`. This was the fourth + // copy of that comparison and the sweep missed it, which meant + // `SHADOW_MODE=TRUE` had the worker correctly withholding posts while this + // step reported shadow mode already disabled and passed — the same + // fail-open, moved onto the cutover path. const currentValue = process.env.SHADOW_MODE; - if (currentValue !== 'true') { + if (!isShadowMode()) { return { step, status: 'PASS', @@ -228,10 +228,7 @@ export async function verifyTicketData(prisma: PrismaClient): Promise { +export async function executeCutover(prisma: PrismaClient, args: CliArgs): Promise { const log: CutoverLog = { startedAt: new Date(), completedAt: null, diff --git a/vitest.config.ts b/vitest.config.ts index 40f3a2b9..b3b2d05f 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,8 +1,27 @@ import { defineConfig } from 'vitest/config'; +import path from 'path'; export default defineConfig({ test: { globals: true, include: ['scripts/__tests__/**/*.test.ts'], }, + resolve: { + alias: { + // `scripts/` is not a workspace package, so the root manifest's + // dependency on `@copilotkit/outpost` is what creates a + // node_modules/@copilotkit for it to resolve through. That alone + // resolves to `shared/dist`, which means this suite would pass only + // after a build and fail on a clean checkout with the same opaque + // "Cannot find package" it was failing with before. + // + // Aliasing to source removes the build from the loop, and matches + // what packages/outpost/vitest.config.ts already does for the same + // specifier. + '@copilotkit/outpost/shared': path.resolve( + __dirname, + 'packages/outpost/shared/src/index.ts', + ), + }, + }, });