diff --git a/.vscode/settings.json b/.vscode/settings.json index 3c426dce5918..55c479eb48cd 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -14,5 +14,6 @@ }, "search.exclude": { ".repos/**": true - } + }, + "js/ts.experimental.useTsgo": true } diff --git a/AGENTS.md b/AGENTS.md index 8d5bc1b6849e..fbe3060b157e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,9 +167,14 @@ whole branching model. See work dir (`~/.t3/compose-work`, not tmpfs `/tmp`) before install. See [docs/fork-stack.md](./docs/fork-stack.md) ("Integration overlay compose and lockfiles"). -## Pull requests (required handoff) +## Pull requests (when publishing) -When implementation work for a user request is done (code, docs, config — not pure Q&A): +Do not commit, rebase, push, or open/update a PR merely because an edit is complete. Do those +things only when the user explicitly requests publication or the specific version-control action, +or when another workflow in this file explicitly requires it (for example, Discord-originated +work). A request to change code, docs, or config does not by itself authorize publication. + +When publication or a PR handoff is in scope: 1. **Commit** the changes on a feature branch cut from `fork/dev`. 2. **Open or update a PR against `fork/dev`** before handing off — for every kind of work, including diff --git a/apps/server/src/ntbs/ExchangeRepository.test.ts b/apps/server/src/ntbs/ExchangeRepository.test.ts new file mode 100644 index 000000000000..058c7a8e4b7b --- /dev/null +++ b/apps/server/src/ntbs/ExchangeRepository.test.ts @@ -0,0 +1,260 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit } from "effect"; +import { MessageId, ProjectId, ThreadId, TurnId } from "@t3tools/contracts"; +import { + ExchangeRepositoryError, + ExchangeRepository, + inMemoryExchangeRepository, +} from "./ExchangeRepository.ts"; +import { + makeRequestAccepted, + toReplyPending, + toReplyPosted, + toThreadCreated, + toUndeliverable, + toWorkPlanned, + type Reply, +} from "./exchange.ts"; + +const now = 1_700_000_000_000; + +const makeAccepted = (sourceUri: string) => + makeRequestAccepted( + { sourceUri, snapshot: "request", attachments: [] }, + { projectId: ProjectId.make("project"), startBranchName: "main" }, + now, + ); + +const makeExchange = (sourceUri: string, threadId: string) => + toWorkPlanned( + makeAccepted(sourceUri), + { + projectId: ProjectId.make("project"), + startBranchName: "main", + startCommitSha: "start-commit-sha", + threadId: ThreadId.make(threadId), + userMessageId: MessageId.make(`message-${threadId}`), + worktreeBranchName: `branch-${threadId}`, + }, + now, + ); + +const answerFrom = (threadId: string, text: string): Reply => ({ + type: "answer", + text, + threadId: ThreadId.make(threadId), + userMessageId: MessageId.make(`message-${threadId}`), + turnId: TurnId.make(`turn-${threadId}`), +}); + +describe("inMemoryExchangeRepository", () => { + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("allows the same sourceUri to advance its state", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const planned = makeExchange("test://request/1", "thread-1"); + const threadCreated = toThreadCreated(planned, now); + + yield* repository.upsert(planned); + yield* repository.upsert(threadCreated); + + expect(yield* repository.findBySourceUri(planned.sourceUri)).toEqual(threadCreated); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("allows the same state to be rewritten", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const planned = makeExchange("test://request/1", "thread-1"); + + yield* repository.upsert(planned); + yield* repository.upsert(planned); + + expect(yield* repository.findBySourceUri(planned.sourceUri)).toEqual(planned); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("refuses a replacement that is not an update of the stored exchange", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const planned = makeExchange("test://request/1", "thread-1"); + const threadCreated = toThreadCreated(planned, now); + const replyPending = toReplyPending(threadCreated, answerFrom("thread-1", "reply"), now); + const posted = toReplyPosted(replyPending, "test://reply/1", now); + + yield* repository.upsert(threadCreated); + const backwards = yield* Effect.flip(repository.upsert(planned)); + expect(backwards).toBeInstanceOf(ExchangeRepositoryError); + + const skippingAhead = yield* Effect.flip(repository.upsert(posted)); + expect(skippingAhead).toBeInstanceOf(ExchangeRepositoryError); + + yield* repository.upsert(replyPending); + yield* repository.upsert(posted); + const afterTerminal = yield* Effect.flip(repository.upsert(threadCreated)); + expect(afterTerminal).toBeInstanceOf(ExchangeRepositoryError); + + expect(yield* repository.findBySourceUri(planned.sourceUri)).toEqual(posted); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("rejects a threadId already owned by another sourceUri", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const existing = makeExchange("test://request/1", "shared-thread"); + const conflicting = makeExchange("test://request/2", "shared-thread"); + + yield* repository.upsert(existing); + const error = yield* Effect.flip(repository.upsert(conflicting)); + + expect(error).toBeInstanceOf(ExchangeRepositoryError); + expect(error.reason).toContain(existing.t3.threadId); + expect(yield* repository.findBySourceUri(existing.sourceUri)).toEqual(existing); + expect(yield* repository.findBySourceUri(conflicting.sourceUri)).toBeNull(); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("keeps a replied exchange's thread out of reach of other sourceUris", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const replied = toReplyPending( + toThreadCreated(makeExchange("test://request/1", "shared-thread"), now), + answerFrom("shared-thread", "done"), + now, + ); + const conflicting = makeExchange("test://request/2", "shared-thread"); + + yield* repository.upsert(replied); + const error = yield* Effect.flip(repository.upsert(conflicting)); + + expect(error).toBeInstanceOf(ExchangeRepositoryError); + expect(yield* repository.findBySourceUri(conflicting.sourceUri)).toBeNull(); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("finds an exchange by threadId", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const exchange = makeExchange("test://request/1", "thread-1"); + + yield* repository.upsert(exchange); + + expect(yield* repository.findByThreadId(exchange.t3.threadId)).toEqual(exchange); + expect(yield* repository.findByThreadId(ThreadId.make("unknown-thread"))).toBeNull(); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("finds only non-terminal exchanges", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const accepted = makeAccepted("test://request/accepted"); + const planned = makeExchange("test://request/planned", "thread-planned"); + const threadCreated = toThreadCreated( + makeExchange("test://request/thread-created", "thread-created"), + now, + ); + const replyPending = toReplyPending( + toThreadCreated( + makeExchange("test://request/reply-pending", "thread-reply-pending"), + now, + ), + answerFrom("thread-reply-pending", "pending reply"), + now, + ); + const replyPosted = toReplyPosted( + toReplyPending( + toThreadCreated( + makeExchange("test://request/reply-posted", "thread-reply-posted"), + now, + ), + answerFrom("thread-reply-posted", "posted reply"), + now, + ), + "test://reply/posted", + now, + ); + const undeliverable = toUndeliverable( + toReplyPending( + toThreadCreated( + makeExchange("test://request/undeliverable", "thread-undeliverable"), + now, + ), + answerFrom("thread-undeliverable", "undeliverable reply"), + now, + ), + { message: "platform rejected the reply" }, + now, + ); + + yield* Effect.forEach( + [accepted, planned, threadCreated, replyPending, replyPosted, undeliverable], + repository.upsert, + ); + + const results = yield* repository.findNonTerminalExchanges; + + expect(results).toHaveLength(4); + expect(results).toEqual( + expect.arrayContaining([accepted, planned, threadCreated, replyPending]), + ); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("preserves existing records when a replacement has a conflicting threadId", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const first = makeExchange("test://request/1", "thread-1"); + const second = makeExchange("test://request/2", "thread-2"); + const conflictingReplacement = makeExchange("test://request/2", "thread-1"); + + yield* repository.upsert(first); + yield* repository.upsert(second); + yield* Effect.flip(repository.upsert(conflictingReplacement)); + + expect(yield* repository.findBySourceUri(first.sourceUri)).toEqual(first); + expect(yield* repository.findBySourceUri(second.sourceUri)).toEqual(second); + expect(yield* repository.findByThreadId(first.t3.threadId)).toEqual(first); + expect(yield* repository.findByThreadId(second.t3.threadId)).toEqual(second); + }), + ); + }); + + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("atomically rejects concurrent upserts with the same threadId", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const first = makeExchange("test://request/1", "shared-thread"); + const second = makeExchange("test://request/2", "shared-thread"); + + const outcomes = yield* Effect.all( + [Effect.exit(repository.upsert(first)), Effect.exit(repository.upsert(second))], + { concurrency: "unbounded" }, + ); + + expect(outcomes.filter(Exit.isSuccess)).toHaveLength(1); + expect(outcomes.filter(Exit.isFailure)).toHaveLength(1); + + const stored = yield* Effect.all([ + repository.findBySourceUri(first.sourceUri), + repository.findBySourceUri(second.sourceUri), + ]); + + expect(stored.filter((state) => state !== null)).toHaveLength(1); + }), + ); + }); +}); diff --git a/apps/server/src/ntbs/ExchangeRepository.ts b/apps/server/src/ntbs/ExchangeRepository.ts new file mode 100644 index 000000000000..99ce6bc100ee --- /dev/null +++ b/apps/server/src/ntbs/ExchangeRepository.ts @@ -0,0 +1,154 @@ +/* + * Defines the repository for durable NTBS exchanges. + * + * An exchange links an admitted external-platform request to its planned T3 + * work and tracks its progress through delivery of the eventual reply. + * + * The repository owns persistence, lookup, and recovery. Each stored exchange + * is identified by its `sourceUri`, while the processor decides how to handle + * duplicate requests. It does not communicate with T3 or the originating + * platform. + */ +import { Array, Effect, Context, Data, HashMap, Ref, Layer, Result } from "effect"; +import { + getThreadId, + isNonTerminal, + isUpdateOf, + type Exchange, + type NonTerminalExchange, +} from "./exchange.ts"; +import type { ThreadId } from "@t3tools/contracts"; +import { isSome } from "effect/Option"; + +export class ExchangeRepositoryError extends Data.TaggedError("ExchangeRepositoryError")<{ + readonly reason: string; + readonly cause: unknown; +}> {} + +export interface ExchangeRepository { + readonly findBySourceUri: ( + sourceUri: string, + ) => Effect.Effect; + + readonly findByThreadId: ( + threadId: ThreadId, + ) => Effect.Effect; + + readonly findNonTerminalExchanges: Effect.Effect< + ReadonlyArray, + ExchangeRepositoryError + >; + + /** + * Inserts or replaces the exchange identified by its `sourceUri`, as long as it is a legal update of the stored one and the thread it refers to does not already belong to another exchange. + * + * The checks and the write are atomic, so of two conflicting concurrent upserts at most one succeeds. + */ + readonly upsert: (exchange: Exchange) => Effect.Effect; +} + +export const ExchangeRepository = Context.Service( + "t3code/ntbs/ExchangeRepository", +); + +const inMemoryER: Effect.Effect = Effect.gen(function* () { + const exchanges: Ref.Ref> = yield* Ref.make( + HashMap.empty(), + ); + + /** Whether `exchange` may be written into `map`, with the reason when it may not. */ + const validate = ( + map: HashMap.HashMap, + exchange: Exchange, + ): Result.Result => { + // Rule 1: a stored exchange may only be replaced by an update of itself. + const previous = HashMap.get(map, exchange.sourceUri); + + if (isSome(previous) && !isUpdateOf(exchange, previous.value)) { + return Result.fail( + new ExchangeRepositoryError({ + reason: `Exchange ${exchange.sourceUri} cannot move from ${previous.value.tag} to ${exchange.tag}`, + cause: { sourceUri: exchange.sourceUri, from: previous.value.tag, to: exchange.tag }, + }), + ); + } + + // Rule 2: the thread an exchange refers to may not belong to another exchange. + const threadId = getThreadId(exchange); + + if (threadId === null) { + return Result.void; + } + + const owner = HashMap.findFirst( + map, + (existing, sourceUri) => + sourceUri !== exchange.sourceUri && getThreadId(existing) === threadId, + ); + + if (isSome(owner)) { + return Result.fail( + new ExchangeRepositoryError({ + reason: `Thread ${threadId} already belongs to exchange ${owner.value[0]}`, + cause: { + threadId, + existingSourceUri: owner.value[0], + incomingSourceUri: exchange.sourceUri, + }, + }), + ); + } + + return Result.void; + }; + + // Validating and writing share one modify, so concurrent upserts cannot both pass. + const upsert = Effect.fn("ExchangeRepository.upsert")((exchange: Exchange) => + exchanges.pipe( + Ref.modify((map) => { + const result = validate(map, exchange); + return [ + result, + Result.isSuccess(result) ? HashMap.set(map, exchange.sourceUri, exchange) : map, + ]; + }), + Effect.flatMap(Effect.fromResult), + ), + ); + + const findBySourceUri = (uri: string) => + Ref.get(exchanges).pipe( + Effect.map((map) => HashMap.get(map, uri)), + Effect.map((o) => (isSome(o) ? o.value : null)), + ); + + const findByThreadId = (threadId: ThreadId) => + Ref.get(exchanges).pipe( + Effect.map((map) => HashMap.filter(map, (val) => getThreadId(val) === threadId)), + // if we get more than one Exchange in the HashMap, something's wrong + Effect.andThen((map) => + HashMap.size(map) > 1 + ? new ExchangeRepositoryError({ + reason: "Exchange Repository contains more than one entry for thredId: " + threadId, + cause: map, + }) + : Effect.succeed(Array.fromIterable(HashMap.entries(map))).pipe( + Effect.map((arr) => (arr.length === 1 ? arr[0]![1] : null)), + ), + ), + ); + + const findNonTerminalExchanges = Ref.get(exchanges).pipe( + Effect.map((map) => Array.fromIterable(HashMap.entries(map))), + Effect.map((arr) => + Array.filter( + arr.map((el) => el[1]), + isNonTerminal, + ), + ), + ); + + return { upsert, findBySourceUri, findByThreadId, findNonTerminalExchanges }; +}); + +export const inMemoryExchangeRepository = Layer.effect(ExchangeRepository, inMemoryER); diff --git a/apps/server/src/ntbs/adapter.ts b/apps/server/src/ntbs/adapter.ts new file mode 100644 index 000000000000..4ae99913981b --- /dev/null +++ b/apps/server/src/ntbs/adapter.ts @@ -0,0 +1,56 @@ +import type { ReplyPending, ThreadCreated, UndeliverableCause } from "./exchange.ts"; +import { Context, Data, Effect } from "effect"; + +/** + * A platform operation failed without establishing that reply delivery is + * permanently impossible. The processor may retry the operation later. + */ +export class AdapterError extends Data.TaggedError("AdapterError")<{ + readonly reason: string; + readonly cause: unknown; +}> {} + +/** The platform definitively rejected delivery of a pending reply. */ +export class ReplyRejected extends Data.TaggedError("ReplyRejected")<{ + readonly cause: UndeliverableCause; +}> {} + +/** + * Defines the platform-specific operations used by the shared NTBS processor. + * + * An adapter communicates with one originating platform. It posts + * acknowledgements and replies, and can discover whether a particular pending + * reply was already posted. It does not persist exchange state, create T3 + * threads, or interpret T3 events. + */ +export interface NTBSAdapter { + /** + * Posts a best-effort working acknowledgement for an exchange whose T3 + * thread now exists. The acknowledgement is not part of the durable exchange + * lifecycle and its platform identifier is not retained. + */ + readonly acknowledge: (state: ThreadCreated) => Effect.Effect; + + /** + * Posts the exact reply stored in `state` to the destination identified by + * its `sourceUri`. + * + * Returns an adapter-encoded URI locating the posted reply. `ReplyRejected` + * means the platform definitively refused delivery; other failures remain + * retryable. + */ + readonly postReply: (state: ReplyPending) => Effect.Effect; + + /** + * Searches for the exact pending reply in case it was posted before the + * corresponding `ReplyPosted` state could be persisted. + * + * Returns its adapter-encoded source URI when found, or `null` otherwise. + */ + readonly findPostedReply: (state: ReplyPending) => Effect.Effect; +} + +/** + * One tag for every platform. A processor resolves its adapter from the context it is built in, so each one is given the implementation for its own platform. + */ +export const NTBSAdapter = Context.Service("t3code/ntbs/adapter"); diff --git a/apps/server/src/ntbs/exchange.test.ts b/apps/server/src/ntbs/exchange.test.ts new file mode 100644 index 000000000000..677ec3b3efc0 --- /dev/null +++ b/apps/server/src/ntbs/exchange.test.ts @@ -0,0 +1,328 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Duration } from "effect"; +import { + fromReplyPending, + fromRequestAccepted, + fromThreadCreated, + fromWorkPlanned, + getThreadId, + isExpired, + isUpdateOf, + makeRequestAccepted, + toExpired, + toRejected, + toReplyPending, + toReplyPosted, + toThreadCreated, + toUndeliverable, + toWorkPlanned, + type Exchange, + type ExchangeBase, + type NonTerminalExchange, + type ReplyPosted, + type Request, + type RequestAccepted, + type T3Target, + type ThreadCreated, + type TurnCoordinates, + type WorkCoordinates, +} from "./exchange.ts"; +import { MessageId, ProjectId, ThreadId, TurnId } from "@t3tools/contracts"; + +const request = { + sourceUri: "test://exchange/test", + snapshot: "You need to imagine some text here", + attachments: [], +} satisfies Request; + +const target = { + projectId: ProjectId.make("projectId"), + startBranchName: "startBranchName", +} satisfies T3Target; + +const coordinates = { + projectId: target.projectId, + startBranchName: target.startBranchName, + startCommitSha: "startCommitSha", + threadId: ThreadId.make("threadId"), + userMessageId: MessageId.make("messageId"), + worktreeBranchName: "worktreeBranchName", +} satisfies WorkCoordinates; + +const turn = { + threadId: coordinates.threadId, + userMessageId: coordinates.userMessageId, + turnId: TurnId.make("turnId"), +} satisfies TurnCoordinates; + +const now = 1_700_000_000_000; +// Past every state's deadline. +const later = now + Duration.toMillis(Duration.hours(2)); + +const exchangeBase = { ...request, target, createdAt: now, updatedAt: now } satisfies ExchangeBase; + +const rejection = { reason: "T3 said no", method: "someT3Call" }; + +const accepted = makeRequestAccepted(request, target, now); +const planned = toWorkPlanned(accepted, coordinates, now); +const threadCreated = toThreadCreated(planned, now); +const answer = { type: "answer", text: "The turn's final answer", ...turn } as const; + +describe("RequestAccepted", () => { + it("makeRequestAccepted tags the request and target, nothing from T3", () => { + expect(accepted).toEqual({ ...exchangeBase, tag: "request-accepted" }); + }); + + it("toWorkPlanned adds the coordinates", () => { + expect(planned).toEqual({ ...exchangeBase, tag: "work-planned", t3: coordinates }); + }); + + it("a transition stamps updatedAt and keeps createdAt", () => { + expect(toWorkPlanned(accepted, coordinates, later)).toEqual({ + ...exchangeBase, + updatedAt: later, + tag: "work-planned", + t3: coordinates, + }); + }); + + it.each([ + [now, { type: "plan" }], + [later, { type: "expire" }], + ] as const)("decides at %d -> %j", (at, expected) => { + expect(fromRequestAccepted(accepted, at)).toEqual(expected); + }); + + it("a planning rejection becomes a failure reply with no T3 context", () => { + expect(toRejected(accepted, rejection, now)).toEqual({ + ...exchangeBase, + tag: "reply-pending", + reply: { + type: "failure", + text: rejection.reason, + cause: { type: "rejected", method: rejection.method, state: { tag: "request-accepted" } }, + }, + }); + }); + + it("expiry becomes a failure reply with no T3 context", () => { + expect(toExpired(accepted, later)).toEqual({ + ...exchangeBase, + updatedAt: later, + tag: "reply-pending", + reply: { + type: "failure", + text: "T3 did not answer in time.", + cause: { type: "expired", state: { tag: "request-accepted" } }, + }, + }); + }); +}); + +describe("WorkPlanned", () => { + // missing thread -> provision it, unless expired; present thread -> record it, even when expired + it.each([ + [{ thread: "missing" }, now, { type: "provision-thread" }], + [{ thread: "present" }, now, { type: "record-thread-created" }], + [{ thread: "missing" }, later, { type: "expire" }], + [{ thread: "present" }, later, { type: "record-thread-created" }], + ] as const)("decides %j at %d -> %j", (context, at, expected) => { + expect(fromWorkPlanned(planned, context, at)).toEqual(expected); + }); + + it("toThreadCreated retags and carries the coordinates forward", () => { + expect(threadCreated).toEqual({ ...exchangeBase, tag: "thread-created", t3: coordinates }); + }); + + it("a provisioning rejection keeps the planned coordinates inside the reply only", () => { + expect(toRejected(planned, rejection, now)).toEqual({ + ...exchangeBase, + tag: "reply-pending", + reply: { + type: "failure", + text: rejection.reason, + cause: { + type: "rejected", + method: rejection.method, + state: { tag: "work-planned", t3: coordinates }, + }, + }, + }); + }); + + it("expiry keeps the planned coordinates inside the reply only", () => { + expect(toExpired(planned, later).reply).toEqual({ + type: "failure", + text: "T3 did not answer in time.", + cause: { type: "expired", state: { tag: "work-planned", t3: coordinates } }, + }); + }); +}); + +describe("ThreadCreated", () => { + // missing turn -> start it; active turn -> wait; both expire; completed turn -> record its reply, even when expired + it.each([ + [{ turn: "missing" }, now, { type: "start-turn" }], + [{ turn: "active" }, now, { type: "wait" }], + [{ turn: "completed", reply: answer }, now, { type: "record-reply-pending", reply: answer }], + [{ turn: "missing" }, later, { type: "expire" }], + [{ turn: "active" }, later, { type: "expire" }], + [{ turn: "completed", reply: answer }, later, { type: "record-reply-pending", reply: answer }], + ] as const)("decides %j at %d -> %j", (context, at, expected) => { + expect(fromThreadCreated(threadCreated, context, at)).toEqual(expected); + }); + + it("completed turn's reply lands in ReplyPending verbatim and drops the coordinates", () => { + expect(toReplyPending(threadCreated, answer, now)).toEqual({ + ...exchangeBase, + tag: "reply-pending", + reply: answer, + }); + }); + + it("a turn-start rejection keeps the coordinates inside the reply only", () => { + expect(toRejected(threadCreated, rejection, now).reply).toEqual({ + type: "failure", + text: rejection.reason, + cause: { + type: "rejected", + method: rejection.method, + state: { tag: "thread-created", t3: coordinates }, + }, + }); + }); + + it("expiry keeps the coordinates inside the reply only", () => { + expect(toExpired(threadCreated, later).reply).toEqual({ + type: "failure", + text: "T3 did not answer in time.", + cause: { type: "expired", state: { tag: "thread-created", t3: coordinates } }, + }); + }); +}); + +describe("ReplyPending", () => { + const replyPending = toReplyPending(threadCreated, answer, now); + + // missing platform reply -> post it, unless expired; posted -> record its message id, even when expired + it.each([ + [{ platformReply: "missing" }, now, { type: "post-reply" }], + [ + { platformReply: "posted", replySourceUri: "test://exchange/reply" }, + now, + { type: "record-reply-posted", replySourceUri: "test://exchange/reply" }, + ], + [{ platformReply: "missing" }, later, { type: "expire" }], + [ + { platformReply: "posted", replySourceUri: "test://exchange/reply" }, + later, + { type: "record-reply-posted", replySourceUri: "test://exchange/reply" }, + ], + ] as const)("decides %j at %d -> %j", (context, at, expected) => { + expect(fromReplyPending(replyPending, context, at)).toEqual(expected); + }); + + it("accepted delivery lands in ReplyPosted with the platform message id", () => { + expect(toReplyPosted(replyPending, "test://exchange/reply", now)).toEqual({ + ...exchangeBase, + tag: "reply-posted", + reply: answer, + replySourceUri: "test://exchange/reply", + }); + }); + + it("definitive rejection lands in Undeliverable with the reply and cause", () => { + const cause = { message: "original message was deleted" } as const; + expect(toUndeliverable(replyPending, cause, now)).toEqual({ + ...exchangeBase, + tag: "undeliverable", + reply: answer, + cause, + }); + }); +}); + +describe("isExpired", () => { + const replyPending = toReplyPending(threadCreated, answer, now); + + it.each<[string, NonTerminalExchange]>([ + ["request-accepted", accepted], + ["work-planned", planned], + ["thread-created", threadCreated], + ["reply-pending", replyPending], + ])("%s is fresh at its own updatedAt and expired two hours later", (_, state) => { + expect(isExpired(state, now)).toBe(false); + expect(isExpired(state, later)).toBe(true); + }); +}); + +describe("isUpdateOf", () => { + const replyPending = toReplyPending(threadCreated, answer, now); + const posted = toReplyPosted(replyPending, "test://exchange/reply", now); + const undeliverable = toUndeliverable(replyPending, { message: "gone" }, now); + + it.each<[string, Exchange, Exchange]>([ + ["the same state rewritten", accepted, accepted], + ["accepted -> planned", planned, accepted], + ["accepted -> reply-pending", toRejected(accepted, rejection, now), accepted], + ["planned -> thread-created", threadCreated, planned], + ["planned -> reply-pending", toRejected(planned, rejection, now), planned], + ["thread-created -> reply-pending", replyPending, threadCreated], + ["reply-pending -> reply-posted", posted, replyPending], + ["reply-pending -> undeliverable", undeliverable, replyPending], + ])("accepts %s", (_, next, previous) => { + expect(isUpdateOf(next, previous)).toBe(true); + }); + + it.each<[string, Exchange, Exchange]>([ + ["going back", planned, threadCreated], + ["skipping ahead", threadCreated, accepted], + ["moving a posted reply", replyPending, posted], + ["moving an undeliverable reply", posted, undeliverable], + ["another exchange", { ...planned, sourceUri: "test://exchange/other" }, accepted], + ])("refuses %s", (_, next, previous) => { + expect(isUpdateOf(next, previous)).toBe(false); + }); +}); + +describe("getThreadId", () => { + const settled = { + type: "failure", + text: "T3 failed", + cause: { type: "settled", ...turn }, + } as const; + + it.each<[string, Exchange, ThreadId | null]>([ + ["nothing before planning", accepted, null], + ["the coordinates while planned", planned, coordinates.threadId], + ["the coordinates while the thread exists", threadCreated, coordinates.threadId], + ["the answer's thread", toReplyPending(threadCreated, answer, now), turn.threadId], + ["a settled failure's thread", toReplyPending(threadCreated, settled, now), turn.threadId], + ["nothing for a planning rejection", toRejected(accepted, rejection, now), null], + [ + "the planned thread for a later rejection", + toRejected(planned, rejection, now), + coordinates.threadId, + ], + ["nothing for a planning expiry", toExpired(accepted, later), null], + ["the planned thread for a later expiry", toExpired(planned, later), coordinates.threadId], + ])("finds %s", (_, exchange, expected) => { + expect(getThreadId(exchange)).toBe(expected); + }); +}); + +/* +Type-level: forward-only is structural. Never executed — typecheck enforces +these. If an `@ts-expect-error` stops erroring, a constructor's input type +widened and the forward-only guarantee broke. +*/ +const _forwardOnly = (posted: ReplyPosted, accepted: RequestAccepted, created: ThreadCreated) => { + // @ts-expect-error terminal states cannot re-enter thread creation + toThreadCreated(posted, now); + // @ts-expect-error a bare acceptance cannot record a posted reply + toReplyPosted(accepted, "reply://msg", now); + // @ts-expect-error Undeliverable is entered only from ReplyPending + toUndeliverable(accepted, { message: "cause" }, now); + // @ts-expect-error planning happens once, before the thread exists + toWorkPlanned(created, coordinates, now); +}; diff --git a/apps/server/src/ntbs/exchange.ts b/apps/server/src/ntbs/exchange.ts new file mode 100644 index 000000000000..7c3928e3bfc9 --- /dev/null +++ b/apps/server/src/ntbs/exchange.ts @@ -0,0 +1,568 @@ +import type { ChatAttachment, MessageId, ProjectId, ThreadId, TurnId } from "@t3tools/contracts"; +import { Duration } from "effect"; + +/* +This file defines the durable state and pure business rules for an exchange: +one admitted external request, its T3 work, and delivery of the eventual reply back to the originating platform. +*/ + +export type Request = { + /** + * Adapter-encoded URI locating the originating platform message, + * e.g. `discord:////` or + * `jira:///issue//comment/`. + * + * Two contracts: + * + * Identity — the same platform request must carry the same string + * across redeliveries and restarts; distinct requests must carry + * distinct strings. This is the durable dedup key `findBySourceUri` + * looks up, the key the processor serializes concurrent deliveries + * on, and the natural unique key for the repository's stored records. + * + * Addressability — it must contain everything needed to reach the + * message through the platform API from a cold start, because + * recovery reposts with only the stored record. A Discord message + * ID alone fails this: replying requires the channel ID too. + * + * Only the adapter that wrote it may parse it; the processor treats + * it as an opaque string. + */ + readonly sourceUri: string; + /** + * The captured source text sent as the first T3 user message. + * Platform independent. + * Must not exceed T3's 120,000-character input limit. + */ + readonly snapshot: string; + /** + * References to attachments stored by T3 and sent with the first user message. + * The processor creates them from attachment data provided by the adapter. + */ + readonly attachments: ReadonlyArray; +}; + +/** + * Describes _where_ the T3 work goes. Set by the platform-specific inbound code. + */ +export type T3Target = { + readonly projectId: ProjectId; + /** + * The starting point for the thread's worktree: the new branch is created from this one. + * + * Must be a branch that exists on `origin`; it is resolved there, so the worktree starts from the + * latest remote commit even when the local copy is behind. Tags, commit SHAs and local-only + * branches are rejected. + */ + readonly startBranchName: string; +}; + +/** Stable identifiers and locations for an exchange's T3 work. */ +export type WorkCoordinates = { + readonly projectId: ProjectId; + /** + * The branch this work starts from, and the commit it pointed at on `origin` when the work was planned. + * We keep the same SHA across retries so the request always runs against the code selected when it was planned, even if the branch moves later. The name is recorded as the worktree's merge base for later diff and PR flows. + */ + readonly startBranchName: string; + readonly startCommitSha: string; + // Planned while WorkPlanned; confirmed by ThreadCreated. + readonly threadId: ThreadId; + /** + * The first T3 user message created for this external request. + * This identifies the correct turn and reply even if the thread later receives other messages. + */ + readonly userMessageId: MessageId; + /** The branch minted for this request's worktree. */ + readonly worktreeBranchName: string; +}; + +/** Identifiers locating the T3 turn a reply came out of. */ +export type TurnCoordinates = { + readonly threadId: ThreadId; + readonly userMessageId: MessageId; + readonly turnId: TurnId; +}; + +/** + * Why an exchange ended in a failure reply, with the T3 identifiers that existed when it happened. + * Every variant is plain data so the stored reply survives JSON encoding; diagnostics that are not (the underlying error) are logged where the failure is caught, not stored. + */ +export type FailureCause = + /** + * T3 rejected the one action of the state the exchange was in; `method` names the T3 call that answered. + * `state` is that state reduced to its T3 data: nothing for a rejected target, the planned coordinates otherwise. + */ + | { + readonly type: "rejected"; + readonly method: string; + readonly state: + | Pick + | Pick + | Pick; + } + /** T3 settled the thread without an answer to our message; observed, not a rejection. `turnId` is null when no turn ever adopted the message. */ + | { + readonly type: "settled"; + readonly threadId: ThreadId; + readonly userMessageId: MessageId; + readonly turnId: TurnId | null; + } + /** The state outlived its deadline and its action was given up. `state` is reduced like in `rejected`. */ + | { + readonly type: "expired"; + readonly state: + | Pick + | Pick + | Pick; + }; + +export type ReplyAnswer = TurnCoordinates & { + readonly type: "answer"; + readonly text: string; +}; + +export type ReplyFailure = { + readonly type: "failure"; + readonly text: string; + readonly cause: FailureCause; +}; + +export type ReplyCancellation = TurnCoordinates & { + readonly type: "cancellation"; + readonly text: string; +}; + +/** + * What gets posted back to the platform. Each variant carries the T3 context that produced it, so delivery and audit need nothing else from the exchange. + */ +export type Reply = ReplyAnswer | ReplyFailure | ReplyCancellation; + +export type UndeliverableCause = { + readonly message: string; +}; + +/** + * The data every exchange carries, whatever state it has reached: the request and where its work was meant to go. + */ +export type ExchangeBase = Request & { + readonly target: T3Target; + /** Epoch millis of when the request was recorded, which is when it was accepted. */ + readonly createdAt: number; + /** + * Epoch millis of the last transition. + * The processor never rewrites a state in place, so this is when the current state began. + */ + readonly updatedAt: number; +}; + +/** + * The platform inbound code (Jira Webhook e.g.) admitted the request, + * trigger and actor checks passed, and the processor recorded it. + * + * Nothing in T3 exists yet. From here, the processor alone drives the exchange to a terminal state, and everything that fails from here on becomes a failure reply like any other. + */ +export type RequestAccepted = ExchangeBase & { + readonly tag: "request-accepted"; +}; + +/** + * T3 accepted the target and the identifiers for its work are minted and stored. + * Still nothing created in T3: the thread, worktree and turn come next. + */ +export type WorkPlanned = ExchangeBase & { + readonly tag: "work-planned"; + readonly t3: WorkCoordinates; +}; + +/** + * The planned T3 thread exists. The first turn may not have started yet. Turn existence and progress are T3-owned. + */ +export type ThreadCreated = ExchangeBase & { + readonly tag: "thread-created"; + readonly t3: WorkCoordinates; +}; + +/** + * A reply exists; the exact payload is stored verbatim so every posting attempt sends the same content. + * Reached from any earlier state: a rejection during planning, provisioning or turn start lands here as much as a finished turn does, so this and later states do not imply the thread existed. + * Delivery needs only `sourceUri` and the reply; what T3 produced it lives inside the reply. + */ +export type ReplyPending = ExchangeBase & { + readonly tag: "reply-pending"; + readonly reply: Reply; +}; + +/** + * Terminal state. + * The platform accepted the reply; its message ID is stored. + */ +export type ReplyPosted = ExchangeBase & { + readonly tag: "reply-posted"; + readonly reply: Reply; + readonly replySourceUri: string; +}; + +/** + * Terminal state. + * A finished reply exists, but the platform definitively rejected delivery. + * Stores the undelivered payload and the cause. + * Common causes could be: the original discussion or message has been deleted + * or locked (Jira/Github issue, Discord thread), the bot has been kicked, etc. + * The tombstone keeps dedup intact and stops the processor from retrying forever. + */ +export type Undeliverable = ExchangeBase & { + readonly tag: "undeliverable"; + readonly reply: Reply; + readonly cause: UndeliverableCause; +}; + +/** Exchanges that still have work left to do. */ +export type NonTerminalExchange = RequestAccepted | WorkPlanned | ThreadCreated | ReplyPending; + +/** Exchanges that have finished, with the reply either posted or undeliverable. */ +export type TerminalExchange = ReplyPosted | Undeliverable; + +/** + * One exchange between an external platform and T3, from request acceptance through + * final-reply delivery. The tag says how far it got; the repository stores the + * latest value per `sourceUri` so non-terminal exchanges resume after a restart. + */ +export type Exchange = NonTerminalExchange | TerminalExchange; + +export const isTerminal = (state: Exchange): state is TerminalExchange => { + // An exhaustive switch makes new lifecycle states require an explicit classification. + // This way it is impossible to break the program semantics by adding a new state + // and forgetting to deal with it, because it would not typecheck. + switch (state.tag) { + case "reply-posted": + case "undeliverable": + return true; + + case "request-accepted": + case "work-planned": + case "thread-created": + case "reply-pending": + return false; + } +}; + +export const isNonTerminal = (state: Exchange): state is NonTerminalExchange => !isTerminal(state); + +/** The states an exchange may move to from each state; the same table the constructors below enforce in types. */ +const successors: { readonly [Tag in Exchange["tag"]]: ReadonlyArray } = { + "request-accepted": ["work-planned", "reply-pending"], + "work-planned": ["thread-created", "reply-pending"], + "thread-created": ["reply-pending"], + "reply-pending": ["reply-posted", "undeliverable"], + "reply-posted": [], + undeliverable: [], +}; + +/** + * Whether `next` is a later version of the same exchange as `previous`: the same state rewritten, or a state the exchange may legally move to. Going back, skipping ahead, and moving a terminal exchange are all refused. + */ +export const isUpdateOf = (next: Exchange, previous: Exchange): boolean => + next.sourceUri === previous.sourceUri && + (next.tag === previous.tag || successors[previous.tag].includes(next.tag)); + +/** + * How long an exchange may stay in each non-terminal state, in millis, before its action is given up. + * First guesses: planning and provisioning fail on git and the filesystem, a turn on the agent, delivery on the platform. + */ +const deadlines: { readonly [Tag in NonTerminalExchange["tag"]]: number } = { + "request-accepted": Duration.toMillis(Duration.minutes(5)), + "work-planned": Duration.toMillis(Duration.minutes(15)), + "thread-created": Duration.toMillis(Duration.hours(1)), + "reply-pending": Duration.toMillis(Duration.hours(1)), +}; + +/** Whether the current state is older than its deadline. */ +export const isExpired = (state: NonTerminalExchange, now: number): boolean => + now - state.updatedAt > deadlines[state.tag]; + +const getFailureThreadId = (cause: FailureCause): ThreadId | null => { + switch (cause.type) { + case "settled": + return cause.threadId; + + case "rejected": + case "expired": + return "t3" in cause.state ? cause.state.t3.threadId : null; + } +}; + +const getReplyThreadId = (reply: Reply): ThreadId | null => { + switch (reply.type) { + case "answer": + case "cancellation": + return reply.threadId; + + case "failure": + return getFailureThreadId(reply.cause); + } +}; + +/** + * The T3 thread this exchange refers to, wherever the state keeps it: in `t3` while work is in progress, inside the reply afterwards. Null before planning and when planning was rejected. + */ +export const getThreadId = (exchange: Exchange): ThreadId | null => { + switch (exchange.tag) { + case "request-accepted": + return null; + + case "work-planned": + case "thread-created": + return exchange.t3.threadId; + + case "reply-pending": + case "reply-posted": + case "undeliverable": + return getReplyThreadId(exchange.reply); + } +}; + +export const makeRequestAccepted = ( + request: Request, + target: T3Target, + now: number, +): RequestAccepted => ({ + ...request, + target, + createdAt: now, + updatedAt: now, + tag: "request-accepted", +}); + +/* +Decider/Policy pattern. + +The decider answers: given the stored state and the relevant live context, what should happen next? + +decider: (state, context) -> action + +The decider/policy pattern is important in the NTBS module because we have to frequently ask: "given this information I have about the exchange and this context (e.g. checking external platforms or t3 thread states) what should we do next?" + +A decision does not transition the exchange. The processor first executes the chosen effect; only after it succeeds does the processor construct the legal transition, passing along any result data the effect produced. +The reconciliation flow is: + +1. load state effect +2. retrieve observations effect +3. make decision pure +4. execute decision effect +5. construct the transition from its result pure, then persist as an effect + +Every decider also receives the current time. When the observation shows the state's action is still needed and the state is past its deadline, the decision is to expire instead. An observation that completes the state wins over expiry, so a late result is still recorded. +RequestAccepted observes nothing but the clock: planning creates nothing in T3, so there is nothing else to check before doing it. +*/ + +export type WorkPlannedContext = { readonly thread: "missing" } | { readonly thread: "present" }; + +export type ThreadCreatedContext = + | { + readonly turn: "missing"; + } + | { readonly turn: "active" } + | { readonly turn: "completed"; readonly reply: Reply }; + +export type ReplyPendingContext = + | { + readonly platformReply: "missing"; + } + | { + readonly platformReply: "posted"; + readonly replySourceUri: string; + }; + +// Picks the base fields so states that must not carry `t3` do not inherit it at runtime. +const baseOf = ({ + sourceUri, + snapshot, + attachments, + target, + createdAt, + updatedAt, +}: ExchangeBase): ExchangeBase => ({ + sourceUri, + snapshot, + attachments, + target, + createdAt, + updatedAt, +}); + +export const toWorkPlanned = ( + state: RequestAccepted, + coordinates: WorkCoordinates, + now: number, +): WorkPlanned => ({ + ...state, + updatedAt: now, + tag: "work-planned", + t3: coordinates, +}); + +export const toThreadCreated = (state: WorkPlanned, now: number): ThreadCreated => ({ + ...state, + updatedAt: now, + tag: "thread-created", +}); + +export const toReplyPending = ( + state: RequestAccepted | WorkPlanned | ThreadCreated, + reply: Reply, + now: number, +): ReplyPending => ({ + ...baseOf(state), + updatedAt: now, + tag: "reply-pending", + reply, +}); + +/** What T3 said when it rejected a step for good; `method` names the T3 call that answered. */ +export type Rejection = { + readonly reason: string; + readonly method: string; +}; + +/** + * T3 rejected the work for good while the exchange was in `state`. Each state has exactly one action, so the state itself records which step was rejected and what T3 data existed. + */ +export const toRejected = ( + state: RequestAccepted | WorkPlanned | ThreadCreated, + rejection: Rejection, + now: number, +): ReplyPending => + toReplyPending( + state, + { + type: "failure", + text: rejection.reason, + cause: { + type: "rejected", + method: rejection.method, + state: + state.tag === "request-accepted" ? { tag: state.tag } : { tag: state.tag, t3: state.t3 }, + }, + }, + now, + ); + +/** The state outlived its deadline; the reply tells the user we gave up and the cause records where. */ +export const toExpired = ( + state: RequestAccepted | WorkPlanned | ThreadCreated, + now: number, +): ReplyPending => + toReplyPending( + state, + { + type: "failure", + text: "T3 did not answer in time.", + cause: { + type: "expired", + state: + state.tag === "request-accepted" ? { tag: state.tag } : { tag: state.tag, t3: state.t3 }, + }, + }, + now, + ); + +export const toReplyPosted = ( + state: ReplyPending, + replySourceUri: string, + now: number, +): ReplyPosted => ({ + ...state, + updatedAt: now, + tag: "reply-posted", + replySourceUri, +}); + +export const toUndeliverable = ( + state: ReplyPending, + cause: UndeliverableCause, + now: number, +): Undeliverable => ({ + ...state, + updatedAt: now, + tag: "undeliverable", + cause, +}); + +export type RequestAcceptedDecision = { readonly type: "plan" } | { readonly type: "expire" }; + +export type WorkPlannedDecision = + | { readonly type: "provision-thread" } + | { readonly type: "record-thread-created" } + | { readonly type: "expire" }; + +export type ThreadCreatedDecision = + | { readonly type: "start-turn" } + | { readonly type: "wait" } + | { + readonly type: "record-reply-pending"; + readonly reply: Reply; + } + | { readonly type: "expire" }; + +export type ReplyPendingDecision = + | { readonly type: "post-reply" } + | { + readonly type: "record-reply-posted"; + readonly replySourceUri: string; + } + | { readonly type: "expire" }; + +export const fromRequestAccepted = (state: RequestAccepted, now: number): RequestAcceptedDecision => + isExpired(state, now) ? { type: "expire" } : { type: "plan" }; + +export const fromWorkPlanned = ( + state: WorkPlanned, + context: WorkPlannedContext, + now: number, +): WorkPlannedDecision => { + switch (context.thread) { + case "missing": + return isExpired(state, now) ? { type: "expire" } : { type: "provision-thread" }; + + case "present": + return { type: "record-thread-created" }; + } +}; + +export const fromThreadCreated = ( + state: ThreadCreated, + context: ThreadCreatedContext, + now: number, +): ThreadCreatedDecision => { + switch (context.turn) { + case "missing": + return isExpired(state, now) ? { type: "expire" } : { type: "start-turn" }; + + case "active": + return isExpired(state, now) ? { type: "expire" } : { type: "wait" }; + + case "completed": + return { + type: "record-reply-pending", + reply: context.reply, + }; + } +}; + +export const fromReplyPending = ( + state: ReplyPending, + context: ReplyPendingContext, + now: number, +): ReplyPendingDecision => { + switch (context.platformReply) { + case "missing": + return isExpired(state, now) ? { type: "expire" } : { type: "post-reply" }; + + case "posted": + return { + type: "record-reply-posted", + replySourceUri: context.replySourceUri, + }; + } +}; diff --git a/apps/server/src/ntbs/processor.test.ts b/apps/server/src/ntbs/processor.test.ts new file mode 100644 index 000000000000..6193a56dadc6 --- /dev/null +++ b/apps/server/src/ntbs/processor.test.ts @@ -0,0 +1,1548 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Clock, Deferred, Effect, Fiber, Layer, Queue, Stream } from "effect"; +import { TestClock } from "effect/testing"; +import { + ExchangeRepository, + ExchangeRepositoryError, + inMemoryExchangeRepository, +} from "./ExchangeRepository.ts"; +import { makeNTBSProcessor, type NTBSProcessor } from "./processor.ts"; +import { MessageId, ProjectId, ThreadId, TurnId } from "@t3tools/contracts"; +import { FatalError, RetryableError, T3Gateway } from "./t3gateway.ts"; +import { AdapterError, NTBSAdapter, ReplyRejected } from "./adapter.ts"; +import { + makeRequestAccepted, + toExpired, + toRejected, + toReplyPending, + toReplyPosted, + toThreadCreated, + toUndeliverable, + toWorkPlanned, + type Exchange, + type Request, + type T3Target, + type WorkCoordinates, +} from "./exchange.ts"; + +/** + * The test configuration of the services. + * While T3Gateway tests took flags as input (failX?: boolean, etc), this does not scale for processor test, because instead of a single call we're often testing an entire choreography of events and how the processor behaves in that situation. + * + * Flags, on the other hand, work when behaviors are small, enumerable and reused. Scripts pay off for testing choreography. + */ +type ServiceInput = { + readonly t3Gateway?: Partial; + readonly adapter?: Partial; +}; + +type Call = { + service: string; + method: string; + args: ReadonlyArray; +}; + +/** + * Everything the test harness hands back to the file. + * + * The assembled service as well as probes to look into its state. + */ +type ProcessorTestContext = { + /** The subject under test, built from mocked services.*/ + readonly processor: NTBSProcessor; + /** Assesses the persisted state in most of the tests. What has been recorded about the events and changes? */ + readonly repository: ExchangeRepository; + /** The shared ordered call log. We know what has been dispatched and with which arguments. This is not testing internals but actual business-logic. */ + readonly calls: ReadonlyArray; + /** Pushes a threadId to the `threadActivity` stream, waking up the processor. */ + readonly pingActivity: (threadId: ThreadId) => Effect.Effect; + /** + * Polls the repository until the exchange at `sourceUri` carries `tag`. + * A Deferred signalled from inside a mock fires before the processor persists the transition, so anything that asserts on stored state after a mock call must wait on the store itself. + */ + readonly awaitStoredTag: ( + sourceUri: string, + tag: Exchange["tag"], + ) => Effect.Effect; +}; + +const request: Request = { + sourceUri: "test://request/1", + snapshot: "Please fix the bug", + attachments: [], +}; + +const defaultProjectId = ProjectId.make("defaultProjectId"); +const defaultThreadId = ThreadId.make("defaultThreadId"); +const defaultUserMessageId = MessageId.make("defaultUserMessageId"); + +const target: T3Target = { + projectId: defaultProjectId, + startBranchName: "fork/dev", +}; + +const defaultWorkCoordinates: WorkCoordinates = { + projectId: defaultProjectId, + startBranchName: "fork/dev", + startCommitSha: "start-commit-sha", + threadId: defaultThreadId, + userMessageId: defaultUserMessageId, + worktreeBranchName: "ntbs/defaultThreadId", +}; + +const secondRequest: Request = { + ...request, + sourceUri: "test://request/2", +}; + +const secondThreadId = ThreadId.make("secondThreadId"); + +const secondWorkCoordinates: WorkCoordinates = { + ...defaultWorkCoordinates, + startCommitSha: "second-start-commit-sha", + threadId: secondThreadId, + userMessageId: MessageId.make("secondUserMessageId"), + worktreeBranchName: "ntbs/secondThreadId", +}; + +const postedReplyUri = "test://reply/1"; + +/** The TestClock starts at epoch, so every transition the processor makes without adjusting it is stamped 0. */ +const now = 0; + +/** The states the default request walks through, for asserting on stored records. */ +const accepted = makeRequestAccepted(request, target, now); +const planned = toWorkPlanned(accepted, defaultWorkCoordinates, now); +const threadCreated = toThreadCreated(planned, now); + +const secondThreadCreated = toThreadCreated( + toWorkPlanned(makeRequestAccepted(secondRequest, target, now), secondWorkCoordinates, now), + now, +); + +/** An answer out of the turn our message started on the given coordinates. */ +const answer = (text: string, coordinates: WorkCoordinates = defaultWorkCoordinates) => + ({ + type: "answer", + text, + threadId: coordinates.threadId, + userMessageId: coordinates.userMessageId, + turnId: TurnId.make(`turn-${coordinates.threadId}`), + }) as const; + +/** + * Happy-path defaults: + * - fresh request flowing to a started turn + * - nothing settled yet + * - replies deliverable + * + * Each test overrides only the methods its scenario changes. + */ +const defaultT3Gateway: Omit = { + planCoordinates: () => Effect.succeed(defaultWorkCoordinates), + getThreadStatus: () => Effect.succeed({ thread: "missing" }), + provisionThread: () => Effect.void, + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => Effect.void, +}; + +const defaultAdapter: NTBSAdapter = { + acknowledge: () => Effect.void, + postReply: () => Effect.succeed(postedReplyUri), + findPostedReply: () => Effect.succeed(null), +}; + +/** + * The harness. + * + * The term "harness" comes from electrical engineering for describing hardware test benches: the wiring harness is the fixed rig that holds the device under test and connects it to instruments, so each experiment only varies the stimulus. + * + * In software it means the same thing: the _fixed_ part of the test setup such as system assembly, instrumentation, probes, as opposed to fixtures (the data) and tests (the scenarios). + * + * `withProcessor` is our harness. + * + * generics allow for our test callback to pass through its types. + * We never specify A and E manually, and we rarely care, but if we ever have to chain the result of withProcessor or do anything with its returned value they are useful to avoid spreading `any`s. + */ +const withProcessor = ( + servicesInput: ServiceInput, + test: (context: ProcessorTestContext) => Effect.Effect, +) => + Effect.gen(function* () { + const calls: Call[] = []; + const activity = yield* Queue.unbounded(); + + /** + * Records the call, then runs the wrapped behavior. + * Recording lives here so no implementation or override can forget it. + * The push happens when the effect runs, not when it's created, which keeps the log ordering honest in the concurrency tests. + */ + const wrap = + (service: string) => + , B, E2>( + method: string, + fn: (...args: Args) => Effect.Effect, + ) => + (...args: Args) => + Effect.suspend(() => { + calls.push({ service, method, args }); + return fn(...args); + }); + + const wrapT3 = wrap("T3Gateway"); + const wrapAdapter = wrap("NTBSAdapter"); + + const t3 = { ...defaultT3Gateway, ...servicesInput.t3Gateway }; + const adapter = { ...defaultAdapter, ...servicesInput.adapter }; + + const layer = Layer.mergeAll( + Layer.mock(T3Gateway, { + planCoordinates: wrapT3("planCoordinates", t3.planCoordinates), + getThreadStatus: wrapT3("getThreadStatus", t3.getThreadStatus), + getTurnStatus: wrapT3("getTurnStatus", t3.getTurnStatus), + provisionThread: wrapT3("provisionThread", t3.provisionThread), + startTurn: wrapT3("startTurn", t3.startTurn), + threadActivity: Stream.fromQueue(activity), + }), + Layer.mock(NTBSAdapter, { + acknowledge: wrapAdapter("acknowledge", adapter.acknowledge), + findPostedReply: wrapAdapter("findPostedReply", adapter.findPostedReply), + postReply: wrapAdapter("postReply", adapter.postReply), + }), + inMemoryExchangeRepository, + ); + + return yield* Effect.gen(function* () { + const processor = yield* makeNTBSProcessor; + const repository = yield* ExchangeRepository; + + return yield* test({ + processor, + repository, + calls, + pingActivity: (threadId) => Queue.offer(activity, threadId).pipe(Effect.asVoid), + awaitStoredTag: (sourceUri, tag) => + Effect.gen(function* () { + while (true) { + const state = yield* repository.findBySourceUri(sourceUri); + if (state !== null && state.tag === tag) { + return state; + } + yield* Effect.yieldNow; + } + }), + }); + }).pipe(Effect.provide(layer)); + }); + +describe("NTBSProcessor", () => { + /* + Harness smoke test: the happy-path defaults drive a fresh request to ThreadCreated with a started turn, and the shared log shows the full cross-service pipeline in order. + */ + it.effect("records a fresh request and starts its turn on the default behaviors", () => + withProcessor({}, ({ processor, repository, calls }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + + // Still ThreadCreated: a successful startTurn transitions nothing, the exchange only moves when getTurnStatus observes a settled turn. + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + }), + ), + ); + + /* + `process` is idempotent per sourceUri: a redelivery finds the recorded exchange and returns without touching T3 or the adapter. + The smoke test above pins the exact pipeline, so here we check the log length twice: once after the first delivery to prove it actually did the work, once after the second to prove it added nothing — the log is append-only, so an unchanged length means zero service calls. + */ + it.effect("starts a new request and ignores its sequential redelivery", () => + withProcessor({}, ({ processor, repository, calls }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + // The five T3 pipeline steps plus the acknowledgement. + expect(calls.length).toBe(6); + + yield* processor.process(request, target); + + expect(calls.length).toBe(6); + + // And the stored exchange is still the one the first delivery produced. + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + }), + ), + ); + + /* + The acknowledgement is best-effort: a failing `acknowledge` must not stop the pipeline. + The log proves the failure was swallowed in place, the T3 steps after it still ran, and the exchange still reached ThreadCreated. + */ + it.effect("continues after a best-effort acknowledgement fails", () => + withProcessor( + { + adapter: { + acknowledge: () => + new AdapterError({ + reason: "The acknowledgement could not be posted", + cause: "test failure", + }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + }), + ), + ); + + /* + A stored ThreadCreated whose turn is still running is a no-op, whether startup recovery or thread activity looks at it. + The processor asks T3 for the turn status and stops there: no planning, no acknowledgement, no second startTurn, no reply lookup. + */ + it.effect("leaves an exchange unchanged while its turn is active", () => { + const recoveryStatusRead = Deferred.makeUnsafe(); + const activityStatusRead = Deferred.makeUnsafe(); + let statusReads = 0; + + return withProcessor( + { + t3Gateway: { + getTurnStatus: () => + Effect.gen(function* () { + statusReads += 1; + yield* Deferred.succeed( + statusReads === 1 ? recoveryStatusRead : activityStatusRead, + undefined, + ); + return { turn: "active" as const }; + }), + }, + }, + ({ processor, repository, calls, pingActivity }) => + Effect.gen(function* () { + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(recoveryStatusRead); + yield* Effect.yieldNow; + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + ]); + + yield* pingActivity(defaultThreadId); + yield* Deferred.await(activityStatusRead); + // Give the processor a chance to do anything else it might wrongly want to do after the status reads. + yield* Effect.yieldNow; + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + "T3Gateway.getTurnStatus", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + T3 refusing the target for good (a branch that is not on origin, say) is found out at planning, before anything from T3 exists. + The request was already recorded, so the rejection becomes a failure reply like any other and is posted; the stored record never carries T3 coordinates. + */ + it.effect("posts a failure reply when T3 rejects the request at planning", () => { + const rejection = new FatalError({ + reason: "Branch 'nope' does not exist on origin", + cause: null, + method: "gitWorkflowService.resolveRemoteTrackingCommit", + }); + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => rejection, + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + + const replyPending = toRejected(accepted, rejection, now); + expect(replyPending.reply).toEqual({ + type: "failure", + text: rejection.reason, + cause: { + type: "rejected", + method: rejection.method, + state: { tag: "request-accepted" }, + }, + }); + expect(calls.at(-1)?.args).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toReplyPosted(replyPending, postedReplyUri, now), + ); + }), + ); + }); + + /* + A transient planning failure happens after the request was recorded, so the record survives it: the delivery dies, the exchange stays RequestAccepted. + A redelivery finds the record and plans nothing. The sweeper then re-plans from the record and drives the exchange on to its turn. + */ + it.effect( + "keeps an accepted request whose planning failed and re-plans it on the next sweep", + () => { + const turnStarted = Deferred.makeUnsafe(); + let planCalls = 0; + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + if (planCalls === 1) { + return yield* new RetryableError({ + reason: "Could not fetch origin", + cause: "test failure", + method: "planCoordinates", + }); + } + return defaultWorkCoordinates; + }), + startTurn: () => Deferred.succeed(turnStarted, undefined), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + // Started first so that the sweep, not startup recovery, is what re-plans. + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + const exit = yield* Effect.exit(processor.process(request, target)); + expect(exit._tag).toBe("Failure"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + // Redelivery after acceptance does no planning. + yield* processor.process(request, target); + expect(calls.length).toBe(1); + + yield* TestClock.adjust("1 minute"); + yield* Deferred.await(turnStarted); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + // The sweep resumes from the record. + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + // The sweep re-planned a minute in, so its transitions carry that time. + const sweptAt = yield* Clock.currentTimeMillis; + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toThreadCreated(toWorkPlanned(accepted, defaultWorkCoordinates, sweptAt), sweptAt), + ); + + yield* Fiber.interrupt(run); + }), + ); + }, + ); + + /* + A transient provisioning failure leaves the exchange at WorkPlanned; the delivery dies, the record survives. + Startup recovery then picks the exchange up where it stopped: it re-checks the thread, provisions it, and carries on to the turn. + Planning is not repeated, the coordinates were already persisted. + */ + it.effect("retries a transient provisioning failure during later recovery", () => { + const turnStarted = Deferred.makeUnsafe(); + let provisionCalls = 0; + + return withProcessor( + { + t3Gateway: { + provisionThread: () => + Effect.gen(function* () { + provisionCalls += 1; + if (provisionCalls === 1) { + return yield* new RetryableError({ + reason: "Thread provisioning temporarily failed", + cause: "test failure", + method: "provisionThread", + }); + } + }), + startTurn: () => Deferred.succeed(turnStarted, undefined), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const exit = yield* Effect.exit(processor.process(request, target)); + expect(exit._tag).toBe("Failure"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(planned); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(turnStarted); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + // Recovery resumes from the persisted plan. + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + A transient turn-start failure happens after ThreadCreated was persisted, so the exchange stays there and the delivery dies. + Recovery resumes from ThreadCreated: the thread is neither re-checked nor re-provisioned, and the acknowledgement is not repeated. Only the turn is retried. + */ + it.effect("retries a transient turn-start failure during later recovery", () => { + const turnStarted = Deferred.makeUnsafe(); + let startTurnCalls = 0; + + return withProcessor( + { + t3Gateway: { + startTurn: () => + Effect.gen(function* () { + startTurnCalls += 1; + if (startTurnCalls === 1) { + return yield* new RetryableError({ + reason: "Turn start temporarily failed", + cause: "test failure", + method: "startTurn", + }); + } + yield* Deferred.succeed(turnStarted, undefined); + }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const exit = yield* Effect.exit(processor.process(request, target)); + expect(exit._tag).toBe("Failure"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(turnStarted); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + // Recovery resumes from ThreadCreated. + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + Deliveries of the same sourceUri are serialized behind a per-source lock. + The first delivery is held inside planCoordinates with the request already recorded, so a naive second delivery would find the record and return while the first is still working on it. + Instead it waits: no calls from it while the first is blocked, and once released the log shows a single pipeline, the second delivery having found the record and returned. + */ + it.effect("serializes concurrent deliveries of the same request", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => + Effect.gen(function* () { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + return defaultWorkCoordinates; + }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(firstPlanStarted); + + const second = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + /* Fiber.pollUnsafe() is a synchronous, non-blocking peek at a fiber's state. It returns `undefined` if the fiber is still running. + It's an indirect soft-assertion that the second delivery is still suspended on the source lock. + */ + expect(second.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + yield* Fiber.join(first); + yield* Fiber.join(second); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + }), + ); + }); + + /* + The lock serializes, it does not couple outcomes. + When the first delivery fails inside planCoordinates, the request was already recorded, so the queued delivery finds the record and returns without planning again; recovery, not the redelivery, finishes the pipeline. + */ + it.effect("ignores a queued redelivery of an accepted request whose planning failed", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + const turnStarted = Deferred.makeUnsafe(); + let planCalls = 0; + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + return yield* new RetryableError({ + reason: "The first planning attempt failed", + cause: "test failure", + method: "planCoordinates", + }); + } + return defaultWorkCoordinates; + }), + startTurn: () => Deferred.succeed(turnStarted, undefined), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(firstPlanStarted); + + const second = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + expect(second.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + expect((yield* Fiber.await(first))._tag).toBe("Failure"); + yield* Fiber.join(second); + + // The redelivery found the record and added nothing to the log. + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(turnStarted); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + // Recovery plans again from the record. + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + The same, one step later: the first delivery fails inside getThreadStatus, after the plan was persisted. + The queued redelivery finds the plan and returns without touching any service, and it is recovery, not the redelivery, that eventually finishes the pipeline. + */ + it.effect("retains a persisted plan for later recovery and ignores its queued redelivery", () => { + const threadStatusStarted = Deferred.makeUnsafe(); + const releaseThreadStatus = Deferred.makeUnsafe(); + const turnStarted = Deferred.makeUnsafe(); + let threadStatusCalls = 0; + + return withProcessor( + { + t3Gateway: { + getThreadStatus: () => + Effect.gen(function* () { + threadStatusCalls += 1; + if (threadStatusCalls === 1) { + yield* Deferred.succeed(threadStatusStarted, undefined); + yield* Deferred.await(releaseThreadStatus); + return yield* new RetryableError({ + reason: "Failed after persisting the plan", + cause: "test failure", + method: "getThreadStatus", + }); + } + return { thread: "missing" as const }; + }), + startTurn: () => Deferred.succeed(turnStarted, undefined), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(threadStatusStarted); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(planned); + + const second = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + expect(second.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + ]); + + yield* Deferred.succeed(releaseThreadStatus, undefined); + expect((yield* Fiber.await(first))._tag).toBe("Failure"); + yield* Fiber.join(second); + + // The redelivery found the plan and added nothing to the log. + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(planned); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(turnStarted); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + // Recovery resumes from the persisted plan. + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + Interruption must release the source lock like any other exit, otherwise one cancelled delivery would wedge its sourceUri forever. + The first delivery is interrupted while holding the lock inside planCoordinates; the queued one then acquires it, finds the record the first one left, and returns. + */ + it.effect("releases the source lock when its holder is interrupted", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const keepFirstPlanBlocked = Deferred.makeUnsafe(); + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => + Effect.gen(function* () { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(keepFirstPlanBlocked); + return defaultWorkCoordinates; + }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(firstPlanStarted); + + const second = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + expect(second.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + yield* Fiber.interrupt(first); + yield* Fiber.join(second); + + // The queued delivery got the lock, found the record, and planned nothing. + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + }), + ); + }); + + /* + The other side of interruption: cancelling a delivery that is *waiting* for the lock must not disturb the holder or the lock itself. + The holder keeps running, a later delivery queues behind it as usual, and the final log is one pipeline with no extra plan. + */ + it.effect("interrupting a queued delivery preserves the lock for later deliveries", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => + Effect.gen(function* () { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + return defaultWorkCoordinates; + }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(firstPlanStarted); + + const interruptedWaiter = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + expect(interruptedWaiter.pollUnsafe()).toBeUndefined(); + + yield* Fiber.interrupt(interruptedWaiter); + + // The holder is unaffected: still blocked in planCoordinates, only the record stored. + expect(first.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + const later = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + expect(later.pollUnsafe()).toBeUndefined(); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + yield* Fiber.join(first); + yield* Fiber.join(later); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + }), + ); + }); + + /* + The lock is per sourceUri, not global. + While the first request is blocked in planCoordinates, a different request runs its whole pipeline to completion. + `planCoordinates` does not receive the request, so the mock hands out coordinates by call order. + */ + it.effect("allows different requests to proceed concurrently", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + let planCalls = 0; + + return withProcessor( + { + t3Gateway: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + return defaultWorkCoordinates; + } + return secondWorkCoordinates; + }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(firstPlanStarted); + + yield* processor.process(secondRequest, target); + + // The second request completed while the first is still held in planCoordinates. + expect(first.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + expect(yield* repository.findBySourceUri(secondRequest.sourceUri)).toEqual( + secondThreadCreated, + ); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + yield* Fiber.join(first); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + // The first request finishes its own pipeline once released. + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "T3Gateway.startTurn", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + }), + ); + }); + + /* + Startup recovery walks every non-terminal exchange. + A stored ThreadCreated whose turn has meanwhile completed is carried to ReplyPosted: the reply is looked up on the platform first, not found, then posted. + */ + it.effect("resumes non-terminal exchanges when run starts", () => { + const reply = answer("Recovered reply"); + + return withProcessor( + { + t3Gateway: { + getTurnStatus: () => Effect.succeed({ turn: "completed", reply }), + }, + }, + ({ processor, repository, calls, awaitStoredTag }) => + Effect.gen(function* () { + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + + const replyPending = toReplyPending(threadCreated, reply, now); + expect(calls.at(-1)?.args).toEqual([replyPending]); + expect(posted).toEqual(toReplyPosted(replyPending, postedReplyUri, now)); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + Thread activity is routed by threadId. An unknown thread is dropped without any service call; a stored one gets its turn status re-read. + Recovery sees the turn still active, so the reply only arrives through the ping. + */ + it.effect("routes thread activity only for stored exchanges", () => { + const reply = answer("Reply after thread activity"); + let statusReads = 0; + + return withProcessor( + { + t3Gateway: { + getTurnStatus: () => + Effect.sync(() => { + statusReads += 1; + return statusReads === 1 + ? { turn: "active" as const } + : { turn: "completed" as const, reply }; + }), + }, + }, + ({ processor, repository, calls, pingActivity, awaitStoredTag }) => + Effect.gen(function* () { + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* pingActivity(ThreadId.make("unknown-thread")); + yield* pingActivity(defaultThreadId); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + // Startup recovery. + "T3Gateway.getTurnStatus", + // The unknown thread contributed nothing; this is the stored thread's ping. + "T3Gateway.getTurnStatus", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + // Both status reads were for the stored exchange. + expect(calls[0]?.args).toEqual([threadCreated]); + expect(calls[1]?.args).toEqual([threadCreated]); + expect(posted).toEqual( + toReplyPosted(toReplyPending(threadCreated, reply, now), postedReplyUri, now), + ); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + Activity handling takes the same per-source lock as `process`. + While the ping's status read is held open, a redelivery of the request parks behind it instead of racing on the stored exchange; once released it finds the reply posted and returns without calls. + */ + it.effect("serializes thread activity with a redelivered request", () => { + const activityStatusStarted = Deferred.makeUnsafe(); + const releaseActivityStatus = Deferred.makeUnsafe(); + const reply = answer("Reply from thread activity"); + let statusReads = 0; + + return withProcessor( + { + t3Gateway: { + getTurnStatus: () => + Effect.gen(function* () { + statusReads += 1; + if (statusReads === 1) { + return { turn: "active" as const }; + } + yield* Deferred.succeed(activityStatusStarted, undefined); + yield* Deferred.await(releaseActivityStatus); + return { turn: "completed" as const, reply }; + }), + }, + }, + ({ processor, repository, calls, pingActivity, awaitStoredTag }) => + Effect.gen(function* () { + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* pingActivity(defaultThreadId); + yield* Deferred.await(activityStatusStarted); + + const redelivery = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Effect.yieldNow; + + expect(redelivery.pollUnsafe()).toBeUndefined(); + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + "T3Gateway.getTurnStatus", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Deferred.succeed(releaseActivityStatus, undefined); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + yield* Fiber.join(redelivery); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + "T3Gateway.getTurnStatus", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + expect(posted).toEqual( + toReplyPosted(toReplyPending(threadCreated, reply, now), postedReplyUri, now), + ); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + The happy path end to end: a fresh request whose thread already exists and whose turn completes immediately reaches ReplyPosted inside a single `process` call. + The log shows provisioning skipped for the present thread, and the reply posted with the ReplyPending state the completed turn produced. + */ + it.effect("posts a completed T3 reply", () => { + const reply = answer("The bug is fixed."); + + return withProcessor( + { + t3Gateway: { + getThreadStatus: () => Effect.succeed({ thread: "present" }), + getTurnStatus: () => Effect.succeed({ turn: "completed", reply }), + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "NTBSAdapter.acknowledge", + "T3Gateway.getTurnStatus", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + + const replyPending = toReplyPending(threadCreated, reply, now); + expect(calls.at(-1)?.args).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toReplyPosted(replyPending, postedReplyUri, now), + ); + }), + ); + }); + + /* + Reply discovery guards against double posting. + When the platform already shows the reply (a previous run posted it but crashed before persisting), the exchange is recorded as posted at the discovered URI and `postReply` is never called. + */ + it.effect("records a reply already found on the platform without posting it again", () => { + const reply = answer("Already delivered"); + const discoveredReplyUri = "test://reply/already-posted"; + + return withProcessor( + { + adapter: { + findPostedReply: () => Effect.succeed(discoveredReplyUri), + }, + }, + ({ processor, repository, calls, awaitStoredTag }) => + Effect.gen(function* () { + const replyPending = toReplyPending(threadCreated, reply, now); + yield* repository.upsert(replyPending); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "NTBSAdapter.findPostedReply", + ]); + expect(calls[0]?.args).toEqual([replyPending]); + expect(posted).toEqual(toReplyPosted(replyPending, discoveredReplyUri, now)); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + `ReplyRejected` is the platform saying the reply can never land (the originating discussion is gone, say). + Unlike an AdapterError, which leaves the exchange pending for a retry, a rejection is terminal: the exchange becomes Undeliverable with the platform's cause. + */ + it.effect("records a definitively rejected reply as undeliverable", () => { + const reply = answer("Reply that cannot be delivered"); + const rejectionCause = { message: "The originating discussion was deleted" }; + + return withProcessor( + { + adapter: { + postReply: () => new ReplyRejected({ cause: rejectionCause }), + }, + }, + ({ processor, repository, calls, awaitStoredTag }) => + Effect.gen(function* () { + const replyPending = toReplyPending(threadCreated, reply, now); + yield* repository.upsert(replyPending); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const undeliverable = yield* awaitStoredTag(request.sourceUri, "undeliverable"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + expect(undeliverable).toEqual(toUndeliverable(replyPending, rejectionCause, now)); + + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + A `FatalError` from a T3 action means T3 will never accept the work, so retrying is pointless. + The processor skips straight from the plan to a failure reply: no thread is created, so no acknowledgement and no turn; the user gets told why instead. + The reply keeps the planned coordinates as its context, the stored exchange does not. + */ + it.effect("delivers a failure reply when T3 rejects thread provisioning", () => { + const rejection = new FatalError({ + reason: "T3 cannot provision this request", + cause: { message: "The selected project no longer exists" }, + method: "provisionThread", + }); + + return withProcessor( + { + t3Gateway: { + provisionThread: () => rejection, + }, + }, + ({ processor, repository, calls }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + "T3Gateway.getThreadStatus", + "T3Gateway.provisionThread", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + + const replyPending = toRejected(planned, rejection, now); + expect(replyPending.reply).toEqual({ + type: "failure", + text: rejection.reason, + cause: { + type: "rejected", + method: rejection.method, + state: { tag: "work-planned", t3: defaultWorkCoordinates }, + }, + }); + expect(calls.at(-1)?.args).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toReplyPosted(replyPending, postedReplyUri, now), + ); + }), + ); + }); + + /* + Recovery from ReplyPending, the one non-terminal state the other recovery tests never start from. + A transient posting failure leaves the exchange pending; the next run repeats discovery and posting, and the second attempt lands. + */ + it.effect("retries a transient reply-posting failure during later recovery", () => { + const firstPostAttempted = Deferred.makeUnsafe(); + const reply = answer("Reply after a transient posting failure"); + let postAttempts = 0; + + return withProcessor( + { + adapter: { + postReply: () => + Effect.gen(function* () { + postAttempts += 1; + if (postAttempts === 1) { + yield* Deferred.succeed(firstPostAttempted, undefined); + return yield* new AdapterError({ + reason: "Reply posting temporarily failed", + cause: "test failure", + }); + } + return postedReplyUri; + }), + }, + }, + ({ processor, repository, calls, awaitStoredTag }) => + Effect.gen(function* () { + const replyPending = toReplyPending(threadCreated, reply, now); + yield* repository.upsert(replyPending); + + const firstRun = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(firstPostAttempted); + yield* Effect.yieldNow; + yield* Fiber.interrupt(firstRun); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(replyPending); + + const secondRun = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + expect(posted).toEqual(toReplyPosted(replyPending, postedReplyUri, now)); + + yield* Fiber.interrupt(secondRun); + }), + ); + }); + /* + `run` subscribes to thread activity before startup recovery, so a slow recovery does not delay live events. + Recovery is held open on the first exchange's status read while a ping for a second exchange is delivered; the second reaches ReplyPosted with recovery still blocked. + */ + it.effect("subscribes to thread activity before startup recovery finishes", () => { + const recoveryStarted = Deferred.makeUnsafe(); + const releaseRecovery = Deferred.makeUnsafe(); + const reply = answer("Reply posted while startup recovery is blocked", secondWorkCoordinates); + + return withProcessor( + { + t3Gateway: { + getTurnStatus: (state) => + state.sourceUri === request.sourceUri + ? Effect.gen(function* () { + yield* Deferred.succeed(recoveryStarted, undefined); + yield* Deferred.await(releaseRecovery); + return { turn: "active" as const }; + }) + : Effect.succeed({ turn: "completed" as const, reply }), + }, + }, + ({ processor, repository, calls, pingActivity, awaitStoredTag }) => + Effect.gen(function* () { + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(recoveryStarted); + + // Stored only now, so recovery could not have picked it up: the ping is its only way forward. + yield* repository.upsert(secondThreadCreated); + yield* pingActivity(secondThreadId); + const posted = yield* awaitStoredTag(secondRequest.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + "T3Gateway.getTurnStatus", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + expect(calls[0]?.args).toEqual([threadCreated]); + expect(calls[1]?.args).toEqual([secondThreadCreated]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + expect(posted).toEqual( + toReplyPosted(toReplyPending(secondThreadCreated, reply, now), postedReplyUri, now), + ); + + yield* Deferred.succeed(releaseRecovery, undefined); + yield* Fiber.interrupt(run); + }), + ); + }); + + /* + An accepted request whose planning never succeeds is not retried forever. + Once the record is older than its deadline the next run expires it instead of planning again: the user gets a failure reply, and a later sweep finds nothing left to do. + */ + it.effect("expires an accepted request whose planning never succeeded", () => + withProcessor( + { + t3Gateway: { + planCoordinates: () => + new RetryableError({ + reason: "Could not fetch origin", + cause: "test failure", + method: "planCoordinates", + }), + }, + }, + ({ processor, repository, calls, awaitStoredTag }) => + Effect.gen(function* () { + const exit = yield* Effect.exit(processor.process(request, target)); + expect(exit._tag).toBe("Failure"); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(accepted); + + yield* TestClock.adjust("6 minutes"); + const expiredAt = yield* Clock.currentTimeMillis; + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.planCoordinates", + // Recovery expires the record instead of planning again. + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + const replyPending = toExpired(accepted, expiredAt); + expect(calls.at(-1)?.args).toEqual([replyPending]); + expect(posted).toEqual(toReplyPosted(replyPending, postedReplyUri, expiredAt)); + + yield* TestClock.adjust("1 minute"); + expect(calls.length).toBe(3); + + yield* Fiber.interrupt(run); + }), + ), + ); + + /* + A turn T3 keeps reporting active is given up once ThreadCreated is older than its deadline. + The failure reply keeps the coordinates, so later activity on that thread still finds the exchange, which is terminal and does nothing. + */ + it.effect("expires a turn that never settles and ignores its later activity", () => + withProcessor( + { + t3Gateway: { + getTurnStatus: () => Effect.succeed({ turn: "active" }), + }, + }, + ({ processor, repository, calls, pingActivity, awaitStoredTag }) => + Effect.gen(function* () { + yield* repository.upsert(threadCreated); + + yield* TestClock.adjust("61 minutes"); + const expiredAt = yield* Clock.currentTimeMillis; + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const posted = yield* awaitStoredTag(request.sourceUri, "reply-posted"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "T3Gateway.getTurnStatus", + "NTBSAdapter.findPostedReply", + "NTBSAdapter.postReply", + ]); + const replyPending = toExpired(threadCreated, expiredAt); + expect(replyPending.reply).toEqual({ + type: "failure", + text: "T3 did not answer in time.", + cause: { + type: "expired", + state: { tag: "thread-created", t3: defaultWorkCoordinates }, + }, + }); + expect(posted).toEqual(toReplyPosted(replyPending, postedReplyUri, expiredAt)); + + yield* pingActivity(defaultThreadId); + yield* TestClock.adjust("1 minute"); + expect(calls.length).toBe(3); + + yield* Fiber.interrupt(run); + }), + ), + ); + + /* + A reply the platform never accepts is given up once ReplyPending is older than its deadline: the exchange becomes Undeliverable without another posting attempt. + */ + it.effect("gives up on a reply the platform never accepted", () => + withProcessor( + { + adapter: { + postReply: () => + new AdapterError({ reason: "Reply posting failed", cause: "test failure" }), + }, + }, + ({ processor, repository, calls, awaitStoredTag }) => + Effect.gen(function* () { + const replyPending = toReplyPending(threadCreated, answer("Never delivered"), now); + yield* repository.upsert(replyPending); + + yield* TestClock.adjust("61 minutes"); + const expiredAt = yield* Clock.currentTimeMillis; + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const undeliverable = yield* awaitStoredTag(request.sourceUri, "undeliverable"); + + expect(calls.map((call) => `${call.service}.${call.method}`)).toEqual([ + "NTBSAdapter.findPostedReply", + ]); + expect(undeliverable).toEqual( + toUndeliverable( + replyPending, + { message: "The platform did not accept the reply in time." }, + expiredAt, + ), + ); + + yield* Fiber.interrupt(run); + }), + ), + ); +}); diff --git a/apps/server/src/ntbs/processor.ts b/apps/server/src/ntbs/processor.ts new file mode 100644 index 000000000000..326f9be8d627 --- /dev/null +++ b/apps/server/src/ntbs/processor.ts @@ -0,0 +1,461 @@ +import { type ThreadId } from "@t3tools/contracts"; +import * as NTBS from "./exchange.ts"; +import { Clock, Context, Data, Effect, Semaphore, Stream } from "effect"; +import { NTBSAdapter } from "./adapter.ts"; +import { T3Gateway } from "./t3gateway.ts"; +import { ExchangeRepository } from "./ExchangeRepository.ts"; + +/* +The processor is the executor and orchestrator of non-turn-based surfaces: it applies the business rules and connects T3 to the external platform. It does so through three services: + +- the adapter: communication with the external platform +- the T3 gateway: communication and dispatching of T3 internals +- the exchange repository: durable link between the two, stores the exchange state + +It exposes two public APIs: +1. `process` takes an incoming message and starts the work for it. +2. `run` subscribes to T3 activity and resumes the exchanges a previous run left unfinished. + +`run` also owns an internal sweeper: a periodic pass that re-drives every non-terminal exchange, the same thing startup recovery does but on an interval. +Thread activity is the primary wake signal, but it is a fire-and-forget ping: without the sweeper one missed event would leave an exchange stuck until the next restart. +Sweeping is cheap and safe because the cycle observes before acting: re-driving an exchange whose context has not moved just answers "wait" and stops. + +Both drive an exchange through the same cycle, repeated until it reaches a terminal state: + +load the stored state +-> read live context from the service that owns it +-> decide what to do given state and context +-> execute the decision +-> build the resulting state transition and persist it + +The cycle is replay safe: it observes before acting, so a crash or a redelivered message re-runs it without starting a second thread or posting a second reply. +*/ + +export class NTBSProcessorError extends Data.TaggedError("NTBSProcessorError")<{ + reason: string; + cause: unknown; +}> {} + +const SWEEP_INTERVAL = "1 minute"; + +export interface NTBSProcessor { + /** + * Handles a request coming from an external platform. + * + * Does no filtering: the caller decides whether a request deserves T3 work, and everything passed here starts it. + * + * Returns once the request is recorded, not once it is answered: the reply is posted later, when T3 reports the turn finished. + * Fails with a typed error only when the repository does. Anything that fails after the record dies; `run` retries the recorded exchange. + * + * Idempotent per `sourceUri`: a redelivery of an already-recorded request is a no-op, whatever state that exchange has reached. Concurrent deliveries of the same request are serialized, so only the first records it. + */ + readonly process: ( + request: NTBS.Request, + t3Target: NTBS.T3Target, + ) => Effect.Effect; + + /** + * The main loop of the processor. + * Subscribes to T3 activity, then resumes every non-terminal exchange. Subscribing first means nothing is missed while recovery runs. After that, an exchange moves when its T3 thread does, with a periodic sweep re-driving every non-terminal exchange as the backstop for missed activity pings. + * + * Never returns and has no error channel: a failure anywhere in it is a defect. + */ + readonly run: Effect.Effect; +} + +export const makeNTBSProcessorTag = (key: string) => Context.Service(key); + +type TransitionResult = + | { + readonly type: "transitioned"; + readonly state: NTBS.Exchange; + } + | { + readonly type: "unchanged"; + }; + +type NTBSProcessorRequirements = + /* + Communicates with the external platform. Which platform is decided by the context the processor is built in. + */ + | NTBSAdapter + /* + Creates worktrees and threads, starts turns, reports their progress, and provides the stream of T3 thread activity. + */ + | T3Gateway + /* + Stores and loads the exchange state, including the exchanges a previous run left unfinished. + */ + | ExchangeRepository; + +type ExchangeLock = { + readonly semaphore: Semaphore.Semaphore; + callers: number; +}; + +/** + * Builds a processor for the adapter found in the context. + * + * Build one per platform, each with its own adapter provided. + * + * TODO: Consider collapsing to a single runtime processor with one routing adapter that reads the platform from the sourceUri scheme (jira://, discord://) and delegates to the platform adapter. + * The current one-per-platform design has an unenforced assumption: `findNonTerminalExchanges` returns every stored exchange with no platform filter, so processors sharing a repository would re-drive each other's exchanges through the wrong adapter during recovery and sweeps. + * A single processor also means one lock map, one activity subscription, one sweeper, and retires `makeNTBSProcessorTag`. + */ +export const makeNTBSProcessor: Effect.Effect = + Effect.gen(function* () { + const adapter = yield* NTBSAdapter; + const t3 = yield* T3Gateway; + const repo = yield* ExchangeRepository; + + const orFail = (reason: string) => + Effect.mapError((cause: unknown) => new NTBSProcessorError({ reason, cause })); + + const transitionedTo = (state: NTBS.Exchange): TransitionResult => ({ + type: "transitioned", + state, + }); + + const unchanged: TransitionResult = { type: "unchanged" }; + + const persist = (state: State) => + repo.upsert(state).pipe(orFail("Failed to persist the exchange state"), Effect.as(state)); + + /** + * Serializes concurrent work on the same sourceUri, protecting the check-then-act record in `process` (findBySourceUri -> persist). + * The lock is in-process memory: single-writer is an assumption on the deployment, not something the code or the database enforces. + * Two processors on the same database would each pass the "no exchange yet" check, both record, and the upsert would silently overwrite the first record instead of failing. + * TODO: If we ever run more than one processor, this needs remote locking or a record that can lose (e.g. a unique insert on sourceUri that rejects the second writer). + */ + const exchangeLocks = new Map(); + + const withExchangeLock = (sourceUri: string, effect: Effect.Effect) => + Effect.suspend(() => { + let lock = exchangeLocks.get(sourceUri); + + if (lock === undefined) { + lock = { + semaphore: Semaphore.makeUnsafe(1), + callers: 0, + }; + exchangeLocks.set(sourceUri, lock); + } + + lock.callers += 1; + + return lock.semaphore.withPermit(effect).pipe( + Effect.ensuring( + Effect.sync(() => { + lock.callers -= 1; + if (lock.callers === 0 && exchangeLocks.get(sourceUri) === lock) { + exchangeLocks.delete(sourceUri); + } + }), + ), + ); + }); + + const processRequestAccepted = Effect.fn("NTBSProcessor.processRequestAccepted")(function* ( + state: NTBS.RequestAccepted, + ) { + const now = yield* Clock.currentTimeMillis; + const decision = NTBS.fromRequestAccepted(state, now); + + switch (decision.type) { + case "expire": { + const next = yield* persist(NTBS.toExpired(state, now)); + return transitionedTo(next); + } + + case "plan": { + // Planning creates nothing in T3, so there is nothing to observe first: plan, then record the outcome. + const planned = yield* t3 + .planCoordinates(state.target.projectId, state.target.startBranchName) + .pipe( + Effect.map((coordinates) => NTBS.toWorkPlanned(state, coordinates, now)), + Effect.catchTag("FatalError", (rejection) => + Effect.succeed(NTBS.toRejected(state, rejection, now)), + ), + orFail("Failed to plan the T3 work"), + ); + const next = yield* persist(planned); + return transitionedTo(next); + } + } + }); + + const processWorkPlanned = Effect.fn("NTBSProcessor.processWorkPlanned")(function* ( + state: NTBS.WorkPlanned, + ) { + const now = yield* Clock.currentTimeMillis; + const context = yield* t3 + .getThreadStatus(state) + .pipe(orFail("Failed to get the T3 thread status")); + const decision = NTBS.fromWorkPlanned(state, context, now); + + switch (decision.type) { + case "expire": { + const next = yield* persist(NTBS.toExpired(state, now)); + return transitionedTo(next); + } + + case "provision-thread": { + // TODO: Quite sure there's low hanging fruits here + const rejection = yield* t3.provisionThread(state).pipe( + Effect.as(null), + Effect.catchTag("FatalError", (error) => Effect.succeed(error)), + orFail("Failed to provision the T3 thread"), + ); + + if (rejection !== null) { + const next = yield* persist(NTBS.toRejected(state, rejection, now)); + return transitionedTo(next); + } + + break; + } + + case "record-thread-created": + break; + } + + const threadCreated = yield* persist(NTBS.toThreadCreated(state, now)); + yield* adapter.acknowledge(threadCreated).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed to post the NTBS acknowledgement", { + sourceUri: threadCreated.sourceUri, + threadId: threadCreated.t3.threadId, + cause, + }), + ), + ); + return transitionedTo(threadCreated); + }); + + const processThreadCreated = Effect.fn("NTBSProcessor.processThreadCreated")(function* ( + state: NTBS.ThreadCreated, + ) { + const now = yield* Clock.currentTimeMillis; + const context = yield* t3 + .getTurnStatus(state) + .pipe(orFail("Failed to get the T3 turn status")); + const decision = NTBS.fromThreadCreated(state, context, now); + + switch (decision.type) { + case "expire": { + const next = yield* persist(NTBS.toExpired(state, now)); + return transitionedTo(next); + } + + case "start-turn": { + const rejection = yield* t3.startTurn(state).pipe( + Effect.as(null), + Effect.catchTag("FatalError", (error) => Effect.succeed(error)), + orFail("Failed to start the T3 turn"), + ); + + if (rejection !== null) { + const replyPending = yield* persist(NTBS.toRejected(state, rejection, now)); + return transitionedTo(replyPending); + } + + return unchanged; + } + + case "wait": + return unchanged; + + case "record-reply-pending": { + const next = yield* persist(NTBS.toReplyPending(state, decision.reply, now)); + return transitionedTo(next); + } + } + }); + + const processReplyPending = Effect.fn("NTBSProcessor.processReplyPending")(function* ( + state: NTBS.ReplyPending, + ) { + const now = yield* Clock.currentTimeMillis; + const replySourceUri = yield* adapter + .findPostedReply(state) + .pipe(orFail("Failed to find the posted platform reply")); + const context: NTBS.ReplyPendingContext = + replySourceUri === null + ? { platformReply: "missing" } + : { platformReply: "posted", replySourceUri }; + const decision = NTBS.fromReplyPending(state, context, now); + + switch (decision.type) { + case "expire": { + const next = yield* persist( + NTBS.toUndeliverable( + state, + { message: "The platform did not accept the reply in time." }, + now, + ), + ); + return transitionedTo(next); + } + + case "post-reply": { + const delivery = yield* adapter.postReply(state).pipe( + Effect.map((postedReplySourceUri) => ({ + type: "posted" as const, + replySourceUri: postedReplySourceUri, + })), + Effect.catchTag("ReplyRejected", (error) => + Effect.succeed({ type: "rejected" as const, cause: error.cause }), + ), + orFail("Failed to post the platform reply"), + ); + + const next = yield* persist( + delivery.type === "posted" + ? NTBS.toReplyPosted(state, delivery.replySourceUri, now) + : NTBS.toUndeliverable(state, delivery.cause, now), + ); + return transitionedTo(next); + } + + case "record-reply-posted": { + const next = yield* persist(NTBS.toReplyPosted(state, decision.replySourceUri, now)); + return transitionedTo(next); + } + } + }); + + const advanceExchange = Effect.fn("NTBSProcessor.advanceExchange")(function* ( + initial: NTBS.Exchange, + ) { + let state = initial; + + while (NTBS.isNonTerminal(state)) { + let result: TransitionResult; + + switch (state.tag) { + case "request-accepted": + result = yield* processRequestAccepted(state); + break; + + case "work-planned": + result = yield* processWorkPlanned(state); + break; + + case "thread-created": + result = yield* processThreadCreated(state); + break; + + case "reply-pending": + result = yield* processReplyPending(state); + break; + } + + if (result.type === "unchanged") { + return; + } + + state = result.state; + } + }); + + const advanceSavedExchange = Effect.fn("NTBSProcessor.advanceSavedExchange")(function* ( + sourceUri: string, + ) { + return yield* withExchangeLock( + sourceUri, + Effect.gen(function* () { + const exchange = yield* repo + .findBySourceUri(sourceUri) + .pipe(orFail("Failed to reload the exchange")); + + if (exchange === null || NTBS.isTerminal(exchange)) { + return; + } + + yield* advanceExchange(exchange); + }), + ); + }); + + const process = Effect.fn("NTBSProcessor.process")(function* ( + request: NTBS.Request, + t3Target: NTBS.T3Target, + ) { + return yield* withExchangeLock( + request.sourceUri, + Effect.gen(function* () { + /* + 1. Check whether an Exchange exists for this source URI. + 2. If there is already - we can return. We treat duplicate deliveries of requests with the same sourceUri as duplicates. No ops. + 3. If there isn't we record the request as accepted and advance the exchange. + From the record on, a failure is the exchange's to keep, not the caller's: it is left for `run` to retry. + */ + + const existing = yield* repo + .findBySourceUri(request.sourceUri) + .pipe(orFail("Failed to find the exchange for the platform request")); + + if (existing !== null) { + return; + } + + const now = yield* Clock.currentTimeMillis; + const accepted = yield* persist(NTBS.makeRequestAccepted(request, t3Target, now)); + yield* advanceExchange(accepted).pipe(Effect.orDie); + }), + ); + }); + + const processThreadActivity = Effect.fn("NTBSProcessor.processThreadActivity")(function* ( + threadId: ThreadId, + ) { + const exchange = yield* repo + .findByThreadId(threadId) + .pipe(orFail("Failed to find the exchange for the active T3 thread")); + + if (exchange !== null) { + yield* advanceSavedExchange(exchange.sourceUri); + } + }); + + const subscribeToThreadActivity = Stream.runForEach(t3.threadActivity, (threadId) => + processThreadActivity(threadId).pipe(Effect.orDie), + ); + + const resumeNonTerminalExchanges = repo.findNonTerminalExchanges.pipe( + orFail("Failed to load non-terminal exchanges"), + Effect.flatMap((exchanges) => + Effect.forEach( + exchanges, + (exchange) => advanceSavedExchange(exchange.sourceUri).pipe(Effect.orDie), + { discard: true }, + ), + ), + Effect.orDie, + ); + + /* + The sweeper: the same pass as startup recovery, repeated on an interval for the whole life of `run`. + Thread activity is the primary wake signal but it is fire-and-forget: a ping missed while the process is up would otherwise strand its exchange until the next restart. + Redundant sweeps are safe and cheap because the cycle observes before acting: an exchange whose context has not moved answers "wait" and stops. + The interval is a judgment call, low enough that a stranded exchange recovers within a tolerable wait for whoever asked, high enough that the periodic query stays negligible. + Delay first: `run` has just swept via startup recovery, so an immediate first pass would be pure noise. + */ + const sweepNonTerminalExchanges = resumeNonTerminalExchanges.pipe( + Effect.delay(SWEEP_INTERVAL), + Effect.forever, + ); + + const run = Effect.scoped( + Effect.gen(function* () { + yield* subscribeToThreadActivity.pipe(Effect.forkScoped({ startImmediately: true })); + yield* resumeNonTerminalExchanges; + return yield* sweepNonTerminalExchanges; + }), + ); + + return { + process, + run, + }; + }); diff --git a/apps/server/src/ntbs/t3gateway.test.ts b/apps/server/src/ntbs/t3gateway.test.ts new file mode 100644 index 000000000000..361555025ae0 --- /dev/null +++ b/apps/server/src/ntbs/t3gateway.test.ts @@ -0,0 +1,2091 @@ +import { describe, it, expect } from "@effect/vitest"; +import { t3GatewayLive, T3Gateway } from "./t3gateway.ts"; +import { DateTime, Effect, Layer, Option, Ref, FileSystem, Stream } from "effect"; +import { OrchestrationEngineService } from "../orchestration/Services/OrchestrationEngine.ts"; +import { ProjectionSnapshotQuery } from "../orchestration/Services/ProjectionSnapshotQuery.ts"; +import { + ProjectionTurnRepository, + type ProjectionTurn, +} from "../persistence/Services/ProjectionTurns.ts"; +import { GitWorkflowService } from "../git/GitWorkflowService.ts"; +import { + ProjectSetupScriptOperationError, + ProjectSetupScriptRunner, +} from "../project/ProjectSetupScriptRunner.ts"; +import { Crypto } from "effect/Crypto"; +import { + EventId, + GitCommandError, + MessageId, + type OrchestrationEvent, + OrchestrationProjectShell, + type OrchestrationSessionStatus, + OrchestrationThread, + OrchestrationThreadShell, + ProjectId, + ProviderInstanceId, + ThreadId, + TurnId, + VcsCreateWorktreeResult, + VcsListRefsResult, + VcsStatusLocalResult, +} from "@t3tools/contracts"; +import { buildTemporaryWorktreeBranchName } from "@t3tools/shared/git"; +import { toPersistenceSqlError } from "../persistence/Errors.ts"; +import { OrchestrationCommandInvariantError } from "../orchestration/Errors.ts"; +import { PlatformError, SystemError } from "effect/PlatformError"; +import { + makeRequestAccepted, + toThreadCreated, + toWorkPlanned, + type Request, + type T3Target, + type WorkCoordinates, +} from "./exchange.ts"; +import { ServerConfig } from "../config.ts"; +import { LogLevel } from "effect/Config"; + +/** + * Every mocked dependency records into one shared, ordered log. + * + * One array rather than one per service: only a single sequence can answer questions that span + * services — that the project is loaded before git runs, or that a rejection stopped the gateway + * before it minted anything. Per-service views are derivable from this; the ordering is not + * recoverable from them. + */ +type Call = { service: string; method: string; input: unknown }; + +const createCallLog = () => { + const calls: Array = []; + + const recordResult = + (service: string) => + (method: string, input: unknown, value: A) => + Effect.sync(() => { + calls.push({ service, method, input }); + return value; + }); + + const record = (service: string) => (method: string, input: unknown) => + Effect.sync(() => calls.push({ service, method, input })); + + // Widened so the mock wrappers stay the only writers: tests can read the log, not push into it. + return { calls: calls as ReadonlyArray, recordResult, record }; +}; + +type CallRecordResult = ReturnType["recordResult"]; + +type CallRecord = ReturnType["record"]; + +type CryptoInput = { + failRandomUUIDv4?: boolean; +}; + +const createCryptoMock = (recordResult: CallRecordResult, input?: CryptoInput) => { + const recordCrypto = recordResult("Crypto"); + + return Layer.unwrap( + Effect.gen(function* () { + const counter: Ref.Ref = yield* Ref.make(0); + + return Layer.mock(Crypto, { + "~effect/platform/Crypto": "~effect/platform/Crypto", + randomUUIDv4: + // recordCrypto("randomUUIDv4", undefined, ) + + input?.failRandomUUIDv4 + ? recordCrypto("randomUUIDv4", undefined, "noreach").pipe( + Effect.andThen( + new PlatformError( + new SystemError({ + _tag: "Unknown", + method: "randomUUIDv4", + module: "crypto something", + }), + ), + ), + ) + : Ref.getAndUpdate(counter, (num) => num + 1).pipe( + Effect.flatMap((num) => + recordCrypto("randomUUIDv4", undefined, "randomUUID" + num), + ), + ), + nextDoubleUnsafe: () => 0, + nextIntUnsafe: () => 0, + }); + }), + ); +}; + +const createGitCommandError = (exitCode?: number, detail = "") => + GitCommandError.make({ + command: "resolve", + cwd: "", + detail, + failureKind: "unknown", + operation: "", + exitCode, + }); + +type OrchestrationEngineInput = { + /** `"invariant"` fails with the decider's rejection; `true` with an operational persistence error. */ + dispatchFails?: boolean | "invariant"; + /** Emitted through `streamDomainEvents` as a finite stream, unlike the live infinite PubSub one. */ + domainEvents?: ReadonlyArray; +}; + +// TODO: We need to make sure and analyze what happens when it is dispatched +// Old processor treated it as a synchronous event, but that might be a lie +const createOrchestrationEngineServiceMock = ( + recordResult: CallRecordResult, + callRecord: CallRecord, + input?: OrchestrationEngineInput, +) => { + const _record = recordResult("OrchestrationEngineService"); + + const call = callRecord("OrchestrationEngineService"); + + return Layer.mock(OrchestrationEngineService, { + dispatch: (_command) => + call("dispatch", _command).pipe( + Effect.andThen(() => + input?.dispatchFails + ? Effect.fail( + input.dispatchFails === "invariant" + ? new OrchestrationCommandInvariantError({ + commandType: _command.type, + detail: "rejected by the decider", + }) + : toPersistenceSqlError("some operation")("some cause"), + ) + : Effect.succeed({ sequence: 0 }), + ), + ), + streamDomainEvents: Stream.fromIterable(input?.domainEvents ?? []), + }); +}; + +type PSQMInput = { + getProjectShellById?: + | { + success: Partial; + } + | { failure: unknown } + | { missing: true }; + isThreadMissing?: boolean; + isGetThreadShellByIdError?: boolean; + isThreadDetailMissing?: boolean; + isGetThreadDetailByIdError?: boolean; + /** Rendered as messages on the thread detail; assistant unless a role is given. */ + threadMessages?: ReadonlyArray<{ id: MessageId; text: string; role?: "user" | "assistant" }>; + sessionStatus?: OrchestrationSessionStatus; + sessionLastError?: string; +}; + +const createPSQM = (record: CallRecordResult, input?: PSQMInput) => { + const recordPSQM = record("ProjectionSnapshotQuery"); + + const isProjectMissing = input?.getProjectShellById && "missing" in input.getProjectShellById; + + const isGetProjectError = input?.getProjectShellById && "failure" in input.getProjectShellById; + + return Layer.mock(ProjectionSnapshotQuery, { + getProjectShellById: (projectId) => + recordPSQM("getProjectShellById", projectId, null).pipe( + Effect.andThen( + isGetProjectError + ? toPersistenceSqlError("some sql error")("somecause") + : Effect.option( + isProjectMissing + ? Effect.fail("missing") + : Effect.succeed({ + id: projectId, + workspaceRoot: "root", + title: "project-title", + createdAt: DateTime.formatIso(DateTime.nowUnsafe()), + updatedAt: DateTime.formatIso(DateTime.nowUnsafe()), + defaultModelSelection: null, + scripts: [], + ...(input?.getProjectShellById && + "success" in input.getProjectShellById && { + ...input.getProjectShellById.success, + }), + }), + ), + ), + ), + getThreadShellById: (threadId) => + recordPSQM( + "getThreadShellById", + threadId, + input?.isThreadMissing + ? Option.none() + : Option.some({ + archivedAt: null, + branch: "some-branch", + createdAt: DateTime.formatIso(DateTime.nowUnsafe()), + hasActionableProposedPlan: false, + hasPendingApprovals: false, + hasPendingUserInput: false, + id: threadId, + interactionMode: "default", + latestTurn: null, + latestUserMessageAt: null, + modelSelection: { + instanceId: ProviderInstanceId.make("instanceId"), + model: "custom", + options: [], + }, + projectId: ProjectId.make("projectId"), + runtimeMode: "auto", + session: { + threadId, + activeTurnId: null, + lastError: null, + providerName: null, + runtimeMode: "auto", + status: "ready", + updatedAt: DateTime.formatIso(DateTime.nowUnsafe()), + providerInstanceId: ProviderInstanceId.make("providerInstanceId"), + }, + settledAt: null, + settledOverride: "active", + title: "some title", + updatedAt: DateTime.formatIso(DateTime.nowUnsafe()), + worktreePath: null, + }), + ).pipe( + Effect.filterOrFail( + () => !input || input.isGetThreadShellByIdError !== true, + () => toPersistenceSqlError("some sql error")("somecause"), + ), + ), + getThreadDetailById: (threadId) => + recordPSQM( + "getThreadDetailById", + threadId, + input?.isThreadDetailMissing + ? Option.none() + : Option.some({ + id: threadId, + projectId: ProjectId.make("projectId"), + title: "some title", + modelSelection: { + instanceId: ProviderInstanceId.make("instanceId"), + model: "custom", + options: [], + }, + runtimeMode: "auto", + interactionMode: "default", + branch: "some-branch", + worktreePath: null, + latestTurn: null, + createdAt: DateTime.formatIso(DateTime.nowUnsafe()), + updatedAt: DateTime.formatIso(DateTime.nowUnsafe()), + archivedAt: null, + settledOverride: null, + settledAt: null, + deletedAt: null, + messages: (input?.threadMessages ?? []).map((message) => ({ + id: message.id, + role: message.role ?? ("assistant" as const), + text: message.text, + turnId: null, + streaming: false, + createdAt: DateTime.formatIso(DateTime.nowUnsafe()), + updatedAt: DateTime.formatIso(DateTime.nowUnsafe()), + })), + queuedMessages: [], + pendingTurnStart: null, + proposedPlans: [], + activities: [], + checkpoints: [], + session: { + threadId, + status: input?.sessionStatus ?? "ready", + providerName: null, + activeTurnId: null, + lastError: input?.sessionLastError ?? null, + runtimeMode: "auto", + updatedAt: DateTime.formatIso(DateTime.nowUnsafe()), + providerInstanceId: ProviderInstanceId.make("providerInstanceId"), + }, + }), + ).pipe( + Effect.filterOrFail( + () => !input || input.isGetThreadDetailByIdError !== true, + () => toPersistenceSqlError("some sql error")("somecause"), + ), + ), + }); +}; + +type ProjectionTurnRepositoryInput = { + turns?: ReadonlyArray; + listByThreadIdFails?: boolean; +}; + +const createProjectionTurnRepositoryMock = ( + recordResult: CallRecordResult, + input?: ProjectionTurnRepositoryInput, +) => { + const record = recordResult("ProjectionTurnRepository"); + + return Layer.mock(ProjectionTurnRepository, { + listByThreadId: (callInput) => + record("listByThreadId", callInput, input?.turns ?? []).pipe( + Effect.filterOrFail( + () => !input || input.listByThreadIdFails !== true, + () => toPersistenceSqlError("some sql error")("somecause"), + ), + ), + }); +}; + +type GitLayerInput = { + createWorkreeFails?: boolean | { detail: string }; + failsBranchResolutionWith?: "retryable" | "fatal"; + fetchRemoteFails?: boolean; + remoteExists?: boolean; + remoteExistsFails?: boolean; + resolvedRemoteSha?: string; + removeWorkTreeFails?: boolean; + worktreeBranchExists?: boolean; + localStatus?: { isRepo?: boolean; refName?: string }; +}; + +// TODO: Can't we simplify it by leveraging default values in params? +// we can pass default arguments in JS +const createGitWorkflowServiceMock = ( + recordResult: CallRecordResult, + callRecord: CallRecord, + input?: GitLayerInput, +) => { + const recordGit = recordResult("GitWorkflowService"); + + const record = callRecord("GitWorkflowService"); + + return Layer.mock(GitWorkflowService, { + createWorktree: (callInput) => + record("createWorktree", callInput).pipe( + Effect.andThen(() => + input?.createWorkreeFails + ? Effect.fail( + createGitCommandError( + undefined, + typeof input.createWorkreeFails === "object" + ? input.createWorkreeFails.detail + : "", + ), + ) + : Effect.succeed( + VcsCreateWorktreeResult.make({ + worktree: { path: callInput.path || "path", refName: callInput.refName }, + }), + ), + ), + ), + remoteExists: (callInput) => + recordGit("remoteExists", callInput, !input || input.remoteExists !== false).pipe( + Effect.filterOrFail( + () => !input || !input.remoteExistsFails, + () => createGitCommandError(), + ), + ), + fetchRemote: (callInput) => + recordGit("fetchRemote", callInput, undefined).pipe( + Effect.filterOrFail( + () => !input || input.fetchRemoteFails !== true, + () => createGitCommandError(), + ), + ), + localStatus: (callInput) => + recordGit( + "localStatus", + callInput, + VcsStatusLocalResult.make({ + isRepo: input?.localStatus?.isRepo ?? false, + hasPrimaryRemote: true, + isDefaultRef: false, + refName: input?.localStatus?.refName ?? null, + hasWorkingTreeChanges: false, + workingTree: { files: [], insertions: 0, deletions: 0 }, + }), + ), + listRefs: (callInput) => + recordGit( + "listRefs", + callInput, + VcsListRefsResult.make({ + refs: input?.worktreeBranchExists + ? [ + { + name: callInput.query ?? "worktreeBranchName", + current: false, + isDefault: false, + worktreePath: null, + }, + ] + : [], + isRepo: true, + hasPrimaryRemote: true, + nextCursor: null, + totalCount: input?.worktreeBranchExists ? 1 : 0, + }), + ), + removeWorktree: (callInput) => + record("removeWorktree", callInput).pipe( + Effect.andThen(() => + input?.removeWorkTreeFails ? Effect.fail(createGitCommandError()) : Effect.void, + ), + ), + + resolveRemoteTrackingCommit: (callInput) => + recordGit("resolveRemoteTrackingCommit", callInput, { + commitSha: input?.resolvedRemoteSha ?? "sha123", + remoteRefName: "remoteRefName", + }).pipe( + Effect.flatMap((val) => + input && input.failsBranchResolutionWith + ? createGitCommandError(input.failsBranchResolutionWith === "fatal" ? 1 : undefined) + : Effect.succeed(val), + ), + ), + }); +}; + +type ProjectSetupScriptRunnerInput = { + runForThreadFails?: boolean; +}; + +const ProjectSetupScriptRunnerMock = ( + _recordResult: CallRecordResult, + callRecord: CallRecord, + input?: ProjectSetupScriptRunnerInput, +) => { + // const record = recordResult("ProjestSetupScriptRunnerMock"); + const call = callRecord("ProjectSetupScriptRunnerMock"); + + return Layer.mock(ProjectSetupScriptRunner, { + runForThread: (callInput) => + call("runForThread", input).pipe( + Effect.andThen(() => + input?.runForThreadFails + ? Effect.fail( + ProjectSetupScriptOperationError.make({ + _tag: "ProjectSetupScriptOperationError", + cause: "somecause", + operation: "openTerminal", + threadId: callInput.threadId, + worktreePath: callInput.worktreePath, + }), + ) + : Effect.succeed({ status: "no-script" }), + ), + ), + }); +}; + +const serverConfigMock = Layer.mock(ServerConfig, { + anonymousIdPath: "anonymousIdPath", + attachmentsDir: "attachmentsDir", + autoBootstrapProjectFromCwd: false, + baseDir: "baseDir", + cwd: "cwd", + dbPath: "dbPath", + desktopBootstrapToken: "desktopBootstrapToken", + devAllowedOrigins: [], + devUrl: undefined, + environmentIdPath: "environmentIdPath", + host: "host", + keybindingsConfigPath: "keybindingsConfigPath", + logLevel: LogLevel.make("All"), + logWebSocketEvents: false, + logsDir: "logsDir", + mode: "desktop", + noBrowser: false, + otlpExportIntervalMs: 0, + otlpMetricsUrl: undefined, + otlpServiceName: "otlpServiceName", + otlpTracesUrl: "otlpTracesUrl", + port: 8000, + providerEventLogPath: "providerEventLogPath", + providerLogsDir: "providerLogsDir", + providerStatusCacheDir: "providerStatusCacheDir", + secretsDir: "secretsDir", + serverLogPath: "serverLogPath", + serverRuntimeStatePath: "serverRuntimeStatePath", + serverTracePath: "serverTracePath", + settingsPath: "settingsPath", + startupPresentation: "headless", + stateDir: "stateDir", + staticDir: "staticDir", + tailscaleServeEnabled: false, + tailscaleServePort: 8001, + terminalLogsDir: "terminalLogsDir", + traceBatchWindowMs: 0, + traceMaxBytes: 1024, + traceMaxFiles: 80, + traceMinLevel: LogLevel.make("All"), + traceTimingEnabled: false, + worktreesDir: "/worktreesDir", + desktopTelemetryControlFd: 8002, + desktopTelemetryFd: 8003, + resourceMonitorPath: "resourceMonitorPath", +}); + +type FileSystemInput = { + worktreePathExists?: boolean; + existsFails?: boolean; + removeFails?: boolean; +}; + +const createFileSystemMock = (recordResult: CallRecordResult, input?: FileSystemInput) => { + const recordFs = recordResult("FileSystem"); + + const fail = (method: string) => + new PlatformError(new SystemError({ _tag: "Unknown", method, module: "FileSystem" })); + + return FileSystem.layerNoop({ + exists: (path) => + recordFs("exists", path, input?.worktreePathExists === true).pipe( + Effect.filterOrFail( + () => !input || input.existsFails !== true, + () => fail("exists"), + ), + ), + remove: (path, options) => + recordFs("remove", { path, options }, undefined).pipe( + Effect.filterOrFail( + () => !input || input.removeFails !== true, + () => fail("remove"), + ), + ), + }); +}; + +/** + * Builds a gateway plus the log of everything its dependencies were asked to do. + * + * Called per test rather than per block: each failure mode needs its own mock configuration, so + * there is nothing worth sharing, and a log created per test beats resetting a shared one. + */ +const createT3Gateway = (input?: { + pqsm?: PSQMInput; + gwfs?: GitLayerInput; + crypto?: CryptoInput; + orchestrationEngine?: OrchestrationEngineInput; + projectSetupScriptRunner?: ProjectSetupScriptRunnerInput; + fileSystem?: FileSystemInput; + turnRepository?: ProjectionTurnRepositoryInput; +}) => { + const { calls, recordResult, record } = createCallLog(); + + return { + calls, + layer: t3GatewayLive.pipe( + Layer.provide( + Layer.mergeAll( + createOrchestrationEngineServiceMock(recordResult, record, input?.orchestrationEngine), + createPSQM(recordResult, input?.pqsm), + ProjectSetupScriptRunnerMock(recordResult, record, input?.projectSetupScriptRunner), + createGitWorkflowServiceMock(recordResult, record, input?.gwfs), + createProjectionTurnRepositoryMock(recordResult, input?.turnRepository), + createCryptoMock(recordResult, input?.crypto), + serverConfigMock, + createFileSystemMock(recordResult, input?.fileSystem), + ), + ), + ), + }; +}; + +const now = 1_700_000_000_000; + +describe("T3Gateway", () => { + describe("planCoordinates", () => { + /* + Recap. This will, in order: + - fetch the project details for projectId + - if it cannot load the project due to errors, it will fail with a recoverable error + - it if can: + - if the project exists: it will return it + - if it does not: it will fail with a T3Rejected error, one that cannot be retried + - it checks if the git remote exists + - if it cannot load: retryable fail + - if it can but it does not exist: T3Rejected, it cannot be retried + - it tries to fetch it + - this cannot be rejected, it can only return a retryable error. + It would make no sense to error, as remote exists step before confirmed it exists. + - last step: try to get the commit sha for the remote branch with that name + + Now that we have the git and project references: + - generate a threadId + - generate a userMessageId + - generate a branch name for the temporary git worktree + - return the coordinates + + */ + describe("successful planning", () => { + it.effect("pins the selected branch to the commit fetched from origin", () => { + /* + Declared once and threaded through the project mock, so the assertions below prove the cwd git receives is the workspace the project lookup returned, rather than two literals that happen to agree. + */ + const workspaceRoot = "/workspaces/project-under-test"; + const projectId = ProjectId.make("test-1"); + + const { calls, layer } = createT3Gateway({ + pqsm: { getProjectShellById: { success: { workspaceRoot } } }, + gwfs: { remoteExists: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const coordinates = yield* t3Gateway.planCoordinates(projectId, "main"); + + expect(coordinates).toEqual({ + projectId, + startBranchName: "main", + startCommitSha: "sha123", + threadId: expect.any(String), + userMessageId: expect.any(String), + worktreeBranchName: expect.any(String), + }); + + /* + The identifiers are whatever the crypto mock hands out, so asserting exact values + would only restate the mock. What matters is the two relationships the gateway owns: + the thread and its first message are distinct, and the worktree branch is cut from + the thread so a stray branch traces back to it. + */ + expect(coordinates.threadId).not.toEqual(coordinates.userMessageId); + expect(coordinates.worktreeBranchName).toEqual( + buildTemporaryWorktreeBranchName(() => coordinates.threadId), + ); + + /* + Order matters as much as the arguments. The tip is only current because the fetch + precedes it — reading first would resolve whatever origin pointed at the last time + anything fetched in this workspace. And git must run in the workspace the project + lookup returned: the wrong one still resolves a real sha, from the wrong repository. + */ + expect(calls).toEqual([ + { + service: "ProjectionSnapshotQuery", + method: "getProjectShellById", + input: projectId, + }, + { + service: "GitWorkflowService", + method: "remoteExists", + input: { cwd: workspaceRoot, remoteName: "origin" }, + }, + { + service: "GitWorkflowService", + method: "fetchRemote", + input: { cwd: workspaceRoot, remoteName: "origin" }, + }, + { + service: "GitWorkflowService", + method: "resolveRemoteTrackingCommit", + input: { cwd: workspaceRoot, refName: "main", fallbackRemoteName: "origin" }, + }, + { service: "Crypto", method: "randomUUIDv4", input: undefined }, + { service: "Crypto", method: "randomUUIDv4", input: undefined }, + ]); + }).pipe(Effect.provide(layer)); + }); + }); + + describe("fatal errors", () => { + it.effect( + "rejects a project that does not exist without performing provisioning work", + () => { + const projectId = ProjectId.make("non-existing-project"); + + const { calls, layer } = createT3Gateway({ + pqsm: { getProjectShellById: { missing: true } }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const error = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(error._tag).toBe("FatalError"); + + expect(error.method).toBe("projectionSnapshotQuery.getProjectShellById"); + + // Nothing after the lookup: no git, and no identifiers minted for work that cannot run. + expect(calls).toEqual([ + { + service: "ProjectionSnapshotQuery", + method: "getProjectShellById", + input: projectId, + }, + ]); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect("rejects a project whose repository has no origin remote", () => { + const { layer } = createT3Gateway({ + gwfs: { remoteExists: false }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("FatalError"); + expect(result.method).toBe("gitWorkflowService.remoteExists"); + }).pipe(Effect.provide(layer)); + }); + + it.effect( + "rejects a selected branch that is absent after a successful fetch without performing provisioning work", + () => { + const { layer, calls } = createT3Gateway({ + gwfs: { failsBranchResolutionWith: "fatal", remoteExists: true }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("FatalError"); + + expect(result.method).toBe("gitWorkflowService.resolveRemoteTrackingCommit"); + + const methods = calls.map((call) => call.method); + + expect(methods).toEqual([ + "getProjectShellById", + "remoteExists", + "fetchRemote", + "resolveRemoteTrackingCommit", + ]); + }).pipe(Effect.provide(layer)); + }, + ); + }); + + describe("operational failures", () => { + it.effect("fails retryably when the project lookup fails", () => { + const { layer, calls } = createT3Gateway({ + pqsm: { + getProjectShellById: { failure: "no project resolving" }, + }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + + expect(result.method).toBe("projectionSnapshotQuery.getProjectShellById"); + + const methods = calls.map((call) => call.method); + + expect(methods).toEqual(["getProjectShellById"]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails retryably when checking for the origin remote existance fails", () => { + const { layer, calls } = createT3Gateway({ + gwfs: { + remoteExistsFails: true, + }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + + expect(result.method).toBe("gitWorkflowService.remoteExists"); + + const methods = calls.map((call) => call.method); + + expect(methods).toEqual(["getProjectShellById", "remoteExists"]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails retryably when fetching origin fails", () => { + const { layer, calls } = createT3Gateway({ + gwfs: { + fetchRemoteFails: true, + }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + + expect(result.method).toBe("gitWorkflowService.fetchRemote"); + + const methods = calls.map((call) => call.method); + + expect(methods).toEqual(["getProjectShellById", "remoteExists", "fetchRemote"]); + }).pipe(Effect.provide(layer)); + }); + + /* + A git failure carrying no exit code means git never ran to completion (timeout, spawn failure) rather than that the branch is missing. + */ + it.effect("fails retryably when reading the branch tip fails without a git exit code", () => { + const { layer, calls } = createT3Gateway({ + gwfs: { + failsBranchResolutionWith: "retryable", + }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + + expect(result.method).toBe("gitWorkflowService.resolveRemoteTrackingCommit"); + + const methods = calls.map((call) => call.method); + + expect(methods).toEqual([ + "getProjectShellById", + "remoteExists", + "fetchRemote", + "resolveRemoteTrackingCommit", + ]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails retryably when the exchange IDs cannot be minted", () => { + const { layer, calls } = createT3Gateway({ + crypto: { + failRandomUUIDv4: true, + }, + }); + + const projectId = ProjectId.make("projectId"); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.planCoordinates(projectId, "main").pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + + expect(result.method).toBe("crypto.randomUUIDv4"); + + const methods = calls.map((call) => call.method); + + expect(methods).toEqual([ + "getProjectShellById", + "remoteExists", + "fetchRemote", + "resolveRemoteTrackingCommit", + "randomUUIDv4", + ]); + }).pipe(Effect.provide(layer)); + }); + }); + }); + + describe("getThreadStatus", () => { + /** + * What does getThreadStatus does? + * + * It reports whether the T3 thread for the exchange exists ("present") or not ("missing"). + * + * It runs only once in the processor, for Exchanges that are in the + * `RequestClaimed` status, in the `processRequesClaimed` effect. + * + * It returns the `RequestClaimedContext` needed by the decider function `fromRequestClaimed` to calculate the following policy `RequestClaimedDecision`. + * + * The thread can either be "missing" or "present". + * + * It can only fail with a RetryableError. + */ + describe("happy cases", () => { + it.effect("it returns that the missing thread", () => { + const { layer } = createT3Gateway({ + pqsm: { + isThreadMissing: true, + }, + }); + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const projectId = ProjectId.make("happy cases - missing thread"); + + const coordinates = yield* t3Gateway.planCoordinates(projectId, "main"); + + const state = toWorkPlanned( + makeRequestAccepted( + { + attachments: [], + snapshot: "happy cases - missing thread - snapshot", + sourceUri: "test://happy-cases-missing-thread-1", + }, + { projectId, startBranchName: "main" }, + now, + ), + coordinates, + now, + ); + + const result = yield* t3Gateway.getThreadStatus(state); + + expect(result.thread).toBe("missing"); + }).pipe(Effect.provide(layer)); + }); + + it.effect("it returns that the thread is present", () => { + const { layer } = createT3Gateway({ + pqsm: { + isThreadMissing: false, + }, + }); + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const projectId = ProjectId.make("happy cases - missing thread"); + + const coordinates = yield* t3Gateway.planCoordinates(projectId, "main"); + + const state = toWorkPlanned( + makeRequestAccepted( + { + attachments: [], + snapshot: "happy cases - missing thread - snapshot", + sourceUri: "test://happy-cases-missing-thread-1", + }, + { projectId, startBranchName: "main" }, + now, + ), + coordinates, + now, + ); + + const result = yield* t3Gateway.getThreadStatus(state); + + expect(result.thread).toBe("present"); + }).pipe(Effect.provide(layer)); + }); + }); + + describe("operational failures", () => { + // Note: we're really not caring _why_. + // Albeit, as of writing there's only retryable errors? + it.effect("cannot get the thread", () => { + const { layer } = createT3Gateway({ + pqsm: { + isGetThreadShellByIdError: true, + }, + }); + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const projectId = ProjectId.make("happy cases - missing thread"); + + const coordinates = yield* t3Gateway.planCoordinates(projectId, "main"); + + const state = toWorkPlanned( + makeRequestAccepted( + { + attachments: [], + snapshot: "happy cases - missing thread - snapshot", + sourceUri: "test://happy-cases-missing-thread-1", + }, + { projectId, startBranchName: "main" }, + now, + ), + coordinates, + now, + ); + + const result = yield* t3Gateway.getThreadStatus(state).pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + }).pipe(Effect.provide(layer)); + }); + }); + }); + + describe("provisionThread", () => { + /* + Info: used only once in `processor.ts` in `processRequestClaimed` so when the current status of an exchange is `RequesClaimed`. + + When the exchange is in that status, `getThreadStatus`, tested above, provides the actual context of thread. + + We know that the NTBS system has now received the external request, and saved it along the coordinates minted via `planCoordinates`. + + What we *don't* know is whether the actual T3 thread has been started or not. + + Why is that? + + After thread creation, threads are provisioned, what can happen is that T3 starts the thread but it is not recorded in the NTBS exchange (e.g. thread starts -> app crashes -> thread start doesn't get recorded). + + So we must double check starting from a RequestClaimed Exchange that the thread did not indeed start before. + + What does `provisionThread` even do anyway? + + It handles worktree and thread creation as well as executing setup scripts. + + (N.B. In theory we should skip setup scripts if it was already done as well). + + ## How did it work in the old processor? + + 1. create worktree + 2. "thread.create" in orchestrationEngineService.dispatch command + 2.a if anything goes wrong during thread.create -> removes the worktree + 3. run the scripts via projectScriptRunner.runForThread + + Return value: provisionThread returns nothing. TODO: Is there anything important that gets retrieved there (some information)? + + Quite sure the current implementation can be updated and made better than the current void into null and rejection !== null in `processor.ts` as of 6d70ff461df16d1a052ce3656131613647940028. + + What does `provisionThread` depends on? + 1. ServerConfig to know the worktrees location + 2. GitWorkflowService for worktree creation (and deletion) + 3. OrchestrationEngineService for dispatching the command to create the thread + 4. ProjectScriptRunner for executing the scripts in the thread/cwd + */ + const request: Request = { + attachments: [], + snapshot: "come on, do something", + sourceUri: "test://source-uri", + }; + + const coordinates: WorkCoordinates = { + projectId: ProjectId.make("projectId"), + startBranchName: "startBranchName", + startCommitSha: "startCommitSha", + threadId: ThreadId.make("threadId"), + userMessageId: MessageId.make("userMessageId"), + worktreeBranchName: "worktreeBranchName", + }; + + const target: T3Target = { + projectId: coordinates.projectId, + startBranchName: coordinates.startBranchName, + }; + + const workPlanned = toWorkPlanned(makeRequestAccepted(request, target, now), coordinates, now); + + describe("happy case", () => { + /* + Pristine first attempt: `createWorktree` takes the fresh-create arm, dispatch succeeds so the stale-observation recovery never fires, scripts run last. + */ + it.effect("provisions worktree, thread, and scripts in order from a clean slate", () => { + const { calls, layer } = createT3Gateway(); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + yield* t3Gateway.provisionThread(workPlanned); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + "randomUUIDv4", + "dispatch", + "runForThread", + ]); + + // Fresh-create arm, off the commit pinned at claim. + expect(calls.find((call) => call.method === "createWorktree")?.input).toMatchObject({ + refName: coordinates.startCommitSha, + newRefName: coordinates.worktreeBranchName, + baseRefName: coordinates.startBranchName, + }); + }).pipe(Effect.provide(layer)); + }); + + /* + A failed dispatch is not trusted at face value: the "thread is missing" observation that led us here can be stale (crash after a committed create, projection lag), so the gateway re-checks and adopts the existing thread. + Note the scripts still run — skipping them when provisioning already completed is an open TODO. + */ + it.effect("succeeds when dispatch fails because the thread already exists", () => { + const { calls, layer } = createT3Gateway({ + orchestrationEngine: { dispatchFails: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + yield* t3Gateway.provisionThread(workPlanned); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + "randomUUIDv4", + "dispatch", + "getThreadShellById", + "runForThread", + ]); + }).pipe(Effect.provide(layer)); + }); + + /* + Resume: the path already holds a checkout of the minted branch, so git is asked, agrees, and no worktree work happens at all. + */ + it.effect("reuses an intact worktree left by an interrupted attempt", () => { + const { calls, layer } = createT3Gateway({ + fileSystem: { worktreePathExists: true }, + gwfs: { localStatus: { isRepo: true, refName: coordinates.worktreeBranchName } }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + yield* t3Gateway.provisionThread(workPlanned); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "localStatus", + "randomUUIDv4", + "dispatch", + "runForThread", + ]); + }).pipe(Effect.provide(layer)); + }); + + /* + Resume: something occupies the path but git does not recognize it as a checkout of the minted branch, so it is destroyed and provisioning restarts from the pinned commit. + */ + it.effect("destroys debris at the worktree path and creates fresh", () => { + const { calls, layer } = createT3Gateway({ + fileSystem: { worktreePathExists: true }, + gwfs: { localStatus: { isRepo: false } }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + yield* t3Gateway.provisionThread(workPlanned); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "localStatus", + "removeWorktree", + "listRefs", + "createWorktree", + "randomUUIDv4", + "dispatch", + "runForThread", + ]); + + // Fresh-create arm: branch off the pinned commit, not a checkout of a survivor. + expect(calls.find((call) => call.method === "createWorktree")?.input).toMatchObject({ + refName: coordinates.startCommitSha, + newRefName: coordinates.worktreeBranchName, + }); + }).pipe(Effect.provide(layer)); + }); + + /* + Resume: a crashed attempt created the branch but not the checkout, so the branch is checked out instead of re-branching from the start commit. + */ + it.effect( + "checks out a branch surviving from a crashed attempt instead of re-creating it", + () => { + const { calls, layer } = createT3Gateway({ + gwfs: { worktreeBranchExists: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + yield* t3Gateway.provisionThread(workPlanned); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + "randomUUIDv4", + "dispatch", + "runForThread", + ]); + + // Checkout arm: the surviving branch itself, no new branch minted. + const createInput = calls.find((call) => call.method === "createWorktree")?.input; + expect(createInput).toMatchObject({ refName: coordinates.worktreeBranchName }); + expect(createInput).not.toHaveProperty("newRefName"); + }).pipe(Effect.provide(layer)); + }, + ); + }); + + describe("failures", () => { + it.effect("fails retryably when dispatch fails and the thread is genuinely missing", () => { + const { calls, layer } = createT3Gateway({ + orchestrationEngine: { dispatchFails: true }, + pqsm: { isThreadMissing: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.provisionThread(workPlanned).pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + expect(result.method).toBe("orchestrationEngine.dispatch"); + + /* + A retryable failure cleans up nothing: no removeWorktree, no scripts. + */ + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + "randomUUIDv4", + "dispatch", + "getThreadShellById", + ]); + }).pipe(Effect.provide(layer)); + }); + + /* + The one moment ownership truly ends: a fatal error removes the worktree. + */ + it.effect("fails fatally when setup scripts fail, removing the worktree", () => { + const { calls, layer } = createT3Gateway({ + projectSetupScriptRunner: { runForThreadFails: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.provisionThread(workPlanned).pipe(Effect.flip); + + expect(result._tag).toBe("FatalError"); + expect(result.method).toBe("projectScriptRunner.runForThread"); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + "randomUUIDv4", + "dispatch", + "runForThread", + "removeWorktree", + ]); + }).pipe(Effect.provide(layer)); + }); + + /* + A worktree path that is still registered to a deleted checkout needs manual `git worktree prune`, so retrying would fail forever. + */ + it.effect( + "fails fatally when the worktree path is a stale registration, removing the worktree", + () => { + const { calls, layer } = createT3Gateway({ + gwfs: { + createWorkreeFails: { detail: "'/worktreesDir/x' is missing but already registered" }, + }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.provisionThread(workPlanned).pipe(Effect.flip); + + expect(result._tag).toBe("FatalError"); + expect(result.method).toBe("gitWorkflowService.createWorktree"); + + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + "removeWorktree", + ]); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect("fails retryably when worktree creation fails for any other reason", () => { + const { calls, layer } = createT3Gateway({ + gwfs: { createWorkreeFails: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.provisionThread(workPlanned).pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + expect(result.method).toBe("gitWorkflowService.createWorktree"); + + // Retryable, so the failed creation attempt is not cleaned up. + expect(calls.map((call) => call.method)).toEqual([ + "getProjectShellById", + "exists", + "listRefs", + "createWorktree", + ]); + }).pipe(Effect.provide(layer)); + }); + + /* + Cleanup is best-effort: a worktree that also refuses to be removed must not mask the script failure. + */ + it.effect("still reports the script failure when the cleanup itself fails", () => { + const { calls, layer } = createT3Gateway({ + projectSetupScriptRunner: { runForThreadFails: true }, + gwfs: { removeWorkTreeFails: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.provisionThread(workPlanned).pipe(Effect.flip); + + expect(result._tag).toBe("FatalError"); + expect(result.method).toBe("projectScriptRunner.runForThread"); + + expect(calls.map((call) => call.method)).toContain("removeWorktree"); + }).pipe(Effect.provide(layer)); + }); + }); + }); + + describe("getTurnStatus", () => { + /* + What is `getTurnStatus` used for? + + In `processor.ts` it has one single caller, the `processThreadCreated` function. + + By now, we have an Exchange stored as being in the `ThreadCreated` state: + - we have successfully minted a thread id, a user message id and a new branch name + - we have used those to create a new worktree whose path derives from the workspace basename and the branch name, and which is associated to that specific thread and external platform request + - we have stored this information + + And now? + + Operationally only one thing is needed: starting the turn and having the agent do its thing and come up with some response to the original user. + + But what if a turn was started and then some failure/crash caused the turn start not to be recorded by the system? It would make no sense to re-start the turn, or the operation could fail. Thus, the first thing we want to do when processing a `ThreadCreated` exchange is to verify whether it already started a turn and verify its status. + + Note that the answer is not binary: `ThreadCreatedContext` reports the turn as "missing", "active" or "completed" (carrying the reply), and `fromThreadCreated` maps those to `start-turn`, `wait` and `record-reply-pending` respectively. + + And thus, here, we verify the behavior of `getTurnStatus` on T3Gateway service. + */ + + const threadCreated = toThreadCreated( + toWorkPlanned( + makeRequestAccepted( + { + attachments: [], + snapshot: "getTurnStatus - snapshot", + sourceUri: "test://get-turn-status", + }, + { projectId: ProjectId.make("projectId"), startBranchName: "startBranchName" }, + now, + ), + { + projectId: ProjectId.make("projectId"), + startBranchName: "startBranchName", + startCommitSha: "startCommitSha", + threadId: ThreadId.make("threadId"), + userMessageId: MessageId.make("userMessageId"), + worktreeBranchName: "worktreeBranchName", + }, + now, + ), + now, + ); + + /** The coordinates every reply out of our turn carries. */ + const turn = { + threadId: threadCreated.t3.threadId, + userMessageId: threadCreated.t3.userMessageId, + turnId: TurnId.make("turnId"), + }; + + const settled = { type: "settled", ...turn } as const; + + // A failure observed before any turn adopted our message. + const settledWithoutTurn = { type: "settled", ...turn, turnId: null } as const; + + /** + * A projected turn on the exchange's thread. Defaults describe the turn our own message + * started; tests override `pendingMessageId` to plant other messages' turns, and `state` / + * `assistantMessageId` to shape the outcome. + */ + const makeProjectionTurn = (input?: Partial): ProjectionTurn => ({ + threadId: threadCreated.t3.threadId, + turnId: TurnId.make("turnId"), + pendingMessageId: threadCreated.t3.userMessageId, + sourceProposedPlanThreadId: null, + sourceProposedPlanId: null, + assistantMessageId: null, + state: "pending", + requestedAt: DateTime.formatIso(DateTime.nowUnsafe()), + startedAt: null, + completedAt: null, + checkpointTurnCount: null, + checkpointRef: null, + checkpointStatus: null, + checkpointFiles: [], + ...input, + }); + + it.effect("answers { turn: 'missing' } when the thread has no turns at all", () => { + const { calls, layer } = createT3Gateway({ turnRepository: { turns: [] } }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ turn: "missing" }); + + // Both lookups are scoped to the exchange's own thread. + expect(calls).toEqual([ + { + service: "ProjectionTurnRepository", + method: "listByThreadId", + input: { threadId: threadCreated.t3.threadId }, + }, + { + service: "ProjectionSnapshotQuery", + method: "getThreadDetailById", + input: threadCreated.t3.threadId, + }, + ]); + }).pipe(Effect.provide(layer)); + }); + /* + Turns started by other messages (another platform, the web UI) are not ours: "does this + thread have a turn?" is the wrong question, "did our message start one?" is the right one. + */ + it.effect( + "answers { turn: 'missing' } when the thread has turns but none whose pendingMessageId matches the exchange's userMessageId", + () => { + const { layer } = createT3Gateway({ + turnRepository: { + turns: [ + makeProjectionTurn({ pendingMessageId: MessageId.make("someone-else") }), + makeProjectionTurn({ pendingMessageId: null, state: "completed" }), + ], + }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ turn: "missing" }); + }).pipe(Effect.provide(layer)); + }, + ); + it.effect("answers { turn: 'active' } when the matching turn is pending", () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "pending" })] }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ turn: "active" }); + }).pipe(Effect.provide(layer)); + }); + + it.effect("answers { turn: 'active' } when the matching turn is running", () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "running" })] }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ turn: "active" }); + }).pipe(Effect.provide(layer)); + }); + it.effect( + "answers a completed turn with an answer reply carrying the assistant message text verbatim", + () => { + const assistantMessageId = MessageId.make("assistantMessageId"); + + const { layer } = createT3Gateway({ + turnRepository: { + turns: [makeProjectionTurn({ state: "completed", assistantMessageId })], + }, + pqsm: { + threadMessages: [{ id: assistantMessageId, text: "the agent's answer" }], + }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { type: "answer", text: "the agent's answer", ...turn }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect( + "picks our turn's reply when the thread holds several turns from other messages alongside ours", + () => { + const assistantMessageId = MessageId.make("ourAssistantMessageId"); + const foreignAssistantMessageId = MessageId.make("foreignAssistantMessageId"); + + const { layer } = createT3Gateway({ + turnRepository: { + turns: [ + // A completed foreign turn before ours: picking "the first completed turn" would grab this one. + makeProjectionTurn({ + pendingMessageId: MessageId.make("someone-else"), + state: "completed", + assistantMessageId: foreignAssistantMessageId, + }), + makeProjectionTurn({ state: "completed", assistantMessageId }), + makeProjectionTurn({ pendingMessageId: null, state: "running" }), + ], + }, + pqsm: { + threadMessages: [ + { id: foreignAssistantMessageId, text: "someone else's answer" }, + { id: assistantMessageId, text: "our answer" }, + ], + }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { type: "answer", text: "our answer", ...turn }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + it.effect( + "answers a completed turn with a failure reply when the turn has no assistantMessageId", + () => { + const { layer } = createT3Gateway({ + turnRepository: { + turns: [makeProjectionTurn({ state: "completed", assistantMessageId: null })], + }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { + type: "failure", + text: "T3 completed without producing a response.", + cause: settled, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect( + "answers a completed turn with a failure reply when the assistant message is missing or has empty text", + () => { + const assistantMessageId = MessageId.make("assistantMessageId"); + + const turns = [makeProjectionTurn({ state: "completed", assistantMessageId })]; + + // The turn names an assistant message the thread does not contain. + const missingMessage = createT3Gateway({ + turnRepository: { turns }, + pqsm: { threadMessages: [] }, + }); + + // The message exists but holds nothing worth posting. + const emptyText = createT3Gateway({ + turnRepository: { turns }, + pqsm: { threadMessages: [{ id: assistantMessageId, text: " \n " }] }, + }); + + const expected = { + turn: "completed", + reply: { + type: "failure", + text: "T3 completed without producing a response.", + cause: settled, + }, + }; + + const getTurnStatus = Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + return yield* t3Gateway.getTurnStatus(threadCreated); + }); + + return Effect.gen(function* () { + expect(yield* getTurnStatus.pipe(Effect.provide(missingMessage.layer))).toEqual(expected); + expect(yield* getTurnStatus.pipe(Effect.provide(emptyText.layer))).toEqual(expected); + }); + }, + ); + it.effect( + "answers a completed turn with a failure reply carrying session.lastError when the turn errored", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "error" })] }, + pqsm: { sessionLastError: "provider exploded" }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { type: "failure", text: "provider exploded", cause: settled }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect( + "answers a completed turn with a generic failure reply when the turn errored without a recorded lastError", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "error" })] }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { + type: "failure", + text: "T3 failed while processing this request.", + cause: settled, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + it.effect( + "answers a completed turn with a cancellation reply when the turn was interrupted", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "interrupted" })] }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { + type: "cancellation", + text: "T3 stopped processing this request.", + ...turn, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + /* + An observed fact, not a lookup error: retrying cannot bring the thread back, so the exchange must progress to a failure reply instead of staying open forever. + */ + it.effect( + "answers a completed turn with a failure reply when the turn settled but the thread is gone", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "completed" })] }, + pqsm: { isThreadDetailMissing: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { + type: "failure", + text: "T3 finished, but its thread could no longer be found.", + cause: settled, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + /* + When the provider fails to start a turn, T3 keeps two facts: + 1. our message on the thread is stored + 2. the session is marked as errored + + The current test verifies that if conditions 1 and 2 are met, but there is no turn retrieved from T3, then we're in an error state. + */ + it.effect( + "answers a completed turn with a failure reply when no turn adopted our message and the session errored", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [] }, + pqsm: { + threadMessages: [ + { id: threadCreated.t3.userMessageId, text: "snapshot", role: "user" }, + ], + sessionStatus: "error", + sessionLastError: "codex: command not found", + }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { + type: "failure", + text: "codex: command not found", + cause: settledWithoutTurn, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + /* + The previous case checked whether we had both a session error and a message recorded. If we did, we concluded that there was an error provider-side. + + Here, we test the same situation, but without the user message recorded. As T3 saves the message _before_ starting the turn and the turn is not here, we never dispatched the turn start, so it is safe to do it now. + */ + it.effect( + "answers { turn: 'missing' } when the session errored but our message never reached the thread", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [] }, + pqsm: { sessionStatus: "error", sessionLastError: "codex: command not found" }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ turn: "missing" }); + }).pipe(Effect.provide(layer)); + }, + ); + + /* + In ThreadCreated the thread existed. No turn and no thread means it was deleted since, and + no retry can bring it back. + */ + it.effect( + "answers a failure reply when no turn adopted our message and the thread is gone", + () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [] }, + pqsm: { isThreadDetailMissing: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated); + + expect(result).toEqual({ + turn: "completed", + reply: { + type: "failure", + text: "T3's thread could no longer be found.", + cause: settledWithoutTurn, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect("fails with RetryableError when listing the thread's turns fails", () => { + const { layer } = createT3Gateway({ turnRepository: { listByThreadIdFails: true } }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated).pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + expect(result.method).toBe("projectionTurnRepository.listByThreadId"); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails with RetryableError when the thread detail fetch fails transiently", () => { + const { layer } = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "completed" })] }, + pqsm: { isGetThreadDetailByIdError: true }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.getTurnStatus(threadCreated).pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + expect(result.method).toBe("projectionSnapshotQuery.getThreadDetailById"); + }).pipe(Effect.provide(layer)); + }); + + /* + The detail fetch exists only to read a settled reply; running it earlier would add a failure mode to arms that need nothing from it. + */ + it.effect("performs no thread detail lookup when the turn is active", () => { + const active = createT3Gateway({ + turnRepository: { turns: [makeProjectionTurn({ state: "running" })] }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + yield* t3Gateway.getTurnStatus(threadCreated); + + expect(active.calls.map((call) => call.method)).toEqual(["listByThreadId"]); + }).pipe(Effect.provide(active.layer)); + }); + }); + + describe("startTurn", () => { + /* + What is `startTurn` used for? + + It has a single call site, `processThreadCreated`. From a business-logic perspective the following has happened: + 1. A user on an external platform has sent a message that has been processed and recorded. + 2. A threadId, userMessageId and branch names have been minted for this incoming message. + 3. A `RequestClaimed` is saved to the exchange repository. We now have a durable record of the incoming request and the t3 coordinates of the work associated to it. + 4. A git worktree is created, the branch is checked out, the scripts of the project have been run. a `ThreadCreated` is recorded to the `Exchange` repository. + 5. Now that we have a thread, we want to start the turn, but only if there is no turn already for this very thread and userMessageId. `getTurnStatus` does exactly that: given the `ThreadCreated` it retrieves the related information to determine whether a previous run may have already started a turn that has never been recorded, or never completed. + 6. We now know that we're safe to start the turn. + + So, what do we do in `startTurn`? + + Essentially one thing: dispatch `thread.turn.start` command to the orchestration engine, sending the captured request snapshot as the first user message, under the minted userMessageId, with the stored attachments. That `userMessageId` becomes the turn's `pendingMessageId`, which is exactly the identity `getTurnStatus` matches on. + + Note what `startTurn` does not: + - It does **not** start a turn in the "physical" sense of waiting for a synchronous confirmation that the turn has effectively started in some harness. It merely returns once the command is dispatched and durably accepted; the provider adopting the turn and running it happens asynchronously after. Even a start failure after this point does not surface at `startTurn` level. It will be catched after, during a `getTurnStatus` run. + - Needless to say, it doesn't wait for a turn completion either, as it doesn't even wait for it to start. + + Why is "dispatched and durably accepted" enough? Because the engine appends the events and writes the projected turn rows in one SQL transaction, and only then returns. + `getTurnStatus` reads the very rows that transaction writes, so a crash anywhere leaves both the events and the turn row, or neither: there is no window where a turn started but cannot be seen by the next cycle. + This matters because a duplicate dispatch would not fail: the decider queues it while our turn is pending or active, or starts a second turn if it already completed. + So the protection against double starts is never sending one, not recovering from a rejection. + + A successful `startTurn` transitions nothing: the processor returns "unchanged" and the exchange stays `ThreadCreated` until `getTurnStatus` observes a settled turn. + + TODO: Consider listening to t3 events to confirm the turn has started maybe? + + As `startTurn` is an action both error classifications apply. We may get both Retryable as well as Fatal errors. + */ + + const threadCreated = toThreadCreated( + toWorkPlanned( + makeRequestAccepted( + { + attachments: [], + snapshot: "please fix the flaky login test", + sourceUri: "test://start-turn", + }, + { projectId: ProjectId.make("projectId"), startBranchName: "startBranchName" }, + now, + ), + { + projectId: ProjectId.make("projectId"), + startBranchName: "startBranchName", + startCommitSha: "startCommitSha", + threadId: ThreadId.make("threadId"), + userMessageId: MessageId.make("userMessageId"), + worktreeBranchName: "worktreeBranchName", + }, + now, + ), + now, + ); + + /* + Only the identity relationships are asserted, not the full command payload: the userMessageId linkage is what makes the turn findable by `getTurnStatus`, and minting a fresh id here instead would deadlock every exchange without any error surfacing. + */ + it.effect( + "dispatches the turn start for our thread carrying the snapshot under the exchange's userMessageId", + () => { + const { calls, layer } = createT3Gateway(); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + yield* t3Gateway.startTurn(threadCreated); + + const dispatch = calls.find((call) => call.method === "dispatch"); + expect(dispatch?.input).toMatchObject({ + threadId: threadCreated.t3.threadId, + message: { + messageId: threadCreated.t3.userMessageId, + text: threadCreated.snapshot, + attachments: threadCreated.attachments, + }, + }); + }).pipe(Effect.provide(layer)); + }, + ); + + it.effect("fails with FatalError when the decider rejects the turn start", () => { + const { layer } = createT3Gateway({ orchestrationEngine: { dispatchFails: "invariant" } }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.startTurn(threadCreated).pipe(Effect.flip); + + expect(result._tag).toBe("FatalError"); + expect(result.method).toBe("orchestrationEngine.dispatch"); + }).pipe(Effect.provide(layer)); + }); + + it.effect("fails with RetryableError when the dispatch fails operationally", () => { + const { layer } = createT3Gateway({ orchestrationEngine: { dispatchFails: true } }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const result = yield* t3Gateway.startTurn(threadCreated).pipe(Effect.flip); + + expect(result._tag).toBe("RetryableError"); + expect(result.method).toBe("orchestrationEngine.dispatch"); + }).pipe(Effect.provide(layer)); + }); + }); + + describe("threadActivity", () => { + /* + `threadActivity` represents the last and final piece of the `t3Gateway`. + + It is the only `T3Gateway` service not returning an effect. Instead it exposes a `Stream` to which the processor has to subscribe. + + And the NTBS processor does exactly that. When the `run` Effect of the NTBS processor is yielded it subscribes to thread activity. + + `subscribeToThreadActivity` does `Stream.runForEach(t3.threadActivity, (threadId) => ...does something with the thread id). + + The important takeaway seems to be the fact that it merely seems to signal that something has happened/changed for some `threadId`. It is then up to the processor to lookup whether that threadId is of interest to the processor or not. + + What does `threadActivity` does in practice then and how does it work? Apparently, it should just subscribe to the main t3 event emitter, filter events for those that related to thread changes and merely stream the threadId of those events. There is no "who's listening" gap here: a `Stream` is a lazy description, and nobody subscribes until someone runs it. `OrchestrationEngineService` exposes `streamDomainEvents`, a `Stream.fromPubSub` that creates a fresh subscription each time it is run, so `threadActivity` is just that stream piped through a filter mapping events to their threadId. The subscription to the engine's PubSub is established exactly when the processor's Stream.runForEach starts, with no separate "start listening" step for `T3Gateway` to perform. + + The filter should be generous: emit the threadId of any event whose payload carries one, rather than betting on which specific event types signal a turn settling. The trade off is redundant pings, and we accept it because a ping is answered by observe-before-act, so a redundant ping costs one listByThreadId, while a missed ping strands an exchange until restart. + + A ping never arrives "too early". When the engine dispatches a command, everything happens inside one SQL transaction: events appended, projection rows written, receipt stored. Only after that transaction commits does the engine publish the event to the PubSub feeding this stream. So by the time the processor receives a ping for a thread, the database already contains whatever that event changed: when the ping wakes the processor and it calls getTurnStatus, the turn row it queries is guaranteed to reflect the event that caused the ping. There is no window where we get pinged "turn completed", read the projection, and still see the turn as running. Without this ordering we would need retry-until-visible logic; with it, ping then read is safe as-is. + + But a ping can fail to arrive at all. The PubSub only delivers to subscribers that exist at publish time. If an event fires while nobody is subscribed — the classic case being server startup, before the processor has forked its Stream.runForEach — that ping is simply gone. Nothing replays it. The design accepts that because missed pings are covered elsewhere: startup recovery re-drives every non-terminal exchange when the processor boots, and the planned sweeper periodically re-drives non-terminal exchanges. The ping is an optimization for latency — react immediately instead of waiting for the next sweep — not the mechanism correctness depends on. Pings we do get are always safe to act on immediately; pings we don't get are someone else's job to compensate for. + */ + + const threadId = ThreadId.make("activity-thread"); + const projectId = ProjectId.make("activity-project"); + + const baseEventFields = { + sequence: 0, + occurredAt: "2026-01-01T00:00:00.000Z", + commandId: null, + causationEventId: null, + correlationId: null, + metadata: {}, + } as const; + + const threadEvent: OrchestrationEvent = { + ...baseEventFields, + type: "thread.session-set", + eventId: EventId.make("thread-event"), + aggregateKind: "thread", + aggregateId: threadId, + payload: { + threadId, + session: { + threadId, + status: "running", + providerName: null, + runtimeMode: "full-access", + activeTurnId: null, + lastError: null, + updatedAt: "2026-01-01T00:00:00.000Z", + }, + }, + }; + + const projectEvent: OrchestrationEvent = { + ...baseEventFields, + type: "project.deleted", + eventId: EventId.make("project-event"), + aggregateKind: "project", + aggregateId: projectId, + payload: { projectId, deletedAt: "2026-01-01T00:00:00.000Z" }, + }; + + it.effect("emits the threadId of thread events and drops project events", () => { + const { layer } = createT3Gateway({ + orchestrationEngine: { domainEvents: [projectEvent, threadEvent] }, + }); + + return Effect.gen(function* () { + const t3Gateway = yield* T3Gateway; + + const emitted = yield* Stream.runCollect(t3Gateway.threadActivity); + + expect(emitted).toEqual([threadId]); + }).pipe(Effect.provide(layer)); + }); + }); +}); diff --git a/apps/server/src/ntbs/t3gateway.ts b/apps/server/src/ntbs/t3gateway.ts new file mode 100644 index 000000000000..9aa59da9af78 --- /dev/null +++ b/apps/server/src/ntbs/t3gateway.ts @@ -0,0 +1,845 @@ +/* +The T3 gateway module exposes the interface that the NTBS processor uses to communicate +with T3, similar to how adapter models the interaction with the external platform. + */ + +import * as NodePath from "node:path"; +import { + CommandId, + DEFAULT_PROVIDER_INTERACTION_MODE, + MessageId, + OrchestrationCommand, + type ProjectId, + ThreadId, +} from "@t3tools/contracts"; +import type * as NTBS from "./exchange.ts"; +import { + Context, + Crypto, + Data, + DateTime, + Effect, + FileSystem, + Layer, + Option, + Result, + Stream, +} from "effect"; +import { OrchestrationEngineService } from "../orchestration/Services/OrchestrationEngine.ts"; +import { ProjectionSnapshotQuery } from "../orchestration/Services/ProjectionSnapshotQuery.ts"; +import { ProjectionTurnRepository } from "../persistence/Services/ProjectionTurns.ts"; +import { GitWorkflowService } from "../git/GitWorkflowService.ts"; +import { ProjectSetupScriptRunner } from "../project/ProjectSetupScriptRunner.ts"; +import { buildTemporaryWorktreeBranchName } from "@t3tools/shared/git"; +import { DEFAULT_THREAD_TITLE } from "@t3tools/shared/threadTitle"; +import { ServerConfig } from "../config.ts"; +import { getAutoBootstrapDefaultModelSelection } from "../serverRuntimeStartup.ts"; + +/* + NTBS architecture: + + 1. Adapter + Responsible for the communication with the external platform (Jira, Discord, Teams, etc). + - `acknowledge` confirms T3 is processing the user request + - `postReply` sends the reply to the platform + - `findPostedReplies` retries the replies sent to the platform (but maybe not recorded due to crash) + + 2. ExchangeRepository + Responsible for saving `Exchange` data, entities that model the incoming message -> reply cycle and the relations to T3 data (threads, messages, turns). + + 3. T3 gateway + Models the interaction with T3's own api and VCS lifecycle: creating threads, worktrees, starting turns, etc. + + 4. NTBS Processor + The orchestrator between 1, 2, 3 and 4. +*/ + +/** + * A classification, not a scheduling request: it states the step is safe to retry, not that anything will retry it. + * A failed retryable step leaves the exchange state untouched, so the exchange simply remains non-terminal, and whatever re-drives non-terminal exchanges re-runs the cycle from persisted state. + */ +export class RetryableError extends Data.TaggedError("RetryableError")<{ + reason: string; + cause: unknown; + method: string; +}> {} + +/** + * T3 will never accept this work. + * + * The mirror classification: retrying is pointless, so the exchange must progress to a terminal state instead of staying open. + * + * Actions (`planCoordinates`, `provisionThread`, `startTurn`) fail with `FatalError` when T3 rejects the work, and the processor converts that into a reply-pending failure. + * Reads (`getThreadStatus`, `getTurnStatus`) never carry `FatalError`: an unrecoverable fact they observe goes into the context, and the decider drives the same terminal transition. + * Both roads end at the same place: a failure reply to the user. + */ +export class FatalError extends Data.TaggedError("FatalError")<{ + reason: string; + cause: unknown; + method: string; +}> {} + +type T3GatewayRequirements = + /* + Dispatches thread creation and turn-start commands. + Provides the T3 event stream used to detect outcomes. + */ + | OrchestrationEngineService + /* + Loads the selected T3 project and reads thread outcomes. + */ + | ProjectionSnapshotQuery + /* + Finds the exact projected turn associated with the original T3 user message. + */ + | ProjectionTurnRepository + /* + Creates the isolated branch and worktree for each external request. + */ + | GitWorkflowService + /* + Runs the project setup scripts in the newly created worktree before agent work begins. + */ + | ProjectSetupScriptRunner + /* + Generates unique identifiers for the new thread, message, commands, and worktree branch. + */ + | Crypto.Crypto + /* + Needed to know where the worktrees directory is at. + */ + | ServerConfig + /* + Probes and clears leftover worktree directories during reentrant provisioning. + */ + | FileSystem.FileSystem; + +/** A branch on `origin` and the commit it pointed at when it was resolved. */ +interface RemoteBranchTip { + readonly branchName: string; + readonly commitSha: string; +} + +/* + Git refuses `worktree add` at a path whose directory is gone but is still listed + in `.git/worktrees`. Healing needs `git worktree prune`, which the git driver + does not expose yet, so this state is terminal for the exchange. +*/ +const isStaleWorktreeRegistration = (cause: { readonly detail: string }): boolean => + /missing but (?:already registered|locked)/i.test(cause.detail); + +/** Derives the worktree checkout path. Copied from GitVcsDriverCore.createWorktree. */ +const deriveWorktreePath = (input: { + readonly worktreesDir: string; + readonly workspaceRoot: string; + readonly worktreeBranchName: string; +}): string => + NodePath.join( + input.worktreesDir, + NodePath.basename(input.workspaceRoot), + input.worktreeBranchName.replace(/\//g, "-"), + ); + +export interface T3Gateway { + /** + * Pins the requested branch to its current commit on `origin` and mints the thread, message, and + * worktree branch identifiers recorded at claim. + * + * Creates nothing: no thread, no worktree, no turn. Every call mints fresh identifiers, so call it + * once per request and persist the result — a second call orphans the work the first one planned. + */ + readonly planCoordinates: ( + projectId: ProjectId, + startBranchName: string, + ) => Effect.Effect; + + /** + * A missing thread is a normal answer here, it is what triggers provisioning; only at ThreadCreated does the same observation become an anomaly. + */ + readonly getThreadStatus: ( + state: NTBS.WorkPlanned, + ) => Effect.Effect; + + /** Reentrant: worktree, thread creation and setup scripts, each skipped if already done. */ + readonly provisionThread: ( + state: NTBS.WorkPlanned, + ) => Effect.Effect; + + /** Reports turn progress, interpreting a finished turn into a `Reply`: the agent's verbatim response when it produced one, a synthesized failure or cancellation note otherwise. + * + * The question it answers isn't really "does this thread have a turn?" but "did **our** message start a turn?". This is an important distinction because user messages to the same thread in T3 can come from different sources and interfaces. We want to know about the turn that should start stemming from a user in the external platform with a specific userMessageId. + * + * This is the invariant that makes recovery safe. + * + * It's a pure read, errors are Retryable only. Every lookup failure means "ask again later", nothing it learns can reject the exchange. + * + * Unrecoverable edge cases like "turn completed but thread not found" map to "completed" turns whose reply is a failure. + * + */ + readonly getTurnStatus: ( + state: NTBS.ThreadCreated, + ) => Effect.Effect; + + /** + * Safety here is borrowed from the orchestration engine, not proven locally: dispatch returns only after the events and the projected turn rows commit in one SQL transaction, so a successful dispatch is immediately visible to `getTurnStatus` and a crash leaves both or neither. + * That property is what makes observe-before-act sufficient against double starts, because a duplicate `thread.turn.start` would not fail — the decider queues it or starts a second turn. + * If the engine ever projected asynchronously, this module would silently start duplicate turns and no test in this package would notice; the engine's own atomicity tests (OrchestrationEngine.test.ts, "rolls back all events for a multi-event command when projection fails mid-dispatch") are what pin it. + */ + readonly startTurn: ( + state: NTBS.ThreadCreated, + ) => Effect.Effect; + + /** Threads whose T3 state just changed; the processor reconciles each. */ + readonly threadActivity: Stream.Stream; +} + +export const T3Gateway = Context.Service("t3code/ntbs/t3Gateway"); + +const T3GatewayLive: Effect.Effect = Effect.gen( + function* () { + const orFail = + (severity: S) => + (method: string, reason: string) => + Effect.mapError( + (cause: unknown) => + (severity === "fatal" + ? new FatalError({ reason, cause, method }) + : new RetryableError({ reason, cause, method })) as S extends "fatal" + ? FatalError + : RetryableError, + ); + + const projectionSnapshotQuery = yield* ProjectionSnapshotQuery; + + const serverConfig = yield* ServerConfig; + + const orchestrationEngine = yield* OrchestrationEngineService; + + /* + The lookup failing is operational; the project being absent is not. + A deleted or archived project will never come back, so retrying is pointless. + */ + const getProject = (projectId: ProjectId) => + projectionSnapshotQuery.getProjectShellById(projectId).pipe( + orFail("retryable")( + "projectionSnapshotQuery.getProjectShellById", + "Could not load project " + projectId, + ), + Effect.flatMap( + Option.match({ + onSome: Effect.succeed, + onNone: () => + Effect.fail( + new FatalError({ + method: "projectionSnapshotQuery.getProjectShellById", + reason: "Project " + projectId + " does not exist", + cause: null, + }), + ), + }), + ), + ); + + const gitWorkflowService = yield* GitWorkflowService; + + /** + * Resolves `branchName` to the commit it currently points at on `origin`. + * + * Fetches first, so the answer reflects the current remote tip even when the local copy is behind. Only `origin` is consulted: local state is never a fallback, because two requests naming the same branch must start from the same commit. + * + * Rejects when the project has no `origin`, or when `branchName` is not a branch on it. + */ + const resolveRemoteBranchTip = ( + cwd: string, + startBranchName: string, + ): Effect.Effect => + Effect.gen(function* () { + // Check if origin exist. If not, T3 will never be able to accept this work. + // The lookup failing is operational; a definitive `false` is not. + yield* gitWorkflowService.remoteExists({ cwd, remoteName: "origin" }).pipe( + orFail("retryable")( + "gitWorkflowService.remoteExists", + "Could not check whether the remote 'origin' exists", + ), + Effect.filterOrFail( + (exists) => exists, + () => + new FatalError({ + method: "gitWorkflowService.remoteExists", + reason: "Remote 'origin' does not exist", + cause: null, + }), + ), + ); + + // Since it exists, let's fetch the latest remote state + yield* gitWorkflowService + .fetchRemote({ cwd, remoteName: "origin" }) + .pipe( + orFail("retryable")( + "gitWorkflowService.fetchRemote", + "Could not fetch origin. try again", + ), + ); + + /* + Reads the local `refs/remotes/origin/*` namespace the fetch above just refreshed; + no network is involved. + + A missing branch is permanent, but git can also fail here for reasons that are not: + the command timing out behind the git process semaphore, or the process failing to + spawn. Only a non-zero exit means git ran and rejected the ref, so only that becomes + a rejection. Everything else is retried, because a wrong rejection is unrecoverable + while a wrong retry is merely noisy. + */ + return yield* gitWorkflowService + .resolveRemoteTrackingCommit({ + cwd, + refName: startBranchName, + fallbackRemoteName: "origin", + }) + .pipe( + Effect.map((resolved) => ({ + branchName: startBranchName, + commitSha: resolved.commitSha, + })), + Effect.mapError((cause) => + cause.exitCode === undefined + ? new RetryableError({ + method: "gitWorkflowService.resolveRemoteTrackingCommit", + reason: "Could not read the tip of '" + startBranchName + "' on origin", + cause, + }) + : new FatalError({ + method: "gitWorkflowService.resolveRemoteTrackingCommit", + reason: "Branch '" + startBranchName + "' does not exist on origin", + cause, + }), + ), + ); + }); + + const fileSystem = yield* FileSystem.FileSystem; + + /** + * Guarantees `worktreePath` is a registered checkout of `worktreeBranchName`, + * adopting whatever an interrupted attempt left behind: an intact checkout is + * reused, debris at the path is destroyed, a surviving branch is checked out + * instead of re-created, and only then is anything created fresh from the + * commit pinned at claim. + */ + const ensureWorktree = (input: { + readonly workspaceRoot: string; + readonly worktreePath: string; + readonly worktreeBranchName: string; + readonly startCommitSha: string; + readonly startBranchName: string; + }): Effect.Effect => + Effect.gen(function* () { + const pathExists = yield* fileSystem + .exists(input.worktreePath) + .pipe(orFail("retryable")("fileSystem.exists", "Could not inspect the worktree path")); + + if (pathExists) { + // Ask git, not the filesystem: the step is only done when the + // directory is a checkout of the minted branch. + const status = yield* gitWorkflowService + .localStatus({ cwd: input.worktreePath }) + .pipe( + orFail("retryable")( + "gitWorkflowService.localStatus", + "Could not inspect the existing worktree", + ), + ); + + if (status.isRepo && status.refName === input.worktreeBranchName) { + return; + } + + /* + The path is namespaced by this exchange's minted branch, so whatever + else sits here is our own debris (partial checkout, junk). Plain + directory removal is the fallback for content git does not recognize + as a worktree. + */ + yield* gitWorkflowService + .removeWorktree({ cwd: input.workspaceRoot, path: input.worktreePath, force: true }) + .pipe( + Effect.catch(() => fileSystem.remove(input.worktreePath, { recursive: true })), + orFail("retryable")( + "gitWorkflowService.removeWorktree", + "Could not clear the leftover worktree path", + ), + ); + } + + const branchExists = yield* gitWorkflowService + .listRefs({ + cwd: input.workspaceRoot, + query: input.worktreeBranchName, + refKind: "local", + }) + .pipe( + Effect.map((result) => + result.refs.some((ref) => ref.name === input.worktreeBranchName), + ), + orFail("retryable")( + "gitWorkflowService.listRefs", + "Could not check whether the worktree branch already exists", + ), + ); + + yield* gitWorkflowService + .createWorktree( + branchExists + ? { + // A previous attempt created the branch; check it out instead of re-branching. + cwd: input.workspaceRoot, + path: input.worktreePath, + refName: input.worktreeBranchName, + deferDependencyInstall: true, + } + : { + // First real attempt: branch off the commit pinned at claim. + cwd: input.workspaceRoot, + path: input.worktreePath, + refName: input.startCommitSha, + newRefName: input.worktreeBranchName, + baseRefName: input.startBranchName, + deferDependencyInstall: true, + }, + ) + .pipe( + Effect.mapError((cause) => + isStaleWorktreeRegistration(cause) + ? new FatalError({ + method: "gitWorkflowService.createWorktree", + reason: + "The worktree path is still registered to a deleted checkout and needs `git worktree prune`", + cause, + }) + : new RetryableError({ + method: "gitWorkflowService.createWorktree", + reason: "Could not create the worktree at " + input.worktreePath, + cause, + }), + ), + ); + }); + + const crypto = yield* Crypto.Crypto; + const randomUUID = crypto.randomUUIDv4.pipe( + orFail("retryable")("crypto.randomUUIDv4", "Failed creating a UUID v4"), + ); + + const getNow = DateTime.now.pipe(Effect.map(DateTime.formatIso)); + + const projectScriptRunner = yield* ProjectSetupScriptRunner; + + const projectionTurnRepository = yield* ProjectionTurnRepository; + + const planCoordinates = ( + projectId: ProjectId, + startBranchName: string, + ): Effect.Effect => + Effect.gen(function* () { + /* + 1. Resolve the target project + 2. Resolve the branch - commit pair against which we will create our work tree. + 3. Mind thread, branch, message IDs + */ + + const project = yield* getProject(projectId); + + const remoteBranchTip = yield* resolveRemoteBranchTip( + project.workspaceRoot, + startBranchName, + ); + + const threadUUID = yield* randomUUID; + const threadId = ThreadId.make(threadUUID); + + const userMessageId = MessageId.make(yield* randomUUID); + + // Derived from the thread UUID so a stray branch points back at its thread. + const worktreeBranchName = buildTemporaryWorktreeBranchName(() => threadUUID); + + const coordinates: NTBS.WorkCoordinates = { + projectId, + startBranchName: remoteBranchTip.branchName, + startCommitSha: remoteBranchTip.commitSha, + threadId, + userMessageId, + worktreeBranchName, + }; + return coordinates; + }); + + /* TODO: We doing a lot of work behind the scenes just to know whether the thread exists or is missing, this screams sql query or something not a snapshot query + */ + const getThreadStatus = ( + state: NTBS.WorkPlanned, + ): Effect.Effect => + projectionSnapshotQuery.getThreadShellById(state.t3.threadId).pipe( + Effect.map((maybeThread) => ({ + thread: Option.isNone(maybeThread) ? ("missing" as const) : ("present" as const), + })), + orFail("retryable")( + "projectionSnapshotQuery.getThreadShellById", + "Could not check whether thread " + state.t3.threadId + " exists", + ), + ); + + const getTurnStatus = ( + state: NTBS.ThreadCreated, + ): Effect.Effect => + Effect.gen(function* () { + const turns = yield* projectionTurnRepository + .listByThreadId({ threadId: state.t3.threadId }) + .pipe( + orFail("retryable")( + "projectionTurnRepository.listByThreadId", + "Could not load the turns of thread " + state.t3.threadId, + ), + ); + + /* + `.find` is safe: a userMessageId labels at most one turn. It is minted once per request, + and a turn start is only repeated by recovery when no turn exists for it yet. + */ + const turn = turns.find((turn) => turn.pendingMessageId === state.t3.userMessageId); + + // The thread detail holds both the thread's messages and its session. + const loadThreadDetail = projectionSnapshotQuery + .getThreadDetailById(state.t3.threadId) + .pipe( + orFail("retryable")( + "projectionSnapshotQuery.getThreadDetailById", + "Could not load thread " + state.t3.threadId, + ), + ); + + if (turn === undefined) { + /* + No turn for our message. Two cases: + 1. we never dispatched the turn start -> missing, start it + 2. the provider failed before starting -> T3 keeps our message and marks the session + as errored, but never links a turn to our message -> failure reply + + Note: Loading the whole thread detail is expensive. That is fine for the one-minute sweep, but this arm also runs on every activity ping between our turn-start and T3 marking the session as running, and any streaming thread in the system pings during that window. TODO: We should review and find cheaper strategies. + */ + // No turn ever adopted our message, so a failure here refers to the thread and message only. + const settledWithoutTurn: NTBS.FailureCause = { + type: "settled", + threadId: state.t3.threadId, + userMessageId: state.t3.userMessageId, + turnId: null, + }; + + const maybeThread = yield* loadThreadDetail; + if (Option.isNone(maybeThread)) { + // The thread existed when we reached ThreadCreated, so it was deleted since. + // Retrying cannot bring it back: report the failure. + return { + turn: "completed", + reply: { + type: "failure", + text: "T3's thread could no longer be found.", + cause: settledWithoutTurn, + }, + } as const; + } + + const thread = maybeThread.value; + const hasOurMessage = thread.messages.some( + (message) => message.role === "user" && message.id === state.t3.userMessageId, + ); + + if (hasOurMessage && thread.session?.status === "error") { + return { + turn: "completed", + reply: { + type: "failure", + text: thread.session.lastError ?? "T3 failed while processing this request.", + cause: settledWithoutTurn, + }, + } as const; + } + + return { turn: "missing" } as const; + } + + // A row without a turn id is the pending-start placeholder; T3 assigns one when the provider adopts the turn. + if (turn.turnId === null || turn.state === "pending" || turn.state === "running") { + return { turn: "active" } as const; + } + + // Every reply from here on came out of this turn. + const coordinates: NTBS.TurnCoordinates = { + threadId: state.t3.threadId, + userMessageId: state.t3.userMessageId, + turnId: turn.turnId, + }; + + // Only a settled turn needs the thread detail: the reply text lives in its messages. + const maybeThread = yield* loadThreadDetail; + + if (Option.isNone(maybeThread)) { + // The turn settled but its thread is gone. An observed fact, not a lookup error: retrying cannot bring the thread back, so it becomes a failure reply. + return { + turn: "completed", + reply: { + type: "failure", + text: "T3 finished, but its thread could no longer be found.", + cause: { type: "settled", ...coordinates }, + }, + } as const; + } + + const thread = maybeThread.value; + + switch (turn.state) { + case "completed": { + const assistantMessage = + turn.assistantMessageId === null + ? undefined + : thread.messages.find((message) => message.id === turn.assistantMessageId); + const text = assistantMessage?.text.trim() ?? ""; + + return { + turn: "completed", + reply: + text.length > 0 + ? ({ type: "answer", text, ...coordinates } as const) + : ({ + type: "failure", + text: "T3 completed without producing a response.", + cause: { type: "settled", ...coordinates }, + } as const), + } as const; + } + + case "error": + return { + turn: "completed", + reply: { + type: "failure", + text: thread.session?.lastError ?? "T3 failed while processing this request.", + cause: { type: "settled", ...coordinates }, + }, + } as const; + + case "interrupted": + return { + turn: "completed", + reply: { + type: "cancellation", + text: "T3 stopped processing this request.", + ...coordinates, + }, + } as const; + } + }); + + const startTurn = ( + state: NTBS.ThreadCreated, + ): Effect.Effect => + Effect.gen(function* () { + const commandId = CommandId.make(yield* randomUUID); + const createdAt = yield* getNow; + + yield* orchestrationEngine + .dispatch( + OrchestrationCommand.make({ + type: "thread.turn.start", + commandId, + threadId: state.t3.threadId, + message: { + messageId: state.t3.userMessageId, + role: "user", + text: state.snapshot, + attachments: state.attachments, + }, + runtimeMode: "full-access", + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + createdAt, + }), + ) + .pipe( + /* + Verdict vs accident. An invariant error is the decider rejecting the command against current state (thread deleted, queue full): deterministic, retrying re-asks a question already answered, so it is fatal and becomes a failure reply. + Everything else is infrastructure failing before any verdict; the transaction rolled back, nothing committed, and asking again later is meaningful. + Judgment call: "queue full" is an invariant that time can heal (the queue drains when the active turn completes), but a full queue on an NTBS-owned thread means something else is hammering it, and a visible failure beats silently retrying into it. + */ + Effect.mapError((cause) => + cause._tag === "OrchestrationCommandInvariantError" + ? new FatalError({ + method: "orchestrationEngine.dispatch", + reason: "T3 rejected the turn start for thread " + state.t3.threadId, + cause, + }) + : new RetryableError({ + method: "orchestrationEngine.dispatch", + reason: "Could not dispatch thread.turn.start for thread " + state.t3.threadId, + cause, + }), + ), + ); + }); + + /* + This pings on every thread event — token deltas, message streams, all of it, for all threads in the system, not just NTBS ones. + The processor's lookup makes that correct but it's a per-event exchange-repository hit while any turn is streaming. + If that ever shows up in profiles, the fix is a narrower filter (turn/session lifecycle event types) — the generous filter is the right starting point, not necessarily the endpoint. + */ + const threadActivity = orchestrationEngine.streamDomainEvents.pipe( + Stream.filterMap((event) => + event.aggregateKind === "thread" + ? Result.succeed(event.aggregateId as ThreadId) // safe to cast as filtered the aggregate + : Result.failVoid, + ), + ); + + /** + * Creates the actual thread in T3 with the recorded claimed request. + * + */ + const provisionThread = ( + state: NTBS.WorkPlanned, + ): Effect.Effect => + Effect.gen(function* () { + /* + * In order we need to: + * 1. get the actual project details, where is the workspace root path located at? + * 2. get the worktrees path location. T3 does not create worktrees inside the project workspace, because a checkout nested inside the user's project repository directory would pollute it (untracked noise in `git status`, IDE/watcher/grep pickup, accidental commits) and worktrees are T3-owned disposable state, so they live inside T3's home where they can be wiped without touching the user's code (which may even be a bare repo with no working tree to nest into at all). + * 3. Derive the worktree path: where are we going to put the files we're going to work with? + * 4. Create the actual worktree + * 5. Dispatch T3 thread creation + * 6. Run the scripts for that project + */ + // We refetch because the project details we had from `planCoordinates` might have changed, the project might've been deleted, etc + + /* + `provisionThread` is a resumable checklist, not a transaction. + + Every attempt re-derives its *facts* from live state, the project's `workspaceRoot`, and the worktree path computed from the pinned branch name, then walks three steps, each one "check, then do", so a retry after any interruption skips whatever already happened. + + **Worktree**. If the directory exists, reuse it. If only the branch survives from a crashed attempt, recreate the checkout from that branch instead of re-branching from the start commit. Otherwise create it fresh from the pinned commit. + + **Thread**. Create it. If the dispatch fails but the thread turns out to exist, a stale observation raced us and the step is already done. + + **Setup scripts**. Failures are fatal, like any other provisioning error: we don't pretend the workspace works when it doesn't. + + On a retryable failure, cleanup nothing. The half-finished work is owned by the exchange record and is exactly what the next reconcile pass resumes from. + + On a fatal failure, the one moment ownership truly ends, remove the worktree best-effort, and let only the cheap branch ref leak. + */ + const project = yield* getProject(state.t3.projectId); + const { workspaceRoot } = project; + const { worktreesDir } = serverConfig; + + const worktreePath = deriveWorktreePath({ + workspaceRoot, + worktreesDir, + worktreeBranchName: state.t3.worktreeBranchName, + }); + + yield* Effect.gen(function* () { + yield* ensureWorktree({ + workspaceRoot, + worktreePath, + worktreeBranchName: state.t3.worktreeBranchName, + startCommitSha: state.t3.startCommitSha, + startBranchName: state.t3.startBranchName, + }); + + const commandId = CommandId.make(yield* randomUUID); + const createdAt = yield* getNow; + + const modelSelection = + project.defaultModelSelection ?? getAutoBootstrapDefaultModelSelection(); + + yield* orchestrationEngine + .dispatch( + OrchestrationCommand.make({ + type: "thread.create", + branch: state.t3.worktreeBranchName, + worktreePath: worktreePath, + threadId: state.t3.threadId, + // T3 generates the real title after the first turn starts. + title: DEFAULT_THREAD_TITLE, + modelSelection: modelSelection, + commandId, + createdAt, + projectId: project.id, + runtimeMode: "full-access", + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + }), + ) + .pipe( + Effect.asVoid, + /* + Our "thread is missing" observation can be stale (crash after a + committed create, projection lag). If the thread turns out to + exist, this step is already done: swallow the failure and carry on. + */ + Effect.catch((cause) => + projectionSnapshotQuery.getThreadShellById(state.t3.threadId).pipe( + Effect.map(Option.isSome), + Effect.orElseSucceed(() => false), + Effect.flatMap((threadExists) => + threadExists + ? Effect.void + : Effect.fail( + new RetryableError({ + method: "orchestrationEngine.dispatch", + reason: + "Could not dispatch thread.create for thread " + state.t3.threadId, + cause, + }), + ), + ), + ), + ), + ); + + /* + Script failures are thread-provisioning errors: fatal. + We don't pretend stuff is working if it's not. + */ + yield* projectScriptRunner + .runForThread({ + threadId: state.t3.threadId, + projectId: project.id, + projectCwd: project.workspaceRoot, + worktreePath, + }) + .pipe( + orFail("fatal")( + "projectScriptRunner.runForThread", + "Failed to run scripts while provisioning thread", + ), + ); + }).pipe( + Effect.tapError((error) => + error._tag === "FatalError" + ? gitWorkflowService + .removeWorktree({ cwd: workspaceRoot, path: worktreePath, force: true }) + .pipe(Effect.ignore) + : Effect.void, + ), + ); + }); + + return { + planCoordinates, + getThreadStatus, + provisionThread, + getTurnStatus, + startTurn, + threadActivity, + }; + }, +); + +export const t3GatewayLive = Layer.effect(T3Gateway, T3GatewayLive); diff --git a/docs/planning/create-thread.adversarial-review.md b/docs/planning/create-thread.adversarial-review.md new file mode 100644 index 000000000000..3a453b50108c --- /dev/null +++ b/docs/planning/create-thread.adversarial-review.md @@ -0,0 +1,72 @@ +# Adversarial review: `createT3Thread` (NTBS processor) + +Target: `createT3Thread` in [apps/server/src/ntbs/processor.ts](../../apps/server/src/ntbs/processor.ts) +(lines ~219–308 at review time). + +Compared against its two existing siblings, which encode lessons this code has not absorbed yet: + +- Jira auto-create flow: [apps/server/src/jira/JiraIssueBridge.ts](../../apps/server/src/jira/JiraIssueBridge.ts) (~395–499) +- ws bootstrap flow: [apps/server/src/ws.ts](../../apps/server/src/ws.ts) (~1098–1156) + +## Business-logic cracks (ranked) + +### 1. Worktree leak on `thread.create` failure — FIXED + +### 2. Every error cause is thrown away — FIXED + +### 3. `t3Context.baseRef` is used raw — FIXED + +The field is renamed `revision` → `baseRef` with its contract documented on `T3Context` and in +`ntbs-architecture.md`. `resolveWorktreeBase` implements the resolution: fetch `origin` (failure +tolerated separately, so an offline host still resolves against its last-known tracking ref), then +prefer the remote state via `resolveRemoteTrackingCommit` (passing `baseRefName` for merge-base +metadata), falling back to raw passthrough where git resolves the ref itself and a genuine failure +surfaces from `createWorktree` with its cause. No new `GitWorkflowService` surface was needed. + +Deferred detail: empty `baseRef` is not rejected up front — it fails in `createWorktree` with a git +cause instead of a crisp contract error. Revisit when the first inbound layer produces the value. + +### 4. Missing `deferDependencyInstall` — FIXED + +`createT3Thread` now passes `deferDependencyInstall: true` to `createWorktree`, matching the ws +pattern, since setup scripts run afterwards. + +### 5. Duplicate-request race (adjacent — `processAdapterRequest`) — MOSTLY FIXED + +Concurrent duplicates are now refused, not raced. The design: + +- `NTBSAdapter.getRequestKey(request)` defines the stable identity of a platform request + (deterministic, distinct per request, stable across redeliveries) — the same identity + `findByRequest` looks up. +- `processAdapterRequest` keeps an in-flight `Set` of keys: check-and-add happens synchronously + before the first yield (single-threaded, so no race), a present key drops the duplicate with a + debug log, and `Effect.ensuring` — wrapping only the admitted work, so a dropped duplicate + cannot erase the winner's key — removes the key on success, failure, or interruption. +- `findByRequest` remains as the durable dedup for later redeliveries (after completion or + restart); the set only covers requests running right now. +- Waiting/queueing duplicates behind the winner was considered and rejected: reliability is the + winner's own job (a bounded `Effect.retry` around creation — still TODO), not a side effect of a + duplicate happening to be queued. + +Remaining follow-up, deferred to the adapter storage schema work: a unique constraint on the +request key for stored `ThreadCreated` records, as the durable backstop for what the in-process +set cannot see (crash mid-creation, multi-process future). + +(The `return Effect.void` smell noted here earlier is fixed — both sites use a bare `return`.) + +## Simplification / abstraction + +### Smaller cleanups + +- `buildTemporaryWorktreeBranchName(() => threadUUID)` works and is a tested pattern + (`packages/shared/src/git.test.ts`), but it ignores the callback's `byteLength` parameter and + truncates the UUID to 8 hex chars — to a reader it looks like a bug. Either a short comment or a + dedicated `buildWorktreeBranchNameFromThreadId(threadUUID)` wrapper in `shared/git` would make + the intent explicit. + +## Deliberate choice needing a conscious sign-off + +`runtimeMode: "full-access"` for threads triggered by _external platform actors_. Jira does the +same, so it is consistent — but it means anyone who passes the platform trigger/actor check gets an +unrestricted agent in the repo. Fine if the actor checks are the trust boundary; write that down +where the boundary is enforced. diff --git a/docs/planning/directories.md b/docs/planning/directories.md new file mode 100644 index 000000000000..10f8104c89e2 --- /dev/null +++ b/docs/planning/directories.md @@ -0,0 +1,82 @@ +# Directories: workspaceRoot, worktreePath, cwd + +Three names, but only **two real places on disk**. `cwd` is not a third place — it is a parameter naming which of the two a git command should run in. + +## The two places + +**`project.workspaceRoot`** — the repository the user registered as the project (`OrchestrationProject.workspaceRoot`, stored on the project row). It is the _source_ repository: fetches, ref resolution, and `git worktree add`/`remove` all run here. It may be a **bare** repo — bare is explicitly supported for exactly this plumbing (`allowBare` in `GitWorkflowService`), because a thread never works inside it. + +**`thread.worktreePath`** — the per-thread checkout minted by `git worktree add`, recorded on the thread row by `thread.create` (or `thread.meta.update`). Nullable: a thread without one works directly in `workspaceRoot`. When `createWorktree` is called with `path: null`, the driver derives the path itself (`GitVcsDriverCore.createWorktree`): + +``` +/worktrees// +``` + +## The one rule + +Everything that runs "inside the thread's code" — provider sessions, terminals, checkpoints, setup scripts, title generation — resolves its directory as: + +```ts +effectiveCwd = thread.worktreePath ?? project.workspaceRoot; +``` + +Canonical helper: `resolveThreadWorkspaceCwd` (`apps/server/src/checkpointing/Utils.ts`). Beware: some call sites bind the _resolved_ value to a parameter named `workspaceRoot` (e.g. `issueAssetUrl` in `ws.ts`) — read the resolution, not the name. + +## Which place does each gateway call use? + +| Operation | `cwd` to pass | +| ------------------------------------------------------------ | ------------------------------------- | +| `remoteExists`, `fetchRemote`, `resolveRemoteTrackingCommit` | `workspaceRoot` | +| `createWorktree`, `removeWorktree` | `workspaceRoot` (+ `path` = worktree) | +| status / branch ops on the thread's work | `worktreePath` | +| setup scripts (`runForThread`) | `worktreePath` (+ `projectId`) | + +So `provisionThread` is (mirroring `processor.old.ts#createT3Thread`): + +1. `workspaceRoot = getProject(state.t3.projectId).workspaceRoot` — looked up fresh, never stored. +2. `worktreePath = deriveDefaultWorktreePath(...)` (see below), then `createWorktree({ cwd: workspaceRoot, refName: startCommitSha, baseRefName: startBranchName, newRefName: worktreeBranchName, path: worktreePath, deferDependencyInstall: true })` +3. `thread.create` with `{ branch: worktreeBranchName, worktreePath }` +4. `runForThread({ threadId, projectId, worktreePath })` + +## Should `WorkCoordinates` reference a path? + +**`workspaceRoot`: no.** It is live project state, always derivable from `projectId`, and can change +if the project is re-registered or moved. A stored copy in a durable exchange record goes stale; +coordinates should stay identity + pinned git state. + +**`worktreePath`: no — derive it, for the same reason.** The path is fully determined by +`worktreeBranchName` (already in the coordinates) joined with live state: `worktreesDir` +(ServerConfig) and `basename(project.workspaceRoot)`. Storing the absolute path would bake the +same staleness in — if t3-home moves, the exchange DB moves with it, and a stored path would point +at the old home while a derived one stays correct by construction. Identity lives in the durable +record; location is computed from live state. + +What that requires: + +- **One shared helper.** The formula lives inline in `GitVcsDriverCore.createWorktree` today. + Extract `deriveDefaultWorktreePath({ worktreesDir, workspaceRoot, branchName })` and use it for + both the driver's `path: null` fallback and the gateway, so they cannot drift. +- **Derive once per provision attempt**, then pass the value explicitly as `path` to + `createWorktree` — the concrete value is needed anyway for the reentrancy checks: + - path exists on disk → skip `worktree add` + - branch exists, path missing → `worktree add ` without `-b` (the recreate + pattern in `ws.ts` bootstrap) + - thread projected → skip `thread.create` +- **After `thread.create`, stop deriving.** The thread row's `worktreePath` records the path + actually used and is the source of truth from then on (the `?? workspaceRoot` rule, cleanup, + restore). Derivation only covers the window before the thread exists. +- **Accepted degradation:** if the formula or the project's path changes inside the crash window, + the retry derives a new path, finds branch-exists/path-missing, and recovers via the + no-`-b` add — leaking one orphan directory, the same class of accepted leak as the branch. + +## Known traps + +- `GitWorkflowService.remoteExists` does **not** pass `allowBare`, while `fetchRemote`, + `resolveRemoteTrackingCommit` and `createWorktree` do. `planCoordinates` calls it first, so a bare + `workspaceRoot` fails there — and as a `RetryableError`, i.e. it retries forever on a permanent + condition. +- `worktreeBranchName` is `t3code/<8 hex>` (`buildTemporaryWorktreeBranchName`); the driver + sanitizes `/` to `-` only for the directory name, not the ref. +- Branch cleanup is intentionally leaky: worktree removal keeps the branch + (`WorktreeLifecycle.cleanupThreadWorktree` invariant), so "branch already exists" on retry is the + expected case, not a corrupt state. diff --git a/docs/planning/ideas.md b/docs/planning/ideas.md new file mode 100644 index 000000000000..a7a4546d39b3 --- /dev/null +++ b/docs/planning/ideas.md @@ -0,0 +1,133 @@ +# NTBS ideas + +## Keep the shared lifecycle small + +There is a tradeoff between recovering every possible interruption and keeping the first implementation simple. A saved `RequestAccepted` state could recover the rare case where the server receives a request but stops before creating its T3 thread. Doing that safely would also require planned thread IDs, startup searches, retries, and duplicate handling. + +For now, the shared lifecycle should begin with `ThreadCreated`. The processor should save it as soon as the basic T3 thread exists, before slower preparation begins. A request can be lost if the server stops before that point, but the missing acknowledgement makes the failure visible and the user can send the request again. + +A stronger recovery system can be added later if real usage requires it. Each adapter could inspect recent messages on its platform, find requests that have no corresponding T3 thread, and submit them again. This belongs to the adapter because Jira, GitHub, Discord, and Teams provide different ways to read their recent messages. + +## Durable request dedup is necessary yet insufficient + +The `adapter.findByRequest` check at the start of `processAdapterRequest` (`processor.ts`) cannot be +removed. It is the only durable dedup in the pipeline: `inFlightRequests` is in-memory, covers only +concurrent deliveries inside one process, and is cleared the moment a request finishes or the server +restarts. The source platforms deliver at-least-once (webhook retries, Discord gateway replays), so +without this check a late redelivery would create a second worktree, thread, and turn, and post a +second final response. Deferring the check to a uniqueness conflict in `adapter.save` would be +worse, because the conflict would surface only after the expensive thread provisioning already ran. + +The check is still insufficient on its own. It is check-then-act against the adapter store, and +between `createT3Thread` and the `adapter.save` of `ThreadCreated` there is a crash window where no +record exists yet: a redelivery after a crash there passes `findByRequest` and provisions a +duplicate thread. The fix, if real usage ever needs it, is not another read but an atomic +insert-if-absent reservation keyed by `getRequestKey` before thread creation. That is the same +tradeoff already described in "Keep the shared lifecycle small": a pre-thread lifecycle state plus +recovery for stale reservations. Defer it until a production adapter observes redelivery during a +crash; the dedup behavior itself is pinned by the processor test that drops a redelivered request +with a recorded thread. + +## Remove acknowledgement from the shared lifecycle + +The acknowledgement is platform feedback, such as a "working on it" message. It should not be a required stage in the shared NTBS lifecycle because failing to post it, or failing to save its message ID, must not prevent the processor from representing and posting the final response. + +Remove `ThreadCreatedAcknowledgement` from the lifecycle and remove `acknowledgementMessageId` from `ResponseAvailable` and `ResponsePosted`. The adapter may still post an acknowledgement and retain its identifier in its own platform-specific storage when needed. When posting the final response, the adapter can reply to the acknowledgement or fall back to the original source message according to the platform's capabilities. + +The shared sequence becomes: + +`Create the T3 thread → record ThreadCreated → start the work and attempt the acknowledgement independently` + +The processor does not use acknowledgement success as a condition for continuing. Posting may happen alongside the start of T3 work, and an adapter may retry a failed acknowledgement, but the final answer always remains tied to the original response destination. If a platform benefits from replying to the acknowledgement, its adapter can use the identifier stored in its own data without adding that dependency to the shared lifecycle. + +## Persist response intent before posting (outbox pattern) + +`postResponse` in `processor.ts` posts the final response to the platform and then saves +`ResponsePosted`. Because the post targets an external platform and the save targets the local +store, no transaction can span both, and compensation (deleting the posted message when the save +fails) cannot close the hole either: the failure mode that matters is process death between post +and save, and a dead process runs no compensation. Deleting an already-read correct answer because +a local write failed is also worse UX than retrying the save, and some adapters (Jira comments, +restricted channels) may lack delete permission entirely. + +Today that crash window is covered by `findMatchingResponseMessage`, which probes the platform by +response content on every post. Content is the wrong identity test: the recomputed outcome can +drift across restarts (a posted timeout resolves as a cancellation after recovery interrupts the +turn; the `error` branch depends on `session.lastError`), so the probe misses the earlier response +and a second, differently-worded final response gets posted. + +The fix is to persist intent before posting: + +1. Save a `thread.response.posting` state carrying the response payload (type + text). +2. Post to the platform. +3. Save `thread.response.posted` with the platform message ID. + +This gives recovery precision (only records stuck in `response.posting` may have an unrecorded +post — `thread.created` records are known-unposted and need no platform search), removes the drift +bug (recovery reposts the stored text instead of recomputing the outcome), and makes a failed +step-3 save trivially retryable. The platform probe shrinks to a rarely-exercised recovery path, +and its contract should be "any final response this adapter already posted for this request" +(`findResponseMessage(state)`, no `response` parameter) rather than content matching. + +## Remove fork-specific provenance after the NTBS migration + +Keep `SourceChannel`, `SourceRef`, `sourceHint`, `originSource`, and related fork-specific provenance out of the NTBS design. Adapters already retain the platform data needed to connect external messages with T3 work. + +Once every external platform has moved to NTBS, remove these fields and the old integration logic that depends on them. + +## Keep remote adapters possible + +The first NTBS adapters can run inside the T3 server, but some platform integrations may remain separate programs. The current Discord bot is one example. + +When a remote adapter is implemented, either move its platform operations into the server or expose the processor and adapter operations through a network API. The shared lifecycle and storage design should not require every adapter to share the T3 server process. Choose the transport when the first remote adapter is ported. + +## Decide thread archival after testing + +Keep NTBS-created T3 threads after their responses are posted for now. Once the workflow has been tested in practice, decide whether completed threads should be archived automatically and under which conditions. + +## Add worktree cleanup to the Jira bridge + +The Jira auto-create flow (`JiraIssueBridge.ts`) creates a worktree before dispatching +`thread.create` but has no compensation: a failed dispatch orphans the branch and worktree. The +NTBS processor fixed this with `Effect.onError` → forced `removeWorktree` (cleanup errors logged, +original cause re-raised). Rather than patching the bridge separately, extract the shared +`provisionThreadWorktree` helper proposed in `create-thread.adversarial-review.md` and let both +flows use it — the Jira bridge is expected to collapse onto NTBS eventually anyway. + +## Delete the temporary branch when thread provisioning fails + +When `thread.create` fails after `createWorktree`, the NTBS processor removes the worktree but +retains the `t3/wt-…` branch (documented in `processor.ts` as an accepted leak). Everywhere else, +branch retention is deliberate — `WorktreeLifecycle.cleanupThreadWorktree` keeps the branch so +`restoreThreadWorktree` can recreate the worktree on unarchive — but a failed provision has no +thread and nothing restorable, so retention buys nothing there. + +If the ref noise ever matters, the shape is: + +- Add `deleteTemporaryWorktreeBranch({ cwd, refName })` to `GitWorkflowService`, hard-guarded with + `isTemporaryWorktreeBranch` so it structurally cannot delete a real branch. Plumbing precedent: + checkpoint refs are deleted via `update-ref -d` in `GitVcsDriver.ts`, which also skips the + checked-out/merged safety checks. +- In the processor's failure cleanup: `removeWorktree({ force: true })` first, then the branch + delete (git refuses to delete a branch still checked out in a worktree), each step best-effort + with its own log warning. +- Comment on the service op why this exception to branch retention exists, so it is not + "harmonized" with `cleanupThreadWorktree`'s keep-the-branch behavior. + +## Use Deferred for asynchronous test synchronization + +Effect's `Deferred` is useful as a one-shot, promise-like latch when a test needs to wait for an +asynchronous operation to reach a specific point. The code under test completes it, while the test +awaits it deterministically, avoiding arbitrary sleeps, flaky timing assumptions, and unnecessary +polling. Use it to coordinate milestones such as a subscriber consuming an event; direct +`processor.process` tests generally do not need it. + +## Start the T3 event subscription with the processor + +Processors should subscribe to T3 events automatically as part of their managed startup +lifecycle, rather than exposing `subscribeToT3Events` for callers to invoke. The public processor +API should focus on business operations such as `process`; the subscription and stored-thread +recovery should start when the processor layer is provided and stop with its application scope. +Use a scoped resource or layer so the background fiber is owned, interruptible, and cannot be +accidentally started twice by callers. Tests should construct the live processor, publish an event, +and assert the observable result without manually starting the subscription. diff --git a/docs/planning/ntbs-architecture.md b/docs/planning/ntbs-architecture.md new file mode 100644 index 000000000000..12e14147b13a --- /dev/null +++ b/docs/planning/ntbs-architecture.md @@ -0,0 +1,178 @@ +# NTBS architecture + +**Status:** exploratory planning + +This document defines the boundary between T3 and adapters for non-turn-based surfaces such as Jira, GitHub, Discord, and Teams. It explains which system retains which information and the shared path from an external event to a T3 result and back to the external platform. + +## Problem + +T3 clients are built around T3 data views such as threads, diffs, and projects. External platforms know none of those concepts. They only know their own messages, comments, conversations, and identifiers. + +An adapter therefore cannot rely on an external platform to retain T3 state, and T3 cannot infer where a later result belongs from its own thread data alone. The adapter must retain the link between its platform's event and the T3 work created from it. + +## Shared model + +An adapter receives a platform event, applies the trigger rules, captures the source snapshot, and creates a new T3 thread. It retains the platform identifiers and the T3 identifiers created from that event. + +The adapter sends an acknowledgement to the external platform. When T3 reports the thread's final outcome, the adapter uses its retained record to post the final answer, failure, timeout, or cancellation in the correct place. + +T3 remains independent of the platform that produced the event. It owns its threads, messages, turns, execution state, worktrees, and branches. The adapter owns platform authentication, event delivery, source snapshots, platform identifiers, response placement, and platform-specific rendering. + +The adapter keeps the full record for its platform. T3 does not receive or interpret platform data. The adapter makes sure the same platform message does not start T3 work twice, creates the snapshot, and asks T3 to start work. T3 receives the snapshot, returns the new thread, message, and turn IDs, and later reports the final outcome. The adapter adds those T3 values to its own record and posts the result on its platform. + +Storage and retention are adapter implementation details, not architecture decisions. Platform-specific edge cases, such as a source item being deleted or closed while T3 is working, also belong to the adapter implementation phase. + +## Passing T3 context + +An incoming platform event carries both platform data and the T3 context needed to start work, such as the project, base ref, and execution context. The base ref is the starting point for the thread's worktree — usually a branch name such as `main`, resolved against `origin` before use, or a commit SHA used as-is. The adapter forwards that T3 context to T3 when it creates the new thread. + +`NtbsEvent` does not retain the project, base ref, or execution context as lifecycle data. Once T3 creates the thread, T3 owns that information. Keeping copies in `NtbsEvent` would require the adapter to keep them in sync with T3. + +## Receiving T3 outcomes + +Adapters subscribe to T3's event log, like other T3 consumers. After T3 starts a thread, `NtbsEvent` contains its turn ID. When the adapter receives the final outcome for that turn from the event log, it uses the same `NtbsEvent` to post the result on the external platform. + +## Event lifecycle + +Starting from an external event, this happens: + +1. The adapter accepts an external event that matches a trigger. It creates an adapter record containing the source identifiers, response destination, and captured snapshot. +2. The adapter asks T3 to create a new thread from that snapshot. +3. T3 creates the thread, user message, and turn. The adapter adds those IDs to its record. +4. The adapter posts the acknowledgement and adds its message ID to the record. +5. T3 produces the final answer, failure, timeout, or cancellation for that turn. +6. The adapter finds the record from the T3 IDs, posts the final message at its stored response destination, and adds the final-message ID to the record. + +## Adapter record + +Before it asks T3 to create a thread, the adapter record contains: + +- the adapter's platform data; +- the captured source snapshot as a string; + +After T3 creates the thread, the adapter adds the T3 thread, user-message, and turn IDs. + +After it posts the acknowledgement and final response, the adapter adds their message IDs. + +The event retains the accepted source event, the new T3 thread it starts, and the messages the adapter sends for that thread. + +`NtbsEvent` is a TypeScript pattern for adapter code, not a shared storage format. Each adapter defines, validates, and stores its own platform data. + +```ts +/** All data that is specific to the external platform. */ +type PlatformData = { + /** Information about the inbound event. */ + source: Source; + /** Information about where replies belong. */ + responseDestination: ResponseDestination; +}; + +/** + * Tracks the lifecycle of external inbound events, such as comments or messages, that trigger T3 work. + * External applications have no relationship to T3, and vice versa. The adapter relates events in one to the other. + */ +type NtbsEvent

