diff --git a/docs/plans/2026-04-18-001-feat-fro-bot-gateway-discord-v1-plan.md b/docs/plans/2026-04-18-001-feat-fro-bot-gateway-discord-v1-plan.md index f9a9b81df9..36da4f2879 100644 --- a/docs/plans/2026-04-18-001-feat-fro-bot-gateway-discord-v1-plan.md +++ b/docs/plans/2026-04-18-001-feat-fro-bot-gateway-discord-v1-plan.md @@ -755,7 +755,9 @@ The Action and gateway both import from `@fro-bot/runtime` (the name is internal **Unit 6 reconciliation (2026-06-01):** - **Shipped (MVP):** mention → thread creation → lock/run-state/heartbeat coordination → remote OpenCode attach (workspace container, bearer-token proxy, HTTP/SSE) → streamed Discord final output; trigger-role/ManageChannels authorization; failure-path partial-output flush. -- **Not yet shipped (deferred from original Unit 6 scope):** tool-approval embeds/buttons + `permission.updated` handling (S5); reactions + working-message progress editor (R9); serial per-channel queue + `/clear-queue` (current behavior REJECTS concurrent same-channel runs rather than queueing, R11); `/review`, `/sessions`, `/resume`, `/approvals`, `/force-release-lock` commands (only `ping` + `add-project` are registered); `no-fro-bot` block role (R6); gateway startup conditional-write self-test wiring (`validateProviderSemantics` exists + is Effect-wrapped + tested but is not called in `program.ts` boot). +- **Not yet shipped (deferred from original Unit 6 scope):** tool-approval embeds/buttons + `permission.updated` handling (S5); reactions + working-message progress editor (R9); serial per-channel queue + `/clear-queue` (current behavior REJECTS concurrent same-channel runs rather than queueing, R11); `/review`, `/sessions`, `/resume`, `/approvals`, `/force-release-lock` commands (only `ping` + `add-project` are registered); reactions + working-message progress editor (R9). +- **Dropped (won't-do):** `no-fro-bot` block role (R6) — redundant deny-list on top of the existing allow-list authorization model. The mention/approval gate already requires the trigger role or guild-level `ManageChannels`; excluding a user is done by not granting the trigger role, so a separate block role adds a second provisioning surface and a precedence question for no real gain. +- **Shipped since:** tool-approval embeds/buttons + `permission.asked`/`permission.replied` handling (S5); gateway startup conditional-write self-test wiring (`validateProviderSemanticsEffect` now runs fail-fast at boot before `client.login`). **Files:** - Create: `packages/gateway/src/execute/local.ts` (orchestrates: acquire lock, create run-state, spawn OpenCode session in workspace, stream events → Discord) diff --git a/docs/plans/2026-06-01-005-feat-gateway-discord-tool-approval-plan.md b/docs/plans/2026-06-01-005-feat-gateway-discord-tool-approval-plan.md index e8041de114..6b221f89d3 100644 --- a/docs/plans/2026-06-01-005-feat-gateway-discord-tool-approval-plan.md +++ b/docs/plans/2026-06-01-005-feat-gateway-discord-tool-approval-plan.md @@ -38,7 +38,7 @@ Sensitive actions (file writes, bash, PR creation) currently run with no human g - Serial queue + `/clear-queue` (R11): separate plan. - `/review`, `/sessions`, `/resume` commands: separate plan. - Gateway startup self-test wiring (#7) and stand-alone safety-wins: can ship independently; not blocked by S5. -- `no-fro-bot` block role (R6): separate small auth-only PR — touches `userIsAuthorized()` independently of S5. +- `no-fro-bot` block role (R6): dropped (won't-do) — redundant deny-list on top of the existing allow-list auth model; exclude a user by not granting the trigger role. - `/fro-bot approvals` list/revoke management command: follow-up plan after S5 core lands. ## Context & Research @@ -286,7 +286,7 @@ Unit 1 probe confirms the full round-trip at 1.14.41? - **Interaction graph:** introduces a Discord `interactionCreate` (button) handler alongside the existing slash-command dispatch; both route through `program.ts` event wiring. The run-core event loop gains `permission.asked` and `permission.replied` branches; the run orchestrator gains a blocking wait on the coordinator promise and a pending-run registry. - **Error propagation:** approval failures (deadline, Discord post failure, reply-endpoint non-2xx) must fail-closed (deny) and flush partial output, never silently drop a run or leave the concurrency slot wedged. -- **Authorization surface:** approve/deny reuses `userIsAuthorized()` so the existing mention auth rules apply uniformly to approvals. The R6 block role is deferred to a separate PR. +- **Authorization surface:** approve/deny reuses `userIsAuthorized()` so the existing mention auth rules apply uniformly to approvals. The R6 block role was dropped (won't-do) as redundant. - **Integration coverage:** the approve→reply→resume path crosses Discord → coordinator → OpenCode HTTP; unit mocks alone won't prove it — include a seam test that asserts the reply endpoint is hit exactly once per authorized decision. - **Unchanged invariants:** the mention loop's existing run path (no permission requested) is unaffected — when no `permission.asked` arrives, behavior is identical to today. The GitHub Action tier is untouched. diff --git a/packages/gateway/src/main.ts b/packages/gateway/src/main.ts index dc7cfab3ac..bd00d52c08 100644 --- a/packages/gateway/src/main.ts +++ b/packages/gateway/src/main.ts @@ -6,6 +6,7 @@ import {loadGatewayConfig} from './config.js' import {createAnnounceServer} from './http/server.js' import {makeDiscordClientFromConfig, makeGatewayProgram, makeLogger} from './program.js' import {setupReadinessFlag} from './readiness.js' +import {validateProviderSemanticsEffect} from './runtime-effect.js' // --------------------------------------------------------------------------- // Main Effect program @@ -26,6 +27,9 @@ const program = Effect.gen(function* () { await client.login(token) }, startAnnounceServer: (serverDeps, serverConfig) => createAnnounceServer(serverDeps, serverConfig), + runProviderSelfTest: async (cc, lg) => { + await Effect.runPromise(validateProviderSemanticsEffect(cc, lg)) + }, }, config, ) diff --git a/packages/gateway/src/program.test.ts b/packages/gateway/src/program.test.ts index 7eaab33a6d..6ce575e0fa 100644 --- a/packages/gateway/src/program.test.ts +++ b/packages/gateway/src/program.test.ts @@ -1,5 +1,8 @@ +import type {CoordinationConfig} from '@fro-bot/runtime' + import type {GatewayConfig} from './config.js' import type {GatewayLogger} from './discord/client.js' +import type {CoordinationLogger} from './runtime-effect.js' import {GatewayIntentBits} from 'discord.js' import {Effect} from 'effect' @@ -179,6 +182,7 @@ describe('makeGatewayProgram', () => { setupReadinessFlag: setupReadinessFlagSpy, login: loginSpy, startAnnounceServer: startAnnounceServerSpy, + runProviderSelfTest: vi.fn(async () => {}), } // #when — run the program with the real Effect runtime. @@ -211,6 +215,7 @@ describe('makeGatewayProgram', () => { setupReadinessFlag: vi.fn(), login: vi.fn().mockResolvedValue(undefined), startAnnounceServer: startAnnounceServerSpy, + runProviderSelfTest: vi.fn(async () => {}), } // #when @@ -247,6 +252,7 @@ describe('makeGatewayProgram', () => { setupReadinessFlag: vi.fn(), login: loginSpy, startAnnounceServer: startAnnounceServerSpy, + runProviderSelfTest: vi.fn(async () => {}), } // #when @@ -260,6 +266,65 @@ describe('makeGatewayProgram', () => { } expect(serverOrder).toBeLessThan(loginOrder) }) + + it('provider self-test runs during boot before login', async () => { + // #given + const fakeConfig = makeFakeConfig() + const fakeClient = makeFakeClient() + const fakeServerHandle = makeFakeServerHandle() + + const callOrder: string[] = [] + const runProviderSelfTestSpy = vi.fn(async (_cc: CoordinationConfig, _lg: CoordinationLogger) => { + callOrder.push('runProviderSelfTest') + }) + const loginSpy = vi.fn(async () => { + callOrder.push('login') + }) + + const deps = { + makeClient: () => fakeClient as unknown as import('discord.js').Client, + setupReadinessFlag: vi.fn(), + login: loginSpy, + startAnnounceServer: vi.fn().mockReturnValue(fakeServerHandle), + runProviderSelfTest: runProviderSelfTestSpy, + } + + // #when + await Effect.runPromise(makeGatewayProgram(deps, fakeConfig)) + + // #then — self-test was called exactly once + expect(runProviderSelfTestSpy).toHaveBeenCalledTimes(1) + + // #and — self-test ran before login + expect(callOrder).toContain('runProviderSelfTest') + expect(callOrder).toContain('login') + expect(callOrder.indexOf('runProviderSelfTest')).toBeLessThan(callOrder.indexOf('login')) + }) + + it('boot fails fast when provider self-test rejects', async () => { + // #given + const fakeConfig = makeFakeConfig() + const fakeClient = makeFakeClient() + const fakeServerHandle = makeFakeServerHandle() + + const loginSpy = vi.fn(async () => {}) + + const deps = { + makeClient: () => fakeClient as unknown as import('discord.js').Client, + setupReadinessFlag: vi.fn(), + login: loginSpy, + startAnnounceServer: vi.fn().mockReturnValue(fakeServerHandle), + runProviderSelfTest: vi.fn(async () => { + throw new Error('IfNoneMatch not honored') + }), + } + + // #when — boot must reject + await expect(Effect.runPromise(makeGatewayProgram(deps, fakeConfig))).rejects.toThrow('IfNoneMatch not honored') + + // #then — login was NOT called (fail before connecting to Discord) + expect(loginSpy).not.toHaveBeenCalled() + }) }) // --------------------------------------------------------------------------- @@ -310,6 +375,7 @@ describe('button interaction handler (approval flow)', () => { setupReadinessFlag: vi.fn(), login: vi.fn().mockResolvedValue(undefined), startAnnounceServer: vi.fn().mockReturnValue(fakeServerHandle), + runProviderSelfTest: vi.fn(async () => {}), } await Effect.runPromise(makeGatewayProgram(deps, fakeConfig)) @@ -593,6 +659,7 @@ describe('button interaction handler (approval flow)', () => { setupReadinessFlag: vi.fn(), login: vi.fn().mockResolvedValue(undefined), startAnnounceServer: vi.fn().mockReturnValue(fakeServerHandle), + runProviderSelfTest: vi.fn(async () => {}), } await Effect.runPromise(makeGatewayProgram(deps, fakeConfig)) diff --git a/packages/gateway/src/program.ts b/packages/gateway/src/program.ts index 61539104f8..c803fdc91f 100644 --- a/packages/gateway/src/program.ts +++ b/packages/gateway/src/program.ts @@ -1,8 +1,10 @@ +import type {CoordinationConfig, ObjectStoreAdapter} from '@fro-bot/runtime' import type {Client, GatewayIntentBits, Message} from 'discord.js' import type {GatewayConfig} from './config.js' import type {GatewayLogger} from './discord/client.js' import type {SinkThread} from './discord/streaming.js' import type {AnnounceServerConfig, AnnounceServerDeps} from './http/server.js' +import type {CoordinationLogger} from './runtime-effect.js' import type {CloseableServer} from './shutdown.js' import { createS3Adapter, @@ -23,6 +25,22 @@ import {createAppClient} from './github/app-client.js' import {installShutdownHandlers, isShuttingDown} from './shutdown.js' import {createWorkspaceClient} from './workspace-api/client.js' +// --------------------------------------------------------------------------- +// Pure helper — builds the coordination config from the shared S3 adapter and +// gateway config. Extracted to avoid repeating the 5-field literal at every +// call site (self-test, mention handler, stale-run recovery). +// --------------------------------------------------------------------------- + +function makeCoordinationConfig(s3Adapter: ObjectStoreAdapter, config: GatewayConfig): CoordinationConfig { + return { + storeAdapter: s3Adapter, + storeConfig: config.objectStore, + lockTtlSeconds: DEFAULT_LOCK_TTL_SECONDS, + heartbeatIntervalMs: DEFAULT_HEARTBEAT_INTERVAL_MS, + staleThresholdMs: DEFAULT_STALE_THRESHOLD_MS, + } +} + // --------------------------------------------------------------------------- // Minimal structured logger — pino can replace this in a later unit. // warn and error use the lint-permitted console channels directly. @@ -80,6 +98,12 @@ export interface GatewayProgramDeps { * Returns a CloseableServer handle that will be passed to installShutdownHandlers. */ readonly startAnnounceServer: (deps: AnnounceServerDeps, config: AnnounceServerConfig) => CloseableServer + /** + * Provider semantics self-test — validates that the S3-compatible store honors + * IfNoneMatch/IfMatch conditional write semantics required for safe coordination. + * Injected so tests can stub it without touching the real S3 adapter. + */ + readonly runProviderSelfTest: (config: CoordinationConfig, logger: CoordinationLogger) => Promise } // --------------------------------------------------------------------------- @@ -117,6 +141,15 @@ export function makeGatewayProgram(deps: GatewayProgramDeps, config: GatewayConf } const s3Adapter = createS3Adapter(config.objectStore, runtimeLogger) + + // Provider semantics self-test — fail-fast before serving so a provider that doesn't honor + // IfNoneMatch/IfMatch conditional writes can't silently corrupt the coordination lock. + const selfTestCoordConfig = makeCoordinationConfig(s3Adapter, config) + yield* Effect.tryPromise({ + try: async () => deps.runProviderSelfTest(selfTestCoordConfig, runtimeLogger), + catch: error => (error instanceof Error ? error : new Error(String(error))), + }) + const concurrencyRegistry = createConcurrencyRegistry(config.maxConcurrentRuns) const bindingsStore = createBindingsStore({ adapter: s3Adapter, @@ -233,13 +266,7 @@ export function makeGatewayProgram(deps: GatewayProgramDeps, config: GatewayConf bindingsStore, triggerRoleId: config.triggerRoleId, run: { - coordinationConfig: { - storeAdapter: s3Adapter, - storeConfig: config.objectStore, - lockTtlSeconds: DEFAULT_LOCK_TTL_SECONDS, - heartbeatIntervalMs: DEFAULT_HEARTBEAT_INTERVAL_MS, - staleThresholdMs: DEFAULT_STALE_THRESHOLD_MS, - }, + coordinationConfig: makeCoordinationConfig(s3Adapter, config), identity: config.identity, concurrency: concurrencyRegistry, attachUrl: config.workspaceOpencodeUrl, @@ -307,13 +334,7 @@ export function makeGatewayProgram(deps: GatewayProgramDeps, config: GatewayConf yield* Effect.tryPromise({ try: async () => recoverStaleRuns({ - coordinationConfig: { - storeAdapter: s3Adapter, - storeConfig: config.objectStore, - lockTtlSeconds: DEFAULT_LOCK_TTL_SECONDS, - heartbeatIntervalMs: DEFAULT_HEARTBEAT_INTERVAL_MS, - staleThresholdMs: DEFAULT_STALE_THRESHOLD_MS, - }, + coordinationConfig: makeCoordinationConfig(s3Adapter, config), identity: config.identity, bindingsStore, resolveThread: async (threadId: string): Promise => {