> = + | NtbsEventAccepted

+ | NtbsEventThreadCreated

+ | NtbsEventAcknowledgementPosted

+ | NtbsEventOutcomeAvailable

+ | NtbsEventResponsePosted

; + +type NtbsEventBase

> = { + /** Adapter-defined data for the external platform. T3 does not inspect it. */ + platformData: P; + /** The captured source text used to create T3's first user message. */ + snapshot: string; +}; + +type NtbsEventAccepted

> = NtbsEventBase

& { + /** The adapter has accepted the inbound event but has not started T3 work. */ + state: "accepted"; +}; + +type NtbsEventWithThread

> = NtbsEventBase

& { + /** The T3 IDs created after the adapter starts work. */ + t3: { + /** The T3 thread created from the source event. */ + threadId: string; + /** The first T3 user message created from the snapshot. */ + userMessageId: string; + /** The T3 turn started from that message. */ + turnId: string; + }; +}; + +type NtbsEventThreadCreated

> = NtbsEventWithThread

& { + /** T3 has created the new thread from the source snapshot. */ + state: "threadCreated"; +}; + +type NtbsEventWithAcknowledgement

> = + NtbsEventWithThread

& { + /** The external acknowledgement message posted by the adapter. */ + acknowledgementMessageId: string; + }; + +type NtbsEventAcknowledgementPosted

> = + NtbsEventWithAcknowledgement

& { + /** The adapter has posted the acknowledgement. */ + state: "acknowledgementPosted"; + }; + +type NtbsEventOutcomeAvailable

> = + NtbsEventWithAcknowledgement

& { + /** T3 has produced a final outcome for the turn. */ + state: "outcomeAvailable"; + }; + +type NtbsEventResponsePosted

> = + NtbsEventWithAcknowledgement

& { + /** The adapter has posted T3's final response. */ + state: "responsePosted"; + /** The external final message posted by the adapter. */ + finalMessageId: string; + }; +``` + +TODO: Define error and retry lifecycle states when adapter behaviour is tested. + +## Jira example + +A user adds top-level Jira comment `10401` on issue `T3-123`: `@agent investigate the failed build`. The adapter accepts source event `jira-event-1`, version `1`, and stores this record before asking T3 to do anything: + +```ts +{ + state: "accepted", + platformData: { + source: { + eventId: "jira-event-1", + version: "1", + contextId: "T3-123", + messageId: "10401", + }, + responseDestination: { + contextId: "T3-123", + parentMessageId: "10401", + }, + }, + snapshot: "@agent investigate the failed build", +} +``` + +When T3 creates the work, the adapter adds its IDs: + +```ts +state: "threadCreated", +t3: { + threadId: "thread-1", + userMessageId: "message-1", + turnId: "turn-1", +} +``` + +The adapter posts an acknowledgement as a reply to Jira comment `10401`, changes the state to `"acknowledgementPosted"`, and adds `acknowledgementMessageId: "10402"`. When T3 produces the final result for `turn-1`, the state becomes `"outcomeAvailable"`. The adapter then posts another reply to comment `10401`, changes the state to `"responsePosted"`, and adds `finalMessageId: "10403"`. + +## Related documents + +- [ntbs.md](./ntbs.md) records the overall scope and agreed decisions. +- [ntbs-event-processing.md](./ntbs-event-processing.md) defines inbound triggers and outbound messages on each platform. diff --git a/docs/planning/ntbs-event-processing.md b/docs/planning/ntbs-event-processing.md new file mode 100644 index 000000000000..22391a340d95 --- /dev/null +++ b/docs/planning/ntbs-event-processing.md @@ -0,0 +1,124 @@ +# NTBS event processing + +**Status:** exploratory planning + +This document defines how events arriving from non-turn-based surfaces are classified and converted into T3 work. It focuses on which events start new T3 threads, which events are ignored, and how independently started threads are correlated with their external events and responses. + +T3 clients are built around T3 data views (projections): threads, diffs, and projects. + +External NTBSs like Jira, Discord, or GitHub know nothing about that: they have only limited capabilities for sending and receiving messages. + +The UX on these platforms has to be thoroughly scoped, and adapters to these platforms have to be extended to retain the information needed to connect T3 events to Jira, Discord, GitHub, or Teams events. + +## Inbound event processing + +### Core rule + +An external event starts a new T3 thread when the adapter recognizes it as one of the trigger forms defined below. Each triggering event creates a new T3 thread. NTBS does not explicitly target, continue, steer, or modify an existing T3 thread. + +The event is captured together with the source snapshot used to construct the first user message. The adapter must distinguish a source event from delivery attempts. Retrying or redelivering the same source event must not create another T3 thread. + +### Adapter storage + +For each inbound event, the adapter retains: + +- the source event ID and version, to avoid handling the same event twice; +- the source context and message or comment IDs, so it knows where the event came from; +- the captured source snapshot; +- the T3 thread, user-message, and turn IDs created from the event. + +### Platform triggers + +The following source interactions start a new thread: + +#### Jira + +- A top-level comment mentioning the agent. +- A reply mentioning the agent. +- A comment edit that adds the agent mention to a comment that previously did not invoke the agent. +- An edit to a comment that already invoked the agent does not trigger a new thread merely because its content changed. A new turn requires a new explicit invocation under the edited comment. + +#### GitHub + +- An issue or pull request comment mentioning the agent. +- A pull-request review comment or reply mentioning the agent. +- A comment edit that adds the agent mention to a comment that previously did not invoke the agent. +- An edit to a comment that already invoked the agent does not trigger a new thread merely because its content changed. A new turn requires a new explicit invocation under the edited comment. + +#### Discord + +- A human message mentioning the configured agent user. +- A human reply to an agent-authored message. +- A message edit that adds the configured agent mention to a message that previously did not invoke the agent. +- Editing a message that already invoked the agent does not start another thread merely because its content changed; a new turn requires a new explicit invocation. + +### Processing a trigger + +When a source interaction matches one of the triggers above, the adapter deduplicates the source event and captures the source snapshot used for the new thread. TODO: define the source event identity and idempotency rules, including late and out-of-order deliveries. + +T3 then starts a new thread from that event and snapshot. A thread already running for the same external interaction does not delay, absorb, continue, or modify the new thread. + +### Events that do not trigger work + +- Edits to a comment that already invoked the agent, including edits that change its content, unless the edited comment contains a new explicit invocation. +- Duplicate or already-accepted deliveries do not create another thread. TODO: define the stable event identity and idempotency rules, including late and out-of-order deliveries. + +Events that do not match one of the triggers above are ignored. Whether adapters retain them for deduplication, audit, or external-state projection is a separate concern. + +### Concurrent turns + +Multiple events from the same external interaction may create T3 threads at the same time. Each thread produces an answer for its own triggering event and sends that answer to the exact response destination associated with that event. Threads may finish in any order; completion order does not change where their answers are sent. + +### Consequences + +- Each event has isolated T3 context; a thread does not inherit the conversation history of another event. +- Each response must retain the exact destination associated with its triggering event. +- Capturing the source snapshot supports reproducibility, but may increase input size, latency, and model cost. +- High-volume external interactions may create many T3 threads and increase storage and discovery noise. + +### Summary + +- An invocation creates an independent T3 thread; it does not target or continue an existing thread. +- Multiple events from the same external interaction may create concurrent threads. +- Each thread produces its own answer, routed to the exact response destination associated with its originating event. +- Duplicate or already-accepted deliveries do not create another thread. TODO: define stable event identity and idempotency rules. + +## Outbound response processing + +Outbound processing adds the acknowledgement and final-outcome message IDs, together with whether each message was posted. + +### Agreed decisions + +Only the acknowledgement and the final answer, failure, timeout, or cancellation are rendered on the external platform. All other T3 events remain internal. + +Each inbound event creates an immediate outbound acknowledgement. When its T3 thread ends, the adapter sends the final answer, failure, timeout, or cancellation as a new message after that acknowledgement, in the platform's native conversation scope. + +#### Response format + +Acknowledgements and final messages are text. Adapters use the platform's Markdown-like formatting, including fenced code snippets when useful. + +T3 does not use interactive controls, permission requests, or multiple-choice prompts on external platforms. Any question is written as ordinary text. + +#### Delivery failures + +The adapter posts the result or error as the final message. If delivery fails for a recoverable reason, it retries; otherwise the original working message remains without a follow-up, and the user may start a new request. + +#### Message identifiers and placement + +Each adapter defines how these identifiers and message relationships map to its platform: + +##### Jira + +The adapter retains the issue ID or key, invoking comment ID, root comment ID, acknowledgement comment ID, and outcome comment ID. It posts the acknowledgement and outcome as separate replies to the same root comment. + +##### GitHub + +The adapter retains the repository, pull-request number, invoking comment ID, root review-comment ID when the invocation is in a review thread, acknowledgement message ID, and outcome message ID. In a review thread, the acknowledgement and outcome both reply to the root review comment. For ordinary issue or pull-request comments, they are separate timeline comments on the pull request. + +##### Discord + +The adapter retains the thread or channel ID, invoking message ID, acknowledgement message ID, and outcome message ID. The acknowledgement replies to the invoking message, and the outcome replies to the acknowledgement. + +##### Teams + +The adapter retains the team and channel or chat ID, root conversation-message ID, invoking message ID, acknowledgement message ID, and outcome message ID. The acknowledgement and outcome are separate replies in the same root conversation. diff --git a/docs/planning/ntbs-plan.md b/docs/planning/ntbs-plan.md new file mode 100644 index 000000000000..802de7c5bdea --- /dev/null +++ b/docs/planning/ntbs-plan.md @@ -0,0 +1,63 @@ +# NTBS implementation plan + +**Status:** exploratory planning + +## 1. Understand the existing mechanics + +Read the orchestration command definitions, the orchestration engine service, and the WebSocket turn-start handling to understand how T3 creates threads, prepares worktrees, starts turns, persists events, and exposes those events to consumers. + +Then follow the current Jira path from the webhook route and payload parser through the Jira bridge, delivery store, and Jira API client. This provides concrete examples of inbound event handling, platform-owned persistence, T3 command dispatch, acknowledgement delivery, outcome detection, and outbound response placement. + +The current Jira bridge is a reference, not the desired architecture. It contains platform-independent behavior that should move into the shared NTBS implementation, and it currently reuses existing threads instead of creating a new thread for every accepted event. + +## 2. Build the platform-agnostic NTBS implementation + +Create `apps/server/src/ntbs` for the shared lifecycle model, adapter contract, and workflow service. + +First, extract the existing create-thread, prepare-worktree, and start-turn mechanic from the WebSocket handler into a reusable orchestration service. Both native T3 clients and NTBS workflows should call this service so thread creation behaves consistently regardless of where the request originated. + +Define an adapter contract that leaves platform data opaque to the shared workflow. Each adapter supplies persistence, duplicate prevention, acknowledgement delivery, final-response delivery, and the platform-specific data needed to place those messages. + +Implement the shared workflow: + +1. Accept the snapshot, T3 context, and opaque platform data from an adapter. +2. Persist the accepted lifecycle state before starting T3 work. +3. Create a new T3 thread and worktree, start its first turn, and retain the resulting T3 identifiers. +4. Ask the adapter to post the acknowledgement and retain its platform message identifier. +5. Consume T3 events, including replay after a restart, and identify the final outcome for the recorded work. +6. Load the final assistant text or failure information and ask the adapter to post the final message. +7. Persist every lifecycle transition so interrupted processing can resume safely. + +Confirm when the T3 turn ID becomes available during this work. The current command path knows the thread and user-message IDs immediately but discovers the turn ID later. The implementation and lifecycle types must represent that sequence accurately. + +Test the shared workflow with an in-memory adapter implementation before connecting it to a real platform. The tests should cover successful completion, failure, duplicate delivery, restart recovery, and concurrent events. + +## 3. Port Jira onto the shared implementation + +Keep Jira webhook verification, payload parsing, trigger recognition, Jira identifiers, and Jira API calls inside the Jira adapter. + +Replace the shared workflow currently embedded in the Jira bridge with an implementation of the NTBS adapter contract. Adapt the Jira delivery store to persist the NTBS lifecycle together with Jira-specific source and response-destination data. + +Change Jira processing so every accepted event creates a new T3 thread. Preserve the agreed outbound behavior: post an acknowledgement for the invoking comment, then post the final answer, failure, timeout, or cancellation as a separate reply in the same Jira comment scope. + +Update the Jira tests to prove trigger handling, duplicate prevention, lifecycle recovery, new-thread creation, acknowledgement placement, final-response placement, and concurrent invocations. + +# Notes + +In `packages/contracts/src/orchestration.ts` we can find the schema `ThreadTurnStartBootstrapCreateThread`. + +The schema wants: + +- `projectId` (project should be inferred by discord/jira/etc) +- `title` (generated somewhere) +- `modelSelection` (some model) +- `runtimeMode` (permissions) +- `interactionMode` (apparently default vs plan) +- `branch` (git branch?) +- `worktreePath` (where is it on filesystem) + +It is then used by the + +`ThreadTurnStartBootstrap` which has some optional data for running setup script, preparing worktrees which is then used by + +`ThreadTurnStartCommand` and `ClientThreadTurnStartCommand` (essentially the same type) diff --git a/docs/planning/ntbs-processor.cc-review.md b/docs/planning/ntbs-processor.cc-review.md new file mode 100644 index 000000000000..480ff7ddc595 --- /dev/null +++ b/docs/planning/ntbs-processor.cc-review.md @@ -0,0 +1,93 @@ +# NTBS directory review + +**Status:** Review notes (Claude Code, 2026-08-14; reconciled with the monitor-free processor on 2026-08-15). Addressed findings have been removed, so numbering gaps are intentional. + +**Scope:** The six files in [`apps/server/src/ntbs`](../../apps/server/src/ntbs/) and the orchestration/projection behavior they directly depend on. + +The processor/adapter boundary is generally clean. Startup recovery and the live `thread.session-set` listener now have distinct roles: recovery starts a missing turn or immediately reconciles a terminal one, while active turns are left to the listener. The remaining findings are refinements, ordered by practical impact. + +--- + +## 1. Simplifications, naming, and contracts + +### API and business logic + +**S6. Consolidate the test harnesses.** + +Most of [`test-helpers.ts`](../../apps/server/src/ntbs/test-helpers.ts#L99) is unexported and unused; only `createGitLayerMock` and `createAdapterRequest` are imported by [`processor.test.ts`](../../apps/server/src/ntbs/processor.test.ts#L17). Meanwhile, [`processor2.test.ts`](../../apps/server/src/ntbs/processor2.test.ts#L27) contains a separate, already-divergent harness. Its state-service design is the stronger base, including the `threadLookups` queue used for synchronization. + +Keep one harness, move any reusable pieces into `test-helpers.ts`, and merge the tests into one `processor.test.ts`. The current “happy case” in [`processor.test.ts`](../../apps/server/src/ntbs/processor.test.ts#L156-L165) has no assertion, and its `eventReceived` deferred is created but never observed ([`processor.test.ts:22–42`](../../apps/server/src/ntbs/processor.test.ts#L22-L42)). Also fix the worktree fake: it reports `input.refName` as the created branch instead of `input.newRefName` ([`test-helpers.ts:34–42`](../../apps/server/src/ntbs/test-helpers.ts#L34-L42)). + +**S7. Deduplicate the fixed runtime settings.** + +`runtimeMode: "full-access"` and `interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE` are repeated in both turn and thread creation ([`processor.ts:223–240`](../../apps/server/src/ntbs/processor.ts#L223-L240), [`processor.ts:538–552`](../../apps/server/src/ntbs/processor.ts#L538-L552)). One module-level constant would state that policy once and provide the natural home for a future override. + +### Naming and contracts + +**N2. `ThreadEvent` is a stored record, not an event.** + +[`ThreadEvent`](../../apps/server/src/ntbs/lifecycle.ts#L39-L50) is the common stored shape for the two lifecycle states. `ThreadRecord` or `LifecycleBase` would say what it is. The contract fields are also mutable while the processor treats them as immutable; make `sourceUri`, `snapshot`, `attachments`, `t3Data`, its nested IDs, `state`, and `responseMessageId` `readonly` ([`lifecycle.ts`](../../apps/server/src/ntbs/lifecycle.ts#L3-L65)). + +**N3. Remove or correct stale comments.** + +- The architecture block still refers to generic `NTBSInput

`, although the generic platform data was removed ([`processor.ts:35–39`](../../apps/server/src/ntbs/processor.ts#L35-L39)). +- The event path says it selects the “last user message,” but it uses the one original user-message ID stored for the request; no selection occurs ([`processor.ts:377–381`](../../apps/server/src/ntbs/processor.ts#L377-L381)). +- Two outcome-lock comments still refer to timeout handling, which no longer exists ([`processor.ts:179–182`](../../apps/server/src/ntbs/processor.ts#L179-L182), [`processor.ts:383–388`](../../apps/server/src/ntbs/processor.ts#L383-L388)). +- The recovery-test TODO still says recovery should “monitor” the turn, and the second harness still mentions monitor baselines ([`processor.test.ts:84–85`](../../apps/server/src/ntbs/processor.test.ts#L84-L85), [`processor2.test.ts:36–38`](../../apps/server/src/ntbs/processor2.test.ts#L36-L38)). +- `acknowledge` returns `Effect`, not a platform message identifier ([`adapter.ts:38–43`](../../apps/server/src/ntbs/adapter.ts#L38-L43)). +- The adapter, not the processor, creates the T3 attachment references passed through by the input ([`lifecycle.ts:32–36`](../../apps/server/src/ntbs/lifecycle.ts#L32-L36)). +- Fix “idenitifier” in the `postResponse` documentation ([`adapter.ts:44–54`](../../apps/server/src/ntbs/adapter.ts#L44-L54)). + +**N4. `adapter.save` does not state its upsert semantics or identity key.** + +The processor writes `thread.created` and later replaces it with `thread.response.posted` ([`processor.ts:340–346`](../../apps/server/src/ntbs/processor.ts#L340-L346), [`processor.ts:702–713`](../../apps/server/src/ntbs/processor.ts#L702-L713)), but the adapter contract only says “stores a lifecycle state” ([`adapter.ts:32–36`](../../apps/server/src/ntbs/adapter.ts#L32-L36)). State explicitly that this is an upsert and identify its key. The tests currently assume records are keyed by `threadId`, while `sourceUri` is documented as the durable request identity. + +**N5. The 120,000-character input limit names no enforcer.** + +[`NTBSInput.snapshot`](../../apps/server/src/ntbs/lifecycle.ts#L26-L31) documents the limit, but the processor does not validate it. Say that adapters must enforce it before calling `process`, or make the contract executable as proposed in the other review. + +**N6. Clarify who owns non-answer response text.** + +The processor supplies fixed English text for empty completions, failures, and cancellations ([`processor.ts:289–315`](../../apps/server/src/ntbs/processor.ts#L289-L315)), while [`NTBSResponse`](../../apps/server/src/ntbs/adapter.ts#L14-L17) carries both the semantic type and rendered text. If adapters may localize or replace this copy, document `text` as a default; otherwise the current contract means every platform must post the processor's prose verbatim. + +**N8. Spell out NTBS once.** + +None of the three production files expands the acronym. The architecture heading is the natural place to write “Non-Turn-Based Surfaces” ([`processor.ts:23–26`](../../apps/server/src/ntbs/processor.ts#L23-L26)). + +--- + +## 2. Bugs, edge cases, and race conditions + +**B1. A turn that never materializes leaves the request unanswered until restart.** + +`thread.turn.start` first creates a pending projected row. If the provider session settles before adopting it, the projection deliberately deletes that row ([`ProjectionPipeline.ts:1389–1406`](../../apps/server/src/orchestration/Layers/ProjectionPipeline.ts#L1389-L1406)). The terminal `thread.session-set` event still reaches the NTBS listener, but [`resolveT3Outcome`](../../apps/server/src/ntbs/processor.ts#L263-L275) treats the missing turn as an error; the event loop logs the failure and moves on ([`processor.ts:730–740`](../../apps/server/src/ntbs/processor.ts#L730-L740)). No final response is posted. + +Startup recovery eventually sees the missing turn and starts it again ([`processor.ts:615–630`](../../apps/server/src/ntbs/processor.ts#L615-L630)), but that makes a restart the only recovery path and may repeat a deterministic provider-start failure. Treat a missing turn as a state: inspect the thread session, return “still pending” for `null`/`starting`/`running`, produce a failure for a settled session (using `lastError` when appropriate), and treat a missing thread as cancellation. Keep restart recovery as the bounded retry path rather than restarting from the live terminal-event path. + +**B2. Startup recovery can race normal processing into two turn-start commands.** + +`process` saves `ThreadCreated` immediately before starting the turn ([`processor.ts:687–716`](../../apps/server/src/ntbs/processor.ts#L687-L716)). If `run` loads that record during the small save-to-dispatch window, recovery also sees no turn and starts it ([`processor.ts:743–771`](../../apps/server/src/ntbs/processor.ts#L743-L771)). The decider queues a second start when the first has already established `pendingTurnStart` ([`decider.ts:1171–1209`](../../apps/server/src/orchestration/decider.ts#L1171-L1209)); it does not make two commands with different command IDs idempotent merely because their message ID matches. + +The narrow fix is to have `recoverThread` skip records whose `sourceUri` is present in [`inFlightRequests`](../../apps/server/src/ntbs/processor.ts#L476-L480). That closes the duplicate-dispatch window without reintroducing monitor tracking; a failed normal turn-start remains the separate redelivery/reconciliation issue described in the other review. + +**B5. A crash between T3 thread creation and `adapter.save` orphans resources.** + +[`createT3Thread`](../../apps/server/src/ntbs/processor.ts#L493-L607) creates the worktree, dispatches `thread.create`, and runs setup before the durable NTBS record is written ([`processor.ts:697–713`](../../apps/server/src/ntbs/processor.ts#L697-L713)). A process exit after successful thread creation but before `save` leaves a thread/worktree that redelivery cannot discover, so redelivery creates another. This may be acceptable at-least-once behavior for the first version, but it should be recorded explicitly as a chosen crash window. + +**B7. Serial event handling creates head-of-line blocking.** + +[`Stream.runForEach`](../../apps/server/src/ntbs/processor.ts#L730-L741) handles session events sequentially, and one event can perform adapter reads plus a remote response post before the next event is consumed ([`processor.ts:355–410`](../../apps/server/src/ntbs/processor.ts#L355-L410)). One slow Discord/Jira call therefore delays all other outcomes for the same adapter. This is reasonable for initial volumes; add a comment that serialization is intentional, then introduce bounded per-event concurrency only if measurements justify it. + +**B8. Failed final-response delivery waits for another event or restart.** + +If terminal-event handling fails while searching for, posting, or recording the response, the event consumer logs the error and continues ([`processor.ts:325–349`](../../apps/server/src/ntbs/processor.ts#L325-L349), [`processor.ts:730–740`](../../apps/server/src/ntbs/processor.ts#L730-L740)). The T3 event stream does not replay that event, so another relevant session event or startup recovery is required before the processor retries. A small bounded retry around terminal-event reconciliation would close this gap. `findMatchingResponseMessage` already protects the post-succeeded/save-failed retry window from an ordinary duplicate ([`processor.ts:330–346`](../../apps/server/src/ntbs/processor.ts#L330-L346)). + +--- + +## Reviewed and deliberately not flagged + +- `ensureUniqueOutcome` and its per-message semaphore correctly serialize the startup-recovery/live-event race and clean up after the response is recorded ([`processor.ts:149–205`](../../apps/server/src/ntbs/processor.ts#L149-L205)). +- Subscribing before recovery is the right ordering for a hot event stream ([`processor.ts:768–773`](../../apps/server/src/ntbs/processor.ts#L768-L773)). +- `resolveWorktreeBase` has sensible fetch and ref-resolution fallbacks ([`processor.ts:412–470`](../../apps/server/src/ntbs/processor.ts#L412-L470)). +- Worktree cleanup on `thread.create` failure, including the documented temporary-branch leak, is deliberate ([`processor.ts:538–588`](../../apps/server/src/ntbs/processor.ts#L538-L588)). +- Consulting `findMatchingResponseMessage` on every response attempt is the idempotency net for the post-then-crash window and belongs in the common path ([`processor.ts:318–349`](../../apps/server/src/ntbs/processor.ts#L318-L349)). diff --git a/docs/planning/ntbs-processor.cod-review.md b/docs/planning/ntbs-processor.cod-review.md new file mode 100644 index 000000000000..528c3c172c21 --- /dev/null +++ b/docs/planning/ntbs-processor.cod-review.md @@ -0,0 +1,124 @@ +# NTBS processor review + +**Status:** Reconciled with the monitor-free processor on 2026-08-15. Addressed findings have been removed, so numbering gaps are intentional. + +**Scope:** The six files in [`apps/server/src/ntbs`](../../apps/server/src/ntbs/), their direct code references, and the orchestration/persistence behavior on which the processor relies. Other planning documents were not used as input. + +The implementation has a sound core: one opaque external-request locator, one fresh T3 thread, an exact user-message ID for finding the corresponding turn, and a two-state adapter record. Completion now has one live owner—the `thread.session-set` event listener—while startup recovery only starts missing turns or reconciles outcomes that finished while the processor was down. The main remaining refinements are durability and making the small contracts say exactly what the processor assumes. + +At present, no production code outside the NTBS directory constructs an adapter or processor, so the component remains inert until runtime wiring is added. + +## 1. Simplifications, naming, and contracts + +### S2. Replace generic storage operations with explicit state transitions + +[`NTBSAdapter.save`](../../apps/server/src/ntbs/adapter.ts#L32-L36) can write either lifecycle variant without stating transition, uniqueness, or upsert semantics. The processor separately checks [`findByRequest`](../../apps/server/src/ntbs/processor.ts#L687-L695), creates resources, and saves afterward. That broad API leaves the important guarantees implicit. + +A plainer repository contract would expose intent: + +- `claimRequest(request)` atomically inserts the external request and reports whether this caller claimed it; +- `attachThread(requestUri, threadId, userMessageId)` records the created T3 resources; +- `findByThreadId(threadId)` returns `null` rather than introducing a second absence convention through `ThreadNotFound`; +- `listPendingResponses()` replaces `loadThreadsAwaitingResponse`; +- `markResponded(threadId, responseMessageId)` is the only terminal transition. + +This introduces a small durable claimed/provisioning state, but removes `inFlightRequests` as a correctness boundary, prevents backwards writes such as `ResponsePosted -> ThreadCreated`, and makes adapter conformance testable. The existing [`JiraDeliveryStore.claim`](../../apps/server/src/jira/JiraDeliveryStore.ts#L66-L66) is a nearby example of atomic admission before side effects. + +The outbound half could likewise be one adapter operation such as `postResponseOnce(record, response)`, with a documented stable platform marker or idempotency key. The current [`findMatchingResponseMessage` then `postResponse`](../../apps/server/src/ntbs/processor.ts#L325-L338) makes the processor understand an adapter recovery protocol without making that pair atomic. + +### S3. Remove the processor tag factory until it has a production consumer + +[`makeNTBSProcessorTag`](../../apps/server/src/ntbs/processor.ts#L96) is used only by the two test harnesses ([`processor.test.ts:45`](../../apps/server/src/ntbs/processor.test.ts#L45), [`processor2.test.ts:159`](../../apps/server/src/ntbs/processor2.test.ts#L159)). The factory already returns the processor service value, so the additional tag factory can wait until production wiring demonstrates a need. `makeNTBSAdapterTag` remains useful if multiple adapter-specific processor layers will be built. + +### S4. Use record-oriented, plain names + +The current names mix events, lifecycle language, and stored adapter state. These values are records, and the processor assumes one external request per fresh T3 thread. + +| Current | Plainer option | Reason | +| ------------------------------------------------------------------- | -------------------------------------- | ------------------------------------------------------------------------------------------------ | +| [`NTBSLifecycle`](../../apps/server/src/ntbs/lifecycle.ts#L65) | `NTBSRequestRecord` | It is the adapter's current stored record, not a process. | +| [`ThreadEvent`](../../apps/server/src/ntbs/lifecycle.ts#L39-L50) | `ThreadRequestRecord` or no base alias | Nothing emits it as an event. | +| [`ThreadCreated`](../../apps/server/src/ntbs/lifecycle.ts#L52-L58) | `PendingResponse` | The processor cares that this record still needs a response. | +| [`ResponsePosted`](../../apps/server/src/ntbs/lifecycle.ts#L60-L63) | `RespondedRequest` | Names the terminal request state. | +| [`t3Data`](../../apps/server/src/ntbs/lifecycle.ts#L40-L49) | `thread` | `record.thread.threadId` and `record.thread.userMessageId` state the contents directly. | +| [`T3Context`](../../apps/server/src/ntbs/processor.ts#L48-L62) | `ThreadTarget` | It contains only the project and base ref used to create a thread. | +| [`snapshot`](../../apps/server/src/ntbs/lifecycle.ts#L26-L31) | `prompt` or `capturedText` | The value is sent verbatim as the first user message; “snapshot” does not say what was captured. | + +The recent removal of generic platform data is a good simplification and should not be reversed. Keeping one opaque, adapter-owned URI is easier to persist and recover. `sourceUri` could become `requestUri` to emphasize identity and addressability, but that rename is optional; the more important change is to validate it as non-empty. + +### S5. Make the input contract executable + +[`NTBSInput`](../../apps/server/src/ntbs/lifecycle.ts#L3-L37) is a plain TypeScript type whose strongest requirements exist only in comments. `sourceUri` may be empty, `snapshot` may be blank or exceed 120,000 characters, and the attachment array may exceed the provider limit of eight. The orchestration command accepts the values, while tighter provider validation occurs later, after resources and a lifecycle record can already exist. + +Define an Effect schema for the inbound boundary and reuse [`PROVIDER_SEND_TURN_MAX_INPUT_CHARS` and `PROVIDER_SEND_TURN_MAX_ATTACHMENTS`](../../packages/contracts/src/orchestration.ts#L146-L147) together with [`ChatAttachment`](../../packages/contracts/src/orchestration.ts#L181-L182). Decode before claiming or creating resources. This reduces prose that can drift and gives every adapter one executable contract. + +### S6. Consolidate the transitional test suite + +The directory currently carries two harnesses and two processor test files: + +- [`processor.test.ts`](../../apps/server/src/ntbs/processor.test.ts#L70-L85) describes a full happy path and missing-turn recovery, but its only test merely calls `process` without assertions ([`processor.test.ts:156–165`](../../apps/server/src/ntbs/processor.test.ts#L156-L165)). +- [`processor2.test.ts`](../../apps/server/src/ntbs/processor2.test.ts#L27-L171) is the more coherent layer harness and should become the sole `processor.test.ts`. +- Most of [`test-helpers.ts`](../../apps/server/src/ntbs/test-helpers.ts#L75) is an unfinished second copy of that harness and is not exported or used. +- [`createGitLayerMock`](../../apps/server/src/ntbs/test-helpers.ts#L34-L42) returns `input.refName` as the created worktree branch rather than `input.newRefName`, so a command assertion would observe the base commit as the thread branch. + +Delete the no-assertion test and unused helper harness, rename `processor2.test.ts`, and grow that one harness around state transitions. The two files currently contain four tests, but only three assert behavior and none covers a complete request-to-response lifecycle. + +## 2. Bugs, edge cases, and race conditions + +### B1. Blocking before integration: no production code constructs or runs NTBS + +The public entry points are [`makeNTBSProcessor`](../../apps/server/src/ntbs/processor.ts#L125-L133), [`makeNTBSAdapterTag`](../../apps/server/src/ntbs/adapter.ts#L95), and [`NTBSProcessor.run`](../../apps/server/src/ntbs/processor.ts#L69-L94), but their only consumers are the NTBS tests. There is no production adapter implementation. Consequently neither request processing nor startup recovery can execute. Treat this as integration status rather than an algorithm bug, but it is the first readiness item. + +### B2. High: inbound deduplication is a check-then-act race + +[`inFlightRequests`](../../apps/server/src/ntbs/processor.ts#L476-L480) protects only one processor instance. After that local check, [`findByRequest`](../../apps/server/src/ntbs/processor.ts#L687-L695) and resource creation are separate effects. Two processes, two processor instances, or an overlapping restart can both observe no record and create a worktree/thread for the same `sourceUri`. The adapter recommends a natural unique key but does not require atomic insertion or define conflict behavior. + +Use the atomic `claimRequest` transition from S2 and enforce a unique key in adapter storage. The in-memory set may remain as a cheap duplicate suppressor, but it should not be the correctness boundary. + +### B3. High: the durable record is written after irreversible resources are created + +[`createT3Thread`](../../apps/server/src/ntbs/processor.ts#L493-L607) creates a worktree, dispatches `thread.create`, and runs setup before [`ThreadCreated` is saved](../../apps/server/src/ntbs/processor.ts#L697-L713). A process exit after successful dispatch, during setup, or before `save` leaves a real T3 thread/worktree with no request record. Redelivery sees no record and creates another. + +Claim and persist the request before provisioning. Record generated thread/message IDs as soon as they are chosen, then make provisioning/recovery resume from that record. Deterministic IDs derived from the claim are another option, but are not required if the transition is durable. + +### B4. High: a saved request can become dormant after turn-start failure + +The processor saves `ThreadCreated` and then dispatches `thread.turn.start` ([`processor.ts:702–716`](../../apps/server/src/ntbs/processor.ts#L702-L716)). If turn start fails or the processing fiber is interrupted after the save, the record remains pending. A redelivery finds any existing lifecycle state and immediately returns ([`processor.ts:687–695`](../../apps/server/src/ntbs/processor.ts#L687-L695)). Only startup recovery reconciles the record ([`processor.ts:609–655`](../../apps/server/src/ntbs/processor.ts#L609-L655)). + +Make `process` mean “ensure this request is processing”: when `findByRequest` returns a pending record, invoke the same idempotent reconciliation used at startup. Only a responded record should be an immediate no-op. + +### B5. High: response delivery has a retry gap and a cross-process duplicate race + +When response lookup, posting, or persistence fails, the event consumer logs the failure and continues ([`processor.ts:325–349`](../../apps/server/src/ntbs/processor.ts#L325-L349), [`processor.ts:730–740`](../../apps/server/src/ntbs/processor.ts#L730-L740)). With no later `thread.session-set` event, nothing retries until processor restart. + +Conversely, two processor instances can both call `findMatchingResponseMessage`, both receive `null`, and both post before either saves `ResponsePosted`. The user-message semaphore is process-local ([`processor.ts:149–205`](../../apps/server/src/ntbs/processor.ts#L149-L205)). The recovery lookup protects the post-succeeded/save-failed window only after one response is visible; it is not an atomic exactly-once guarantee, and the adapter contract does not explain how it distinguishes a final response from an acknowledgement ([`adapter.ts:66–75`](../../apps/server/src/ntbs/adapter.ts#L66-L75)). + +Use a durable response-delivery claim/outbox or an explicitly idempotent `postResponseOnce` adapter primitive. Add a bounded in-process retry; startup recovery should be the fallback rather than the normal retry mechanism. + +### B8. Medium: event processing is serial and includes remote adapter I/O + +[`Stream.runForEach`](../../apps/server/src/ntbs/processor.ts#L730-L741) processes domain events one at a time. A relevant event may perform adapter lookup, response search, response posting, and persistence before the next event is consumed ([`processor.ts:355–410`](../../apps/server/src/ntbs/processor.ts#L355-L410)). One slow or hung platform call therefore blocks outcomes for every other NTBS thread handled by that adapter and can grow the event backlog. + +If this matters at observed volumes, route relevant events to fibers with bounded concurrency while retaining per-request serialization at the outcome transition. + +### B9. Medium: unique requests have no resource bound + +The API explicitly accepts unlimited concurrent distinct requests ([`processor.ts:69–82`](../../apps/server/src/ntbs/processor.ts#L69-L82)). Each can fetch `origin`, create a worktree, run setup, and start a full-access provider turn. A webhook burst can exhaust disk, git subprocesses, or provider capacity even though duplicate URIs are suppressed. + +Put a configurable bound around active requests, ideally at a durable claim/queue boundary. At minimum, bound provisioning per project; concurrent fetches and worktree setup for the same repository provide little benefit. + +### B10. Medium: invalid input fails after side effects instead of at admission + +Because [`NTBSInput` invariants](../../apps/server/src/ntbs/lifecycle.ts#L3-L37) are not decoded, an empty URI can collapse unrelated requests onto one dedup key, and over-limit text or attachments can reach provider validation after resources and a pending record exist. Validate before the durable claim as described in S5 and return a stable rejected outcome rather than relying on a later provider error. + +### B11. Low: an exact-turn error can use another turn's error text + +The processor selects the turn by its recorded user-message ID, but for an errored turn it reads thread-wide `session.lastError` ([`processor.ts:263–309`](../../apps/server/src/ntbs/processor.ts#L263-L309)). If the thread later receives another turn, that text may describe the later session rather than the NTBS turn. Until errors are stored per turn, prefer generic failure text, or use `lastError` only when the selected turn is the current/latest turn. + +### B12. Confidence gap: critical transitions are untested + +The four current tests cover one no-assertion process call, unknown-event routing, durable redelivery deduplication, and ignoring an already-recorded response ([`processor.test.ts`](../../apps/server/src/ntbs/processor.test.ts), [`processor2.test.ts`](../../apps/server/src/ntbs/processor2.test.ts#L215-L281)). They do not cover a successful create/start/terminal-response lifecycle, missing-turn startup recovery, active-turn recovery, terminal-turn recovery, a response found remotely after local-save failure, concurrent recovery versus live completion, duplicate concurrent deliveries, or retry after turn-start failure. + +After consolidating the harness, cover those transitions with controllable deferred adapter calls. In particular, turn the existing missing-turn TODO into a test that proves recovery reuses the stored `userMessageId` and does not start a second turn for pending/running records ([`processor.test.ts:84–85`](../../apps/server/src/ntbs/processor.test.ts#L84-L85)). + +I specifically did not flag a projection/publication race: the orchestration engine applies projections in the same transaction before publishing each event to `streamDomainEvents`. I also did not treat best-effort setup-script failure or the documented temporary-branch leak as new NTBS bugs; both are explicit choices in the implementation ([`processor.ts:554–604`](../../apps/server/src/ntbs/processor.ts#L554-L604)). diff --git a/docs/planning/ntbs-questions.md b/docs/planning/ntbs-questions.md new file mode 100644 index 000000000000..f671219ba7de --- /dev/null +++ b/docs/planning/ntbs-questions.md @@ -0,0 +1,3 @@ +# NTBS open questions + +- Do we need separate t3gateway APIs for `planT3Work`, etc.? diff --git a/docs/planning/ntbs-todos.md b/docs/planning/ntbs-todos.md new file mode 100644 index 000000000000..bd578c7da5f4 --- /dev/null +++ b/docs/planning/ntbs-todos.md @@ -0,0 +1,39 @@ +# NTBS todos + +The exchange model, ports, processor, and T3 gateway are implemented under `apps/server/src/ntbs/`. This file tracks only what is still open. Findings referenced by id are in `review-01-09.md`. + +## Bound every retry (M2, M3) + +Per-state deadlines are implemented: every non-terminal state expires after its deadline. Backoff was left out on purpose. + +- [ ] `provisionThread`: T3 invariant rejection while the thread is absent is `FatalError`, mirroring `startTurn`. +- [ ] `getTurnStatus`: when no turn row carries our `userMessageId` but the session settled in `error`, answer `completed` with a failure reply from `session.lastError`. + +## Durable readiness marker (H3, H4) + +Provisioning today is worktree → `thread.create` with the final path → fire-and-forget setup script. `getThreadStatus` reports `present` from the thread shell alone, so a crash between `thread.create` and setup skips setup permanently, and a fatal cleanup leaves a thread pointing at a removed worktree. + +- [ ] Create the thread with `worktreePath: null`, ensure the worktree, run setup and wait, then dispatch `thread.meta.update` with the final path. +- [ ] Add `ProjectSetupScriptRunner.runForThreadAndWait` backed by `ProcessRunner`, with a timeout and bounded diagnostic output; failure or timeout is `RetryableError`. +- [ ] `getThreadStatus` reports `present` only with a non-null `worktreePath`. Fix the test that pins the opposite. +- [ ] On `FatalError` after `thread.create`, cleanup also dispatches `thread.delete`. +- [ ] Setup is at-least-once; scripts must be idempotent. + +## Other review items + +- [ ] H5: narrow `threadActivity` to session/turn lifecycle events; `Effect.timeout` on every adapter and gateway call. +- [ ] L3: worktree branch uses the full thread UUID with a non-temporary prefix so T3 does not rename it. + +## Tests + +- [ ] Sweeper via `TestClock`. +- [ ] One real-engine integration test for `startTurn` → `getTurnStatus`. +- [ ] Injectable failing repository in the processor harness. +- [ ] `startTurn` fatal → `ReplyPending`. +- [ ] Bound `awaitStoredTag` with a timeout. +- [ ] `ensureWorktree` error branches: `fs.exists`, `localStatus`, `removeWorktree` fallback, `listRefs`, "isRepo but wrong ref", stale locked registration. + +## Next + +- [ ] Jira port as the first real adapter, replacing the legacy bridge path. +- [ ] SQL `ExchangeRepository` with an index on `threadId` and unique constraints on both keys. diff --git a/docs/planning/ntbs.md b/docs/planning/ntbs.md new file mode 100644 index 000000000000..67b2f3f22981 --- /dev/null +++ b/docs/planning/ntbs.md @@ -0,0 +1,70 @@ +# Non-turn-based surfaces + +**Status:** exploratory planning + +## Overview + +T3 currently models interaction primarily as a conversation between one user and an agent. A user submits a message, the agent runs a turn, and T3 presents the resulting conversation and runtime state through clients that understand the full T3 model. + +Non-turn-based surfaces (NTBS) such as Discord, Jira, Teams, GitHub issues, and pull requests do not share those assumptions. They are independently owned collaboration systems where: + +- several people may interact with the same external object; +- messages, comments, and object state may be edited or deleted after T3 first observes them; +- objects may be closed, reopened, moved, locked, or otherwise changed outside T3; +- events may arrive late, more than once, or after T3 has been offline; +- the platform can render only a small part of the state and activity available in a native T3 client. + +The problem is to define how these surfaces participate in T3 without creating a second domain model beside T3's existing one. The existing T3 event log remains the source of truth for T3 state. NTBS support should reuse existing T3 commands, events, and state where possible, while platform adapters translate between T3 and each platform's native concepts. + +This requires a shared contract that answers several questions consistently across platforms: + +- how an external interaction is identified and related to its T3 threads; +- which T3 commands an adapter may issue in response to an external event; +- which T3 events and state an adapter may use to render a response on the external platform; +- how later edits, deletions, multiple participants, retries, and replay affect event handling and response rendering; +- what an adapter does when the external platform cannot represent a T3 event or response; + +The integration protocol should expose only the T3 commands and state needed by these adapters. Adapters should be able to obtain an initial state and then receive subsequent changes. Platform adapters should remain responsible for authentication, source-event translation, transport, and rendering—not for defining their own conversation semantics. + +## Scope of this document + +This document defines the protocol-level relationship between T3 and non-turn-based surfaces. It covers event processing and trigger rules, thread creation, interaction identity, lifecycle, client state, cursors, and adapter behavior. Detailed decisions may be developed in companion planning documents, but remain part of this document's scope. + +Implementation is out of scope for this planning stage. + +## Proposal: A triggering event creates a new thread + +Each event that matches a trigger creates a new T3 thread from the event and its captured source snapshot; the detailed trigger, processing, concurrency, and response-routing rules are defined in [ntbs-event-processing.md](./ntbs-event-processing.md). + +## Agreed decisions + +### Which external messages or state changes trigger an agent turn, and which are ignored or recorded without starting work? + +The platform-specific trigger forms, ignored events, thread creation, and response routing are defined in [ntbs-event-processing.md](./ntbs-event-processing.md). + +### What identifies the same external interaction for correlation and projection? + +- Jira: the issue key or immutable issue ID. Comments and replies are events within that issue. +- Discord: the thread ID. The thread is the interaction. +- GitHub: the repository and pull-request number. Issue comments, review comments, and replies are events within that pull request; the triggering comment and any diff context belong to the individual event. +- Teams: unresolved. The likely scope is the conversation or reply-chain ID, with each message as its own event. + +### When does the adapter capture the source snapshot relative to receiving a trigger and creating the T3 thread? + +The adapter captures the source snapshot while processing the trigger, before creating the T3 thread. The new thread uses that captured snapshot. + +### How does T3 prevent repeated delivery of the same source event from creating multiple threads? + +Each adapter derives an idempotency key from the platform’s source-event identity and version. The adapter stores that key with the T3 thread created for the event. If the same key is delivered again, the adapter reuses the existing record and does not create another thread. A later edit or distinct source event receives a different key and may create a new thread. The exact event identity, versioning, and retention rules are platform-specific and remain to be defined. + +### How are concurrent NTBS threads isolated without an event queue? + +Each NTBS-triggered T3 thread receives its own worktree and branch before provider execution begins. Threads from the same external interaction can therefore run concurrently without sharing a mutable checkout or requiring an event queue. + +### How are completion, failure, timeout, and cancellation reported for an external event? + +They use the same response destination as the triggering event. Normal completion returns the agent’s answer; failure, timeout, or cancellation returns a response that explicitly reports the outcome and, where available, its reason. These outcomes do not create a separate external lifecycle or target a different thread. + +### How does T3 associate a thread's outcome with the external event that created it, and where does the adapter post that outcome? + +Each source event has a unique event ID. T3 stores a correlation record linking that event ID to the T3 thread, user message or turn, and exact response destination. When the turn ends, the adapter uses that record to post the answer or outcome back to the originating source. diff --git a/docs/planning/processor-testing.md b/docs/planning/processor-testing.md new file mode 100644 index 000000000000..51e602df40c6 --- /dev/null +++ b/docs/planning/processor-testing.md @@ -0,0 +1,25 @@ +# Goal 1 - Happy path testing + +## Step 1 - it fetches and resolves the requested base ref + +We test this indirectly via `processor.process(request, {projectId, baseRef: "branchname" })`. + +For a new request, `process()` calls `createT3Thread`, which should: + +1. fetch `branchname` +2. resolve `branchname` against the remote tracking branch +3. pass the resolved commit SHA into `createWorktree` + +## Step 2 - it creates the isolated worktree + +## Step 3 - dispatches `thread.create` and then `thread.turn.start` + +## Step 4 - preserves the snapshot and attachments + +## Step 5 - saves `thread.created` with generated thread and message IDs. + +## Step 6 - runs the project setup script + +## Step 7 - posts the acknowledgement + +## Step 8 - does not duplicate work diff --git a/docs/planning/review-01-09.md b/docs/planning/review-01-09.md new file mode 100644 index 000000000000..eb2d470fd225 --- /dev/null +++ b/docs/planning/review-01-09.md @@ -0,0 +1,106 @@ +# NTBS adversarial review — 2026-09-01 + +**Scope:** `apps/server/src/ntbs/` (`exchange.ts`, `ExchangeRepository.ts`, `adapter.ts`, `t3gateway.ts`, `processor.ts`) and the four test files. Reviewed against the real T3 internals the gateway depends on (`ProjectionPipeline.ts`, `decider.ts`, `OrchestrationEngine.ts`, `ProjectSetupScriptRunner.ts`, git driver) and against `ntbs-architecture.md` / `ntbs-todos.md`. + +**Method:** one manual pass plus three independent adversarial reviewers with separate lenses (processor/exchange soundness, gateway vs T3 internals, test-suite adequacy). Findings below are deduplicated and ranked. Every claim was verified against source; line numbers are as of commit `c80ee1bb8`. + +## Verdict in one paragraph + +The model and the orchestration loop are sound. The five-state exchange, the pure deciders, observe-before-act, the per-source lock, the claim idempotency, and the recovery/sweep loop all hold up under interruption and crash analysis, and the reviewers found no way to create two threads or post two replies for one request. The problems are at the seams with T3: three assumptions the gateway makes about T3 internals are false in realistic failure modes, and each one turns a failure that should end in a failure reply into either silence or an infinite retry. Those must be fixed before the first real adapter, because every one of them is triggered by something a user will do (delete a thread, name a wrong branch, have a provider that fails to launch). The test suite is thorough for the happy and transient-failure paths of the processor and for gateway classification, but it never exercises the sweeper, never tests against the real engine, and in one place pins the opposite of the documented contract. + +## Part 1 — Implementation soundness + +### HIGH + +**H2. A rejected request produces silence, and the documented contract says otherwise.** +`processor.ts:371-373` maps `FatalError` from `planCoordinates` (branch not on origin, project missing, no `origin` remote) to `NTBSProcessorError` with nothing persisted. `t3gateway.ts:72` claims the processor "converts that into a reply-pending failure"; it cannot, because `ExchangeBase` requires the coordinates that just failed. The user who typed a wrong branch name never hears back; the webhook errors, the platform redelivers, and each redelivery does a full `git fetch`. +Fix: decide who owns this reply. Either a typed `NTBSRequestRejected` error the inbound code must render to the platform, or a `RequestRejected` state with `t3: null` that flows through normal delivery. Fix the comment on `t3gateway.ts:72` either way. + +**H3. Crash after `thread.create` but before setup scripts skips setup permanently; fatal cleanup orphans a thread.** +Provisioning order is worktree → `thread.create` with the final `worktreePath` → scripts (`t3gateway.ts:690-765`). `getThreadStatus` (`:480-491`) reports `present` from the shell alone, so recovery after a crash in that window decides `record-thread-created` and never runs setup. On a `FatalError` after `thread.create`, `tapError` (`:767-773`) removes the worktree but leaves the T3 thread pointing at the deleted path; the provider then spawns with a missing cwd and errors, which the gateway now reports as a failure reply (see "Resolved"). `ntbs-todos.md` ("Settled gateway contracts") specified `worktreePath: null` on create, a blocking setup, then `thread.meta.update` as the durable readiness marker. That design was not implemented and the comment on `t3gateway.ts:163` ("each skipped if already done") is false for setup. +Fix: implement the documented readiness marker, or at minimum dispatch `thread.delete` on fatal cleanup. + +**H4. Setup scripts do not block and their failure is never observed.** +`ProjectSetupScriptRunner.runForThread` (`ProjectSetupScriptRunner.ts:141-182`) opens a terminal, writes `command\r`, returns `started`. The comment at `t3gateway.ts:749-752` ("Script failures are … fatal") is false: only terminal open/write failures are errors. `startTurn` can run while `pnpm install` is still executing. `ntbs-todos.md` required `runForThreadAndWait`; it was never added. +Fix: add the blocking runner with a timeout, or rewrite the comment and accept the race explicitly. + +**H5. Head-of-line blocking on the activity loop, fed by an unbounded firehose.** +`threadActivity` (`t3gateway.ts:638-644`) forwards every thread event system-wide, token deltas included, from `PubSub.unbounded` (`OrchestrationEngine.ts:92`). `Stream.runForEach` (`processor.ts:393`) processes pings strictly sequentially, each taking the per-source lock and running port calls with no timeout anywhere. One `postReply` hanging on Jira for 60 s parks every other exchange's completion ping behind it while the pubsub buffer grows without bound. A ping on a `request-claimed` exchange runs `provisionThread` (git fetch, worktree, scripts) inline in the loop. +Fix: (a) narrow the filter to session/turn lifecycle events; (b) make pings non-blocking (record "dirty" and let the holder re-drive, or `withPermitsIfAvailable`); (c) `Effect.timeout` on every adapter and gateway call, classified retryable. + +### MEDIUM + +**M1. `process` can fail after the claim and the caller cannot tell.** `processor.ts:375-376`: `persist(claimed)` succeeds, `advanceExchange` fails transiently, `process` returns an error. The doc says it "returns once the exchange is claimed". A webhook handler will post its own error while the sweeper later posts the real reply. Also heavy provisioning runs inside the webhook request fiber. Fix: after the claim, log-and-succeed (as `run` does) or fork the advance into `run`'s scope. + +**M2. `provisionThread` retries forever when the thread id was deleted.** `t3gateway.ts:729-746` swallows every dispatch failure into `RetryableError` when the thread is absent. `requireThreadAbsent` (`commandInvariants.ts:147-165`) rejects ids in `deletedThreadIds`, so a user deleting the NTBS thread mid-provisioning makes every pass fail with an invariant error that is retried each minute. `startTurn` (`:617-629`) classifies the same error as fatal. Fix: mirror `startTurn`: invariant error and thread absent → `FatalError`. + +**M3. No deadline for any non-terminal state; the sweeper retries forever with no backoff.** A turn T3 calls `active` forever (agent hung, token limit), a provisioning that fails transiently every minute (repeated `git fetch`), a platform that is down for a day: all stay non-terminal indefinitely and `findNonTerminalExchanges` grows monotonically. `ntbs-architecture.md` says the heuristics are undefined; the sweeper shipped anyway. Fix: add `claimedAt`/`updatedAt` and an attempt counter to `ExchangeBase`; per-state deadlines in the decider (`active` past N → `thread.turn.interrupt`, which yields the existing cancellation path); exponential backoff. + +**M4. Stored replies carry raw `cause: unknown` and leak internal text.** `processor.ts:137-144` stores `FatalError.cause` verbatim: error instances, git results, possibly cyclic. A real repository will JSON-encode it; a throw there fails `persist` and strands the exchange. `text: failure.reason` is what gets posted to the platform ("T3 rejected the turn start for thread "). Gateway replies write `cause: null` contrary to the structured causes in `ntbs-todos.md`. Fix: type `cause` as a JSON-safe schema, convert at the boundary, and separate user-facing text from diagnostics. + +**M5. Nothing is wired.** No SQL `ExchangeRepository`, no real adapter, no consumer of `makeNTBSProcessor` outside tests. The "durable" in the design is the in-memory HashMap today. Not a defect, but it bounds what this review can say: the persistence assumptions in M4 and the index needs in L2 are untested. + +### LOW + +**L1. A defect in the activity subscription kills it silently.** `processor.ts:395-401` catches typed errors only; a defect in `findByThreadId` ends the forked fiber and `run` keeps sweeping, so failures show as one-minute latency with no log. Fix: `Effect.catchCause` + log, and restart the subscription. + +**L2. In-memory repository is O(n) per event** (`ExchangeRepository.ts:52-62`, `82-96`) and, per H5, that is every token delta. Fine for tests; the SQL repository needs an index on `threadId` and unique constraints on both keys. + +**L3. Worktree branch token is 8 hex chars and T3 renames it on the first turn.** `buildTemporaryWorktreeBranchName` (`packages/shared/src/git.ts:95-105`) slices to 8 chars, and `ProviderCommandReactor.ts:928-960` renames temporary branches. The comment at `t3gateway.ts:464` ("a stray branch points back at its thread") is false, and a collision would make `ensureWorktree` adopt another thread's branch. Use the full UUID with a non-temporary prefix. + +**L4. `resolveRemoteTrackingCommit` fatal classification is broader than "branch missing".** `t3gateway.ts:306-318`: any non-zero git exit (index.lock, corrupt ref) becomes "Branch does not exist on origin". Acceptable, but say so in the comment. + +**L5. `runtimeMode: "full-access"` for externally-triggered work.** It is already the engine default and bypasses nothing extra, but it is the one place a policy hook for untrusted input would go. Note it; do not solve it now. + +### Verified sound + +- `withExchangeLock` under interruption: waiter cleanup, `callers` bookkeeping, and the `get(sourceUri) === lock` guard are correct; no deadlock path exists. Wake-up is not FIFO but every caller is idempotent. +- Claim idempotency, concurrent-delivery serialization, forward-only constructors, `ReplyRejected` → `Undeliverable`, ack posted once after `ThreadCreated` is persisted. +- Read-your-writes at activity time holds: the engine publishes to the pubsub strictly after the SQL transaction that appends events and projects (`OrchestrationEngine.ts:174-218`). `pendingMessageId` survives completed/error/interrupted transitions on the happy path via the `...existingTurn.value` spreads. +- Sweeper never overlaps itself; sweep and activity on the same exchange serialize under the lock. +- `createWorktree` argument mapping, `deriveWorktreePath`, `localStatus().refName`, `listRefs` substring semantics compensated by the exact `some(...)` check. + +## Part 2 — Test suite + +### Gaps that matter + +1. **The sweeper has never run in a test.** `it.effect` provides a `TestClock`, so `Effect.delay("1 minute")` never fires and every `run` test interrupts first. The design's backstop for missed pings, and a sweep racing an activity ping on the same source, is one `TestClock.adjust("1 minute")` away. +2. **No integration test against the real `OrchestrationEngine` + sqlite.** The whole design rests on dispatch being synchronous with projection; `t3gateway.ts:184-186` says "no test in this package would notice" if that broke. One test that dispatches `thread.turn.start` through the real engine and reads `getTurnStatus` would pin it, and would have caught the provider-failure retry loop fixed on 2026-09-02 (see "Resolved"). +3. **`getThreadStatus` test pins the opposite of the documented contract.** `t3gateway.test.ts:955` asserts `present` with a mock whose `worktreePath` is `null`; `ntbs-todos.md` says that must be `missing`. Either the doc or the test is wrong, and today the code follows the test (H3). +4. **Uncovered processor branches:** `startTurn` `FatalError` → `ReplyPending` (`processor.ts:241-246`); `persist` failure (reachable via two requests planned onto one `threadId`); `findByThreadId` / `findNonTerminalExchanges` failures (harness hard-wires the in-memory repo, so no failing repository can be injected); recovery racing activity for the same exchange; `findPostedReply` transient failure followed by a retry that repeats discovery; a burst of pings during an active turn proving no duplicate `postReply`. +5. **Uncovered gateway branches:** `provisionThread` dispatch payload and `deriveWorktreePath` output are never asserted; `ensureWorktree` error branches (`fs.exists`, `localStatus`, `removeWorktree` → `fs.remove` fallback, `listRefs`, "isRepo but wrong ref", the `locked` stale-registration variant); `getProject` failing inside `provisionThread`; `getThreadShellById` failing inside the dispatch-failure re-check; no test asserts `.cause` is preserved; `startTurn` payload test (`t3gateway.test.ts:1827`) omits `type`, so a `thread.create` carrying a message would pass; `threadActivity` (`:1939`) uses a `session-set` event, which satisfies both the shipped filter and the narrower documented one, so it cannot tell them apart. + +### Weak tests + +- `processor.test.ts:379, 441, 586, 658`: assert `exit._tag === "Failure"` without checking which step failed. +- `processor.test.ts:300-350`: "no further calls" guarded by a single `Effect.yieldNow`; passes if the wrong call is one scheduler tick late. Same pattern at `:508, 577, 649, 723, 780, 796, 1025, 1241, 1304`. Works because every mock is synchronous; the first `Effect.sleep` in the processor path makes these vacuous. +- `processor.test.ts:1209-1268`: the run fiber is interrupted right after the first post attempt, so "stays ReplyPending" holds whether or not the `AdapterError` was observed. +- `t3gateway.test.ts:927-980`: obtains coordinates via `planCoordinates` (needless coupling) and never asserts `getThreadShellById` received the stored thread id. +- `t3gateway.test.ts:624`: `threadId: expect.any(String)` although the UUID mock is deterministic. + +### Harness risks + +- `awaitStoredTag` (`processor.test.ts:196-207`) is an unbounded `yieldNow` spin. On regression it never returns and the only signal is vitest's 5 s timeout with no diagnostic. Wrap in `Effect.timeout`, or signal a `Deferred` from a repository wrapper's `upsert`. +- `withProcessor` interrupts the `run` fiber after the expects, so a failing assertion leaks the fiber. +- Recovery order over the HashMap is nondeterministic; tests correctly filter per source today, but `:1394` reads `calls[0]` and becomes order-sensitive the moment a second exchange is stored before `run`. + +### What is well covered + +Exchange deciders and transitions (exhaustively enumerated); repository conflict and atomicity rules; processor claim/dedup/lock semantics including interruption; transient-failure retries for provision, turn start, reply post, turn status; `Undeliverable`; discovered-reply short circuit; per-exchange isolation in recovery and activity; `planCoordinates` and `getTurnStatus` classification including error/interrupted replies. + +### Stale checklist + +`ntbs-todos.md` marks "findPostedReply retry" and "recovery racing activity" as done; neither test exists. Its gateway checklist still describes `T3Rejected`/`T3GatewayError`, `runForThreadAndWait`, `thread.meta.update` and path-based readiness, none of which match the shipped gateway. Treat it as an unreconciled design doc, not a coverage record, and reconcile it in one of the two directions. + +## Resolved + +- **2026-09-02 — provider fails to launch → infinite turn re-dispatch.** T3 deletes the pending turn-start row when a session settles without adopting it, so `getTurnStatus` answered `missing` and the sweeper restarted the turn every minute. `getTurnStatus` now loads the thread detail when no turn matches: our user message present plus session status `error` becomes a failure reply with `session.lastError`; a missing thread becomes a failure reply; otherwise still `missing`. Verified against `ProviderRuntimeIngestion.ts:1676-1700` (both `session.state.changed(error)` and `turn.completed(failed)` map to `error`; `session.exited` maps to `stopped` and correctly re-dispatches). Pinned by three new tests in `t3gateway.test.ts`. + +## Recommended order + +1. M2 + M3 together: they are one problem, "T3 said no or nothing, and NTBS retries forever". Add an attempt counter and timestamps to `ExchangeBase` and make invariant rejections fatal in `provisionThread`. +2. H3 + H4: implement the documented readiness marker (`worktreePath: null` → blocking setup → `thread.meta.update`) and make `getThreadStatus` honour it. Fix the `getThreadStatus` test to match. +3. H2 + M1: define who posts the reply for a request T3 refuses before a claim exists, and make `process` return once claimed. +4. H5: narrow the activity filter and add timeouts. Non-blocking pings can wait until the platform adapter exists and shows real latency. +5. Tests: sweeper via `TestClock`, one real-engine integration test, injectable failing repository, `startTurn` fatal path, and bound `awaitStoredTag`. +6. M4 before the SQL repository lands, so `cause` never hits the database unserialized.