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..0ca6891b1488 --- /dev/null +++ b/apps/server/src/ntbs/ExchangeRepository.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit } from "effect"; +import { MessageId, ProjectId, ThreadId } from "@t3tools/contracts"; +import { + ExchangeRepositoryError, + ExchangeRepository, + inMemoryExchangeRepository, +} from "./ExchangeRepository.ts"; +import { + makeRequestClaimed, + toReplyPending, + toReplyPosted, + toThreadCreated, + toUndeliverable, +} from "./exchange.ts"; + +const makeExchange = (sourceUri: string, threadId: string) => + makeRequestClaimed( + { + sourceUri, + snapshot: "request", + attachments: [], + }, + { + projectId: ProjectId.make("project"), + startBranchName: "main", + startCommitSha: "start-commit-sha", + threadId: ThreadId.make(threadId), + userMessageId: MessageId.make(`message-${threadId}`), + worktreeBranchName: `branch-${threadId}`, + }, + ); + +describe("inMemoryExchangeRepository", () => { + it.layer(inMemoryExchangeRepository)((it) => { + it.effect("allows the same sourceUri to replace its state", () => + Effect.gen(function* () { + const repository = yield* ExchangeRepository; + const claimed = makeExchange("test://request/1", "thread-1"); + const threadCreated = toThreadCreated(claimed); + + yield* repository.upsert(claimed); + yield* repository.upsert(threadCreated); + + expect(yield* repository.findBySourceUri(claimed.sourceUri)).toEqual(threadCreated); + }), + ); + }); + + 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("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 claimed = makeExchange("test://request/claimed", "thread-claimed"); + const threadCreated = toThreadCreated( + makeExchange("test://request/thread-created", "thread-created"), + ); + const replyPending = toReplyPending( + toThreadCreated(makeExchange("test://request/reply-pending", "thread-reply-pending")), + { type: "answer", text: "pending reply" }, + ); + const replyPosted = toReplyPosted( + toReplyPending( + toThreadCreated(makeExchange("test://request/reply-posted", "thread-reply-posted")), + { type: "answer", text: "posted reply" }, + ), + "test://reply/posted", + ); + const undeliverable = toUndeliverable( + toReplyPending( + toThreadCreated(makeExchange("test://request/undeliverable", "thread-undeliverable")), + { type: "failure", text: "undeliverable reply", cause: "undeliverable te dico" }, + ), + { message: "platform rejected the reply" }, + ); + + yield* Effect.forEach( + [claimed, threadCreated, replyPending, replyPosted, undeliverable], + repository.upsert, + ); + + const results = yield* repository.findNonTerminalExchanges; + + expect(results).toHaveLength(3); + expect(results).toEqual(expect.arrayContaining([claimed, 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..ed0b6e43a241 --- /dev/null +++ b/apps/server/src/ntbs/ExchangeRepository.ts @@ -0,0 +1,111 @@ +/* + * 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 } from "effect"; +import { isNonTerminal, 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`. */ + 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(), + ); + + const upsert = Effect.fn("ExchangeRepository.upsert")(function* (exchange: Exchange) { + // we return conflicting source Uri as the first argument + // in case we find that the same threadId belongs already to a different sourceUri + const conflictingSourceUri = yield* Ref.modify(exchanges, (map) => { + const conflict = HashMap.findFirst( + map, + (existing, sourceUri) => + sourceUri !== exchange.sourceUri && existing.t3.threadId === exchange.t3.threadId, + ); + + return isSome(conflict) + ? [conflict.value[0], map] + : [null, HashMap.set(map, exchange.sourceUri, exchange)]; + }); + + if (conflictingSourceUri !== null) { + return yield* new ExchangeRepositoryError({ + reason: `Thread ${exchange.t3.threadId} already belongs to exchange ${conflictingSourceUri}`, + cause: { + threadId: exchange.t3.threadId, + existingSourceUri: conflictingSourceUri, + incomingSourceUri: exchange.sourceUri, + }, + }); + } + }); + + 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) => val.t3.threadId === 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..de8a5ac9dd17 --- /dev/null +++ b/apps/server/src/ntbs/exchange.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, it } from "@effect/vitest"; +import { + fromReplyPending, + fromRequestClaimed, + fromThreadCreated, + makeRequestClaimed, + toReplyPending, + toReplyPosted, + toThreadCreated, + toUndeliverable, + type ExchangeBase, + type ReplyPosted, + type Request, + type RequestClaimed, + type WorkCoordinates, +} from "./exchange.ts"; +import { MessageId, ProjectId, ThreadId } from "@t3tools/contracts"; + +const request = { + sourceUri: "test://exchange/test", + snapshot: "You need to imagine some text here", + attachments: [], +} satisfies Request; + +const coordinates = { + projectId: ProjectId.make("projectId"), + startBranchName: "startBranchName", + startCommitSha: "startCommitSha", + threadId: ThreadId.make("threadId"), + userMessageId: MessageId.make("messageId"), + worktreeBranchName: "worktreeBranchName", +} satisfies WorkCoordinates; + +const exchangeBase = { + ...request, + t3: coordinates, +} satisfies ExchangeBase; + +describe("RequestClaimed", () => { + const claimed = makeRequestClaimed(request, coordinates); + + it("makeRequestClaimed tags the base unchanged", () => { + expect(claimed).toEqual({ ...exchangeBase, tag: "request-claimed" }); + }); + + // if thread is missing we provision the thread + // if its present we record it's been created + it.each([ + [{ thread: "missing" }, { type: "provision-thread" }], + [{ thread: "present" }, { type: "record-thread-created" }], + ] as const)("decides %j -> %j", (context, expected) => { + expect(fromRequestClaimed(claimed, context)).toEqual(expected); + }); + + it("toThreadCreated retags and carries every claim field forward", () => { + expect(toThreadCreated(claimed)).toEqual({ ...exchangeBase, tag: "thread-created" }); + }); + + it("provisioning failure jumps ahead with the reply stored verbatim", () => { + const reply = { + type: "failure", + text: "provisioning rejected", + cause: "project not found", + } as const; + expect(toReplyPending(claimed, reply)).toEqual({ + ...exchangeBase, + tag: "reply-pending", + reply, + }); + }); +}); + +describe("ThreadCreated", () => { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + const answer = { type: "answer", text: "The turn's final answer" } as const; + + // missing turn -> start it; active turn -> wait; completed turn -> record its reply + it.each([ + [{ turn: "missing" }, { type: "start-turn" }], + [{ turn: "active" }, { type: "wait" }], + [ + { turn: "completed", reply: answer }, + { type: "record-reply-pending", reply: answer }, + ], + ] as const)("decides %j -> %j", (context, expected) => { + expect(fromThreadCreated(threadCreated, context)).toEqual(expected); + }); + + it("completed turn's reply lands in ReplyPending verbatim", () => { + expect(toReplyPending(threadCreated, answer)).toEqual({ + ...exchangeBase, + tag: "reply-pending", + reply: answer, + }); + }); +}); + +describe("ReplyPending", () => { + const reply = { type: "answer", text: "The turn's final answer" } as const; + const replyPending = toReplyPending( + toThreadCreated(makeRequestClaimed(request, coordinates)), + reply, + ); + + // missing platform reply -> post it; posted -> record its message id + it.each([ + [{ platformReply: "missing" }, { type: "post-reply" }], + [ + { platformReply: "posted", replySourceUri: "test://exchange/reply" }, + { type: "record-reply-posted", replySourceUri: "test://exchange/reply" }, + ], + ] as const)("decides %j -> %j", (context, expected) => { + expect(fromReplyPending(replyPending, context)).toEqual(expected); + }); + + it("accepted delivery lands in ReplyPosted with the platform message id", () => { + expect(toReplyPosted(replyPending, "test://exchange/reply")).toEqual({ + ...exchangeBase, + tag: "reply-posted", + reply, + 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)).toEqual({ + ...exchangeBase, + tag: "undeliverable", + reply, + cause, + }); + }); +}); + +/* +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, claimed: RequestClaimed) => { + // @ts-expect-error terminal states cannot re-enter thread creation + toThreadCreated(posted); + // @ts-expect-error a bare claim cannot record a posted reply + toReplyPosted(claimed, "reply://msg"); + // @ts-expect-error Undeliverable is entered only from ReplyPending + toUndeliverable(claimed, { message: "cause" }); +}; diff --git a/apps/server/src/ntbs/exchange.ts b/apps/server/src/ntbs/exchange.ts new file mode 100644 index 000000000000..83756da79945 --- /dev/null +++ b/apps/server/src/ntbs/exchange.ts @@ -0,0 +1,317 @@ +import type { ChatAttachment, MessageId, ProjectId, ThreadId } from "@t3tools/contracts"; + +/* +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; +}; + +export type ReplyFailure = { + readonly type: "failure"; + readonly text: string; + readonly cause: unknown; +}; + +export type ReplyCancellation = { + readonly type: "cancellation"; + readonly text: string; + readonly cause: unknown; +}; + +export type Reply = + | { + readonly type: "answer"; + readonly text: string; + } + | ReplyFailure + | ReplyCancellation; + +export type UndeliverableCause = { + readonly message: 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 request was claimed. + * We keep the same SHA across retries so the request always runs against the + * code selected when it was claimed, 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 RequestClaimed; 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; +}; + +/** + * The data every exchange carries, whatever state it has reached. + */ +export type ExchangeBase = Request & { + readonly t3: WorkCoordinates; +}; + +/** + * The platform inbound code (Jira Webhook e.g.) admitted the request, + * trigger and actor checks passed, and the processor records the request + * as being claimed by the system. + * + * From here, the processor alone drives the exchange to a terminal state. + */ +export type RequestClaimed = ExchangeBase & { + readonly tag: "request-claimed"; +}; + +/** + * 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"; +}; + +/** + * T3 reached a terminal outcome; the exact reply payload is stored + * verbatim so every posting attempt sends the same content. + * This state may also follow `RequestClaimed` directly after a definitive provisioning failure, so it and later states do not imply the thread existed. + * Reply delivery needs only `sourceUri`. + */ +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 = RequestClaimed | 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 claim 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-claimed": + case "thread-created": + case "reply-pending": + return false; + } +}; + +export const isNonTerminal = (state: Exchange): state is NonTerminalExchange => !isTerminal(state); + +export const makeRequestClaimed = ( + request: Request, + coordinates: WorkCoordinates, +): RequestClaimed => ({ + ...request, + t3: coordinates, + tag: "request-claimed", +}); + +/* +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 +*/ + +export type RequestClaimedContext = { 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; + }; + +export const toThreadCreated = (state: RequestClaimed): ThreadCreated => ({ + ...state, + tag: "thread-created", +}); + +export const toReplyPending = ( + state: RequestClaimed | ThreadCreated, + reply: Reply, +): ReplyPending => ({ + ...state, + tag: "reply-pending", + reply, +}); + +export const toReplyPosted = (state: ReplyPending, replySourceUri: string): ReplyPosted => ({ + ...state, + tag: "reply-posted", + replySourceUri, +}); + +export const toUndeliverable = (state: ReplyPending, cause: UndeliverableCause): Undeliverable => ({ + ...state, + tag: "undeliverable", + cause, +}); + +export type RequestClaimedDecision = + | { readonly type: "provision-thread" } + | { readonly type: "record-thread-created" }; + +export type ThreadCreatedDecision = + | { readonly type: "start-turn" } + | { readonly type: "wait" } + | { + readonly type: "record-reply-pending"; + readonly reply: Reply; + }; + +export type ReplyPendingDecision = + | { readonly type: "post-reply" } + | { + readonly type: "record-reply-posted"; + readonly replySourceUri: string; + }; + +export const fromRequestClaimed = ( + _state: RequestClaimed, + context: RequestClaimedContext, +): RequestClaimedDecision => + context.thread === "missing" ? { type: "provision-thread" } : { type: "record-thread-created" }; + +export const fromThreadCreated = ( + _state: ThreadCreated, + context: ThreadCreatedContext, +): ThreadCreatedDecision => { + switch (context.turn) { + case "missing": + return { type: "start-turn" }; + + case "active": + return { type: "wait" }; + + case "completed": + return { + type: "record-reply-pending", + reply: context.reply, + }; + } +}; + +export const fromReplyPending = ( + _state: ReplyPending, + context: ReplyPendingContext, +): ReplyPendingDecision => { + switch (context.platformReply) { + case "missing": + return { type: "post-reply" }; + + case "posted": + return { + type: "record-reply-posted", + replySourceUri: context.replySourceUri, + }; + } +}; diff --git a/apps/server/src/ntbs/processor-new.test.ts b/apps/server/src/ntbs/processor-new.test.ts new file mode 100644 index 000000000000..2da37f349734 --- /dev/null +++ b/apps/server/src/ntbs/processor-new.test.ts @@ -0,0 +1,1815 @@ +import { describe, expect, it } from "@effect/vitest"; +import { MessageId, ProjectId, ThreadId } from "@t3tools/contracts"; +import { Deferred, Effect, Fiber, Layer, Stream } from "effect"; +import { AdapterError, NTBSAdapter, ReplyRejected } from "./adapter.ts"; +import { ExchangeRepository, inMemoryExchangeRepository } from "./ExchangeRepository.ts"; +import { + makeRequestClaimed, + toReplyPending, + toReplyPosted, + toThreadCreated, + toUndeliverable, + type Exchange, + type ReplyPending, + type ReplyPosted, + type Request, + type WorkCoordinates, + type ThreadCreated, +} from "./exchange.ts"; +import { makeNTBSProcessor, type NTBSProcessor, type T3Target } from "./processor.ts"; +import { T3Gateway, T3GatewayError, T3Rejected } from "./t3gateway.ts"; + +/* +Every test in this module is about setting up the dependencies, and seeing what happens as we call `run` and `process` on the processor. + */ + +const withTestProcessor = ( + services: { + readonly t3: Partial; + readonly adapter: Partial; + }, + test: (context: { + readonly processor: NTBSProcessor; + readonly repository: ExchangeRepository; + }) => Effect.Effect, +) => + Effect.gen(function* () { + const processor = yield* makeNTBSProcessor; + const repository = yield* ExchangeRepository; + + return yield* test({ processor, repository }); + }).pipe( + Effect.provide( + Layer.mergeAll( + Layer.mock(T3Gateway)({ + threadActivity: Stream.never, + ...services.t3, + }), + Layer.mock(NTBSAdapter)(services.adapter), + inMemoryExchangeRepository, + ), + ), + ); + +const projectId = ProjectId.make("project-1"); + +const request: Request = { + sourceUri: "test://request/1", + snapshot: "Please fix the bug", + attachments: [], +}; + +const target: T3Target = { + projectId, + startBranchName: "fork/dev", +}; + +const coordinates: WorkCoordinates = { + projectId, + startBranchName: "fork/dev", + startCommitSha: "start-commit-sha", + threadId: ThreadId.make("thread-1"), + userMessageId: MessageId.make("message-1"), + worktreeBranchName: "ntbs/thread-1", +}; + +const secondRequest: Request = { + ...request, + sourceUri: "test://request/2", +}; + +const secondCoordinates: WorkCoordinates = { + ...coordinates, + startCommitSha: "second-start-commit-sha", + threadId: ThreadId.make("thread-2"), + userMessageId: MessageId.make("message-2"), + worktreeBranchName: "ntbs/thread-2", +}; + +const waitForStoredState = ( + repository: ExchangeRepository, + sourceUri: string, + isExpected: (state: Exchange) => state is State, +) => + Effect.gen(function* () { + while (true) { + const state = yield* repository.findBySourceUri(sourceUri); + + if (state !== null && isExpected(state)) { + return state; + } + + yield* Effect.yieldNow; + } + }); + +describe("NTBSProcessor", () => { + describe("process", () => { + it.effect("starts a new request and ignores its sequential redelivery", () => { + const t3Calls: Array = []; + const acknowledgements: Array = []; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.sync(() => { + t3Calls.push("planCoordinates"); + return coordinates; + }), + getThreadStatus: () => + Effect.sync(() => { + t3Calls.push("getThreadStatus"); + return { thread: "missing" as const }; + }), + provisionThread: () => + Effect.sync(() => { + t3Calls.push("provisionThread"); + }), + getTurnStatus: () => + Effect.sync(() => { + t3Calls.push("getTurnStatus"); + return { turn: "missing" as const }; + }), + startTurn: () => + Effect.sync(() => { + t3Calls.push("startTurn"); + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + yield* processor.process(request, target); + yield* processor.process(request, target); + + const expected = toThreadCreated(makeRequestClaimed(request, coordinates)); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + expect(acknowledgements).toEqual([expected]); + expect(t3Calls).toEqual([ + "planCoordinates", + "getThreadStatus", + "provisionThread", + "getTurnStatus", + "startTurn", + ]); + }), + ); + }); + + it.effect("continues after a best-effort acknowledgement fails", () => { + let acknowledgementCalls = 0; + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "present" }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.sync(() => { + startTurnCalls += 1; + }), + }, + adapter: { + acknowledge: () => + Effect.gen(function* () { + acknowledgementCalls += 1; + return yield* new AdapterError({ + reason: "The acknowledgement could not be posted", + cause: "test failure", + }); + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(acknowledgementCalls).toBe(1); + expect(startTurnCalls).toBe(1); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toThreadCreated(makeRequestClaimed(request, coordinates)), + ); + }), + ); + }); + + it.effect("leaves an exchange unchanged while its turn is active", () => { + let startTurnCalls = 0; + let findReplyCalls = 0; + let postReplyCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "present" }), + getTurnStatus: () => Effect.succeed({ turn: "active" }), + startTurn: () => + Effect.sync(() => { + startTurnCalls += 1; + }), + }, + adapter: { + acknowledge: () => Effect.void, + findPostedReply: () => + Effect.sync(() => { + findReplyCalls += 1; + return null; + }), + postReply: () => + Effect.sync(() => { + postReplyCalls += 1; + return "test://reply/unexpected"; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toThreadCreated(makeRequestClaimed(request, coordinates)), + ); + expect(startTurnCalls).toBe(0); + expect(findReplyCalls).toBe(0); + expect(postReplyCalls).toBe(0); + }), + ); + }); + + it.effect("retries a transient provisioning failure during later recovery", () => { + const recoveredTurnStarted = Deferred.makeUnsafe(); + let provisionCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "missing" }), + provisionThread: () => + Effect.gen(function* () { + provisionCalls += 1; + + if (provisionCalls === 1) { + return yield* new T3GatewayError({ + reason: "Thread provisioning temporarily failed", + cause: "test failure", + }); + } + }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => Deferred.succeed(recoveredTurnStarted, undefined), + }, + adapter: { + acknowledge: () => Effect.void, + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + expect((yield* Effect.exit(processor.process(request, target)))._tag).toBe("Failure"); + + const claimed = makeRequestClaimed(request, coordinates); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(claimed); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(recoveredTurnStarted); + + expect(provisionCalls).toBe(2); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toThreadCreated(claimed), + ); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("retries a transient turn-start failure during later recovery", () => { + const recoveredTurnStarted = Deferred.makeUnsafe(); + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "present" }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.gen(function* () { + startTurnCalls += 1; + + if (startTurnCalls === 1) { + return yield* new T3GatewayError({ + reason: "Turn start temporarily failed", + cause: "test failure", + }); + } + + yield* Deferred.succeed(recoveredTurnStarted, undefined); + }), + }, + adapter: { + acknowledge: () => Effect.void, + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + expect((yield* Effect.exit(processor.process(request, target)))._tag).toBe("Failure"); + + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(recoveredTurnStarted); + + expect(startTurnCalls).toBe(2); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Fiber.interrupt(run); + }), + ); + }); + }); + + describe("source serialization", () => { + it.effect("serializes concurrent deliveries of the same request", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + const t3Calls: Array = []; + const acknowledgements: Array = []; + let planCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + t3Calls.push("planCoordinates"); + + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + } + + return coordinates; + }), + getThreadStatus: () => + Effect.sync(() => { + t3Calls.push("getThreadStatus"); + return { thread: "missing" as const }; + }), + provisionThread: () => + Effect.sync(() => { + t3Calls.push("provisionThread"); + }), + getTurnStatus: () => + Effect.sync(() => { + t3Calls.push("getTurnStatus"); + return { turn: "missing" as const }; + }), + startTurn: () => + Effect.sync(() => { + t3Calls.push("startTurn"); + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + // The first request now holds the source lock inside planCoordinates. + yield* Deferred.await(firstPlanStarted); + + const second = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + expect(planCalls).toBe(1); + expect(second.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toBeNull(); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + yield* Fiber.join(first); + yield* Fiber.join(second); + + const expected = toThreadCreated(makeRequestClaimed(request, coordinates)); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + expect(acknowledgements).toEqual([expected]); + expect(t3Calls).toEqual([ + "planCoordinates", + "getThreadStatus", + "provisionThread", + "getTurnStatus", + "startTurn", + ]); + }), + ); + }); + + it.effect("lets a queued delivery claim after the first fails before persistence", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + const acknowledgements: Array = []; + let planCalls = 0; + let provisionCalls = 0; + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + return yield* new T3GatewayError({ + reason: "The first planning attempt failed", + cause: "test failure", + }); + } + + return coordinates; + }), + getThreadStatus: () => Effect.succeed({ thread: "missing" }), + provisionThread: () => + Effect.sync(() => { + provisionCalls += 1; + }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.sync(() => { + startTurnCalls += 1; + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + }, + }, + ({ processor, repository }) => + 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 })); + + expect(planCalls).toBe(1); + expect(second.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toBeNull(); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + + expect((yield* Fiber.await(first))._tag).toBe("Failure"); + yield* Fiber.join(second); + + const expected = toThreadCreated(makeRequestClaimed(request, coordinates)); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + expect(acknowledgements).toEqual([expected]); + expect(planCalls).toBe(2); + expect(provisionCalls).toBe(1); + expect(startTurnCalls).toBe(1); + }), + ); + }); + + it.effect("retains a failed claim for later recovery and ignores its queued redelivery", () => { + const threadStatusStarted = Deferred.makeUnsafe(); + const releaseThreadStatus = Deferred.makeUnsafe(); + const recoveredTurnStarted = Deferred.makeUnsafe(); + let planCalls = 0; + let threadStatusCalls = 0; + let provisionCalls = 0; + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.sync(() => { + planCalls += 1; + return coordinates; + }), + getThreadStatus: () => + Effect.gen(function* () { + threadStatusCalls += 1; + + if (threadStatusCalls === 1) { + yield* Deferred.succeed(threadStatusStarted, undefined); + yield* Deferred.await(releaseThreadStatus); + return yield* new T3GatewayError({ + reason: "Failed after persisting the claim", + cause: "test failure", + }); + } + + return { thread: "missing" as const }; + }), + provisionThread: () => + Effect.sync(() => { + provisionCalls += 1; + }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.gen(function* () { + startTurnCalls += 1; + yield* Deferred.succeed(recoveredTurnStarted, undefined); + }), + }, + adapter: { + acknowledge: () => Effect.void, + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(threadStatusStarted); + + const claimed = makeRequestClaimed(request, coordinates); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(claimed); + + const second = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + expect(second.pollUnsafe()).toBeUndefined(); + expect(planCalls).toBe(1); + expect(threadStatusCalls).toBe(1); + + yield* Deferred.succeed(releaseThreadStatus, undefined); + + expect((yield* Fiber.await(first))._tag).toBe("Failure"); + yield* Fiber.join(second); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(claimed); + expect(planCalls).toBe(1); + expect(threadStatusCalls).toBe(1); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(recoveredTurnStarted); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + toThreadCreated(claimed), + ); + expect(planCalls).toBe(1); + expect(threadStatusCalls).toBe(2); + expect(provisionCalls).toBe(1); + expect(startTurnCalls).toBe(1); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("releases the source lock when its holder is interrupted", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const keepFirstPlanBlocked = Deferred.makeUnsafe(); + const acknowledgements: Array = []; + let planCalls = 0; + let provisionCalls = 0; + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(keepFirstPlanBlocked); + } + + return coordinates; + }), + getThreadStatus: () => Effect.succeed({ thread: "missing" }), + provisionThread: () => + Effect.sync(() => { + provisionCalls += 1; + }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.sync(() => { + startTurnCalls += 1; + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + }, + }, + ({ processor, repository }) => + 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 })); + + expect(planCalls).toBe(1); + expect(second.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toBeNull(); + + yield* Fiber.interrupt(first); + expect((yield* Fiber.await(first))._tag).toBe("Failure"); + yield* Fiber.join(second); + + const expected = toThreadCreated(makeRequestClaimed(request, coordinates)); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + expect(acknowledgements).toEqual([expected]); + expect(planCalls).toBe(2); + expect(provisionCalls).toBe(1); + expect(startTurnCalls).toBe(1); + }), + ); + }); + + it.effect("interrupting a queued delivery preserves the lock for later deliveries", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + const t3Calls: Array = []; + const acknowledgements: Array = []; + let planCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + t3Calls.push("planCoordinates"); + + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + } + + return coordinates; + }), + getThreadStatus: () => + Effect.sync(() => { + t3Calls.push("getThreadStatus"); + return { thread: "missing" as const }; + }), + provisionThread: () => + Effect.sync(() => { + t3Calls.push("provisionThread"); + }), + getTurnStatus: () => + Effect.sync(() => { + t3Calls.push("getTurnStatus"); + return { turn: "missing" as const }; + }), + startTurn: () => + Effect.sync(() => { + t3Calls.push("startTurn"); + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + }, + }, + ({ processor, repository }) => + 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 })); + + expect(planCalls).toBe(1); + expect(interruptedWaiter.pollUnsafe()).toBeUndefined(); + + yield* Fiber.interrupt(interruptedWaiter); + expect((yield* Fiber.await(interruptedWaiter))._tag).toBe("Failure"); + expect(first.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toBeNull(); + + const later = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + expect(planCalls).toBe(1); + expect(later.pollUnsafe()).toBeUndefined(); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + yield* Fiber.join(first); + yield* Fiber.join(later); + + const expected = toThreadCreated(makeRequestClaimed(request, coordinates)); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + expect(acknowledgements).toEqual([expected]); + expect(t3Calls).toEqual([ + "planCoordinates", + "getThreadStatus", + "provisionThread", + "getTurnStatus", + "startTurn", + ]); + }), + ); + }); + + it.effect("allows different requests to proceed concurrently", () => { + const firstPlanStarted = Deferred.makeUnsafe(); + const releaseFirstPlan = Deferred.makeUnsafe(); + const secondPlanStarted = Deferred.makeUnsafe(); + const acknowledgements: Array = []; + let planCalls = 0; + let provisionCalls = 0; + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => + Effect.gen(function* () { + planCalls += 1; + + if (planCalls === 1) { + yield* Deferred.succeed(firstPlanStarted, undefined); + yield* Deferred.await(releaseFirstPlan); + return coordinates; + } + + yield* Deferred.succeed(secondPlanStarted, undefined); + return secondCoordinates; + }), + getThreadStatus: () => Effect.succeed({ thread: "missing" }), + provisionThread: () => + Effect.sync(() => { + provisionCalls += 1; + }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.sync(() => { + startTurnCalls += 1; + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const first = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(firstPlanStarted); + + const second = yield* processor + .process(secondRequest, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + expect(yield* Deferred.isDone(secondPlanStarted)).toBe(true); + yield* Fiber.join(second); + + const expectedSecond = toThreadCreated( + makeRequestClaimed(secondRequest, secondCoordinates), + ); + + expect(first.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toBeNull(); + expect(yield* repository.findBySourceUri(secondRequest.sourceUri)).toEqual( + expectedSecond, + ); + expect(acknowledgements).toEqual([expectedSecond]); + expect(planCalls).toBe(2); + expect(provisionCalls).toBe(1); + expect(startTurnCalls).toBe(1); + + yield* Deferred.succeed(releaseFirstPlan, undefined); + yield* Fiber.join(first); + + const expectedFirst = toThreadCreated(makeRequestClaimed(request, coordinates)); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expectedFirst); + expect(acknowledgements).toEqual([expectedSecond, expectedFirst]); + expect(planCalls).toBe(2); + expect(provisionCalls).toBe(2); + expect(startTurnCalls).toBe(2); + }), + ); + }); + }); + + describe("run", () => { + it.effect("resumes non-terminal exchanges when run starts", () => { + const replyPosted = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Recovered reply", + }; + const replySourceUri = "test://reply/recovered"; + const postedReplies: Array = []; + + return withTestProcessor( + { + t3: { + getTurnStatus: () => + Effect.succeed({ + turn: "completed", + reply, + }), + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.gen(function* () { + postedReplies.push(state); + yield* Deferred.succeed(replyPosted, undefined); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(replyPosted); + + const replyPending = toReplyPending(threadCreated, reply); + const expected = toReplyPosted(replyPending, replySourceUri); + + expect(postedReplies).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("routes thread activity only for stored exchanges", () => { + const unknownActivity = Deferred.makeUnsafe(); + const storedActivity = Deferred.makeUnsafe(); + const replyPosted = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply after thread activity", + }; + const replySourceUri = "test://reply/activity"; + const observedThreads: Array = []; + + return withTestProcessor( + { + t3: { + threadActivity: Stream.concat( + Stream.fromEffect(Deferred.await(unknownActivity)), + Stream.fromEffect(Deferred.await(storedActivity)), + ), + getTurnStatus: (state) => + Effect.sync(() => { + observedThreads.push(state.t3.threadId); + return { + turn: "completed" as const, + reply, + }; + }), + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: () => + Effect.gen(function* () { + yield* Deferred.succeed(replyPosted, undefined); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + // Startup recovery has already observed the empty repository. + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(threadCreated); + + yield* Deferred.succeed(unknownActivity, ThreadId.make("unknown-thread")); + yield* Deferred.succeed(storedActivity, coordinates.threadId); + yield* Deferred.await(replyPosted); + + const expected = toReplyPosted(toReplyPending(threadCreated, reply), replySourceUri); + + expect(observedThreads).toEqual([coordinates.threadId]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("serializes thread activity with a redelivered request", () => { + const threadActivity = Deferred.makeUnsafe(); + const turnStatusStarted = Deferred.makeUnsafe(); + const releaseTurnStatus = Deferred.makeUnsafe(); + const replyPosted = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply from thread activity", + }; + const replySourceUri = "test://reply/activity-race"; + const postedReplies: Array = []; + + return withTestProcessor( + { + t3: { + threadActivity: Stream.fromEffect(Deferred.await(threadActivity)), + getTurnStatus: () => + Effect.gen(function* () { + yield* Deferred.succeed(turnStatusStarted, undefined); + yield* Deferred.await(releaseTurnStatus); + return { + turn: "completed" as const, + reply, + }; + }), + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.gen(function* () { + postedReplies.push(state); + yield* Deferred.succeed(replyPosted, undefined); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(threadCreated); + yield* Deferred.succeed(threadActivity, coordinates.threadId); + yield* Deferred.await(turnStatusStarted); + + const redelivery = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + expect(redelivery.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Deferred.succeed(releaseTurnStatus, undefined); + yield* Deferred.await(replyPosted); + yield* Fiber.join(redelivery); + + const replyPending = toReplyPending(threadCreated, reply); + const expected = toReplyPosted(replyPending, replySourceUri); + + expect(postedReplies).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("serializes startup recovery with a redelivered request", () => { + const turnStatusStarted = Deferred.makeUnsafe(); + const releaseTurnStatus = Deferred.makeUnsafe(); + const replyPosted = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply from startup recovery", + }; + const replySourceUri = "test://reply/recovery-race"; + const postedReplies: Array = []; + + return withTestProcessor( + { + t3: { + getTurnStatus: () => + Effect.gen(function* () { + yield* Deferred.succeed(turnStatusStarted, undefined); + yield* Deferred.await(releaseTurnStatus); + return { + turn: "completed" as const, + reply, + }; + }), + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.gen(function* () { + postedReplies.push(state); + yield* Deferred.succeed(replyPosted, undefined); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(turnStatusStarted); + + const redelivery = yield* processor + .process(request, target) + .pipe(Effect.forkChild({ startImmediately: true })); + + expect(redelivery.pollUnsafe()).toBeUndefined(); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(threadCreated); + + yield* Deferred.succeed(releaseTurnStatus, undefined); + yield* Deferred.await(replyPosted); + yield* Fiber.join(redelivery); + + const replyPending = toReplyPending(threadCreated, reply); + const expected = toReplyPosted(replyPending, replySourceUri); + + expect(postedReplies).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("retries a transient turn-status failure on later thread activity", () => { + const firstStatusFinished = Deferred.makeUnsafe(); + const threadActivity = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply after retrying turn status", + }; + const replySourceUri = "test://reply/turn-status-retry"; + let statusCalls = 0; + + return withTestProcessor( + { + t3: { + threadActivity: Stream.fromEffect(Deferred.await(threadActivity)), + getTurnStatus: () => { + statusCalls += 1; + + return statusCalls === 1 + ? Effect.fail( + new T3GatewayError({ + reason: "Turn status temporarily unavailable", + cause: "test failure", + }), + ).pipe(Effect.ensuring(Deferred.succeed(firstStatusFinished, undefined))) + : Effect.succeed({ turn: "completed" as const, reply }); + }, + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: () => Effect.succeed(replySourceUri), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(firstStatusFinished); + yield* Deferred.succeed(threadActivity, coordinates.threadId); + + const posted = yield* waitForStoredState( + repository, + request.sourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + + expect(statusCalls).toBe(2); + expect(posted).toEqual( + toReplyPosted(toReplyPending(threadCreated, reply), replySourceUri), + ); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("continues startup recovery after one exchange fails", () => { + const reply = { + type: "answer" as const, + text: "Reply recovered after another exchange failed", + }; + const replySourceUri = "test://reply/recovery-continued"; + let failingSourceUri = ""; + let successfulSourceUri = ""; + const postedSources: Array = []; + + return withTestProcessor( + { + t3: {}, + adapter: { + findPostedReply: (state) => + state.sourceUri === failingSourceUri + ? Effect.fail( + new AdapterError({ + reason: "Recovery failed for this exchange", + cause: "test failure", + }), + ) + : Effect.succeed(null), + postReply: (state) => + Effect.sync(() => { + postedSources.push(state.sourceUri); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const firstPending = toReplyPending( + toThreadCreated(makeRequestClaimed(request, coordinates)), + reply, + ); + const secondPending = toReplyPending( + toThreadCreated(makeRequestClaimed(secondRequest, secondCoordinates)), + reply, + ); + yield* repository.upsert(firstPending); + yield* repository.upsert(secondPending); + + const recoveryOrder = yield* repository.findNonTerminalExchanges; + failingSourceUri = recoveryOrder[0]!.sourceUri; + successfulSourceUri = recoveryOrder[1]!.sourceUri; + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + const posted = yield* waitForStoredState( + repository, + successfulSourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + const expectedFailing = + failingSourceUri === firstPending.sourceUri ? firstPending : secondPending; + const expectedSuccessful = + successfulSourceUri === firstPending.sourceUri ? firstPending : secondPending; + + expect(yield* repository.findBySourceUri(failingSourceUri)).toEqual(expectedFailing); + expect(posted).toEqual(toReplyPosted(expectedSuccessful, replySourceUri)); + expect(postedSources).toEqual([successfulSourceUri]); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("continues processing thread activity after one event fails", () => { + const startupStatusRead = Deferred.makeUnsafe(); + const firstActivity = Deferred.makeUnsafe(); + const firstActivityFinished = Deferred.makeUnsafe(); + const secondActivity = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply from the later activity event", + }; + const replySourceUri = "test://reply/later-activity"; + let firstExchangeStatusCalls = 0; + let secondExchangeStatusCalls = 0; + + return withTestProcessor( + { + t3: { + threadActivity: Stream.concat( + Stream.fromEffect(Deferred.await(firstActivity)), + Stream.fromEffect(Deferred.await(secondActivity)), + ), + getTurnStatus: (state) => { + if (state.sourceUri === request.sourceUri) { + firstExchangeStatusCalls += 1; + + if (firstExchangeStatusCalls === 1) { + return Deferred.succeed(startupStatusRead, undefined).pipe( + Effect.as({ turn: "active" as const }), + ); + } + + return Effect.fail( + new T3GatewayError({ + reason: "This activity event could not be processed", + cause: "test failure", + }), + ).pipe(Effect.ensuring(Deferred.succeed(firstActivityFinished, undefined))); + } + + secondExchangeStatusCalls += 1; + return Effect.succeed({ turn: "completed" as const, reply }); + }, + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: () => Effect.succeed(replySourceUri), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const firstThreadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(firstThreadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + yield* Deferred.await(startupStatusRead); + + const secondThreadCreated = toThreadCreated( + makeRequestClaimed(secondRequest, secondCoordinates), + ); + yield* repository.upsert(secondThreadCreated); + yield* Deferred.succeed(firstActivity, coordinates.threadId); + yield* Deferred.await(firstActivityFinished); + yield* Deferred.succeed(secondActivity, secondCoordinates.threadId); + + const posted = yield* waitForStoredState( + repository, + secondRequest.sourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual( + firstThreadCreated, + ); + expect(firstExchangeStatusCalls).toBe(2); + expect(secondExchangeStatusCalls).toBe(1); + expect(posted).toEqual( + toReplyPosted(toReplyPending(secondThreadCreated, reply), replySourceUri), + ); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("subscribes to thread activity before startup recovery finishes", () => { + const recoveryStarted = Deferred.makeUnsafe(); + const releaseRecovery = Deferred.makeUnsafe(); + const threadActivity = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply posted while startup recovery is blocked", + }; + const replySourceUri = "test://reply/during-recovery"; + + return withTestProcessor( + { + t3: { + threadActivity: Stream.fromEffect(Deferred.await(threadActivity)), + 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 }), + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: () => Effect.succeed(replySourceUri), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const recovering = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(recovering); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(recoveryStarted); + + const activeDuringRecovery = toThreadCreated( + makeRequestClaimed(secondRequest, secondCoordinates), + ); + yield* repository.upsert(activeDuringRecovery); + yield* Deferred.succeed(threadActivity, secondCoordinates.threadId); + + const posted = yield* waitForStoredState( + repository, + secondRequest.sourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + + expect(yield* Deferred.isDone(releaseRecovery)).toBe(false); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(recovering); + expect(posted).toEqual( + toReplyPosted(toReplyPending(activeDuringRecovery, reply), replySourceUri), + ); + + yield* Deferred.succeed(releaseRecovery, undefined); + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("posts once when startup recovery races with thread activity", () => { + const threadActivity = Deferred.makeUnsafe(); + const activityHandled = Deferred.makeUnsafe(); + const turnStatusStarted = Deferred.makeUnsafe(); + const releaseTurnStatus = Deferred.makeUnsafe(); + const secondTurnStatusStarted = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply from the recovery and activity race", + }; + const replySourceUri = "test://reply/recovery-activity-race"; + let turnStatusCalls = 0; + let postCalls = 0; + + return withTestProcessor( + { + t3: { + threadActivity: Stream.concat( + Stream.fromEffect(Deferred.await(threadActivity)), + Stream.fromEffect( + Deferred.succeed(activityHandled, undefined).pipe( + Effect.as(ThreadId.make("activity-handled")), + ), + ), + ), + getTurnStatus: () => + Effect.gen(function* () { + turnStatusCalls += 1; + + if (turnStatusCalls === 1) { + yield* Deferred.succeed(turnStatusStarted, undefined); + yield* Deferred.await(releaseTurnStatus); + } else { + yield* Deferred.succeed(secondTurnStatusStarted, undefined); + } + + return { turn: "completed" as const, reply }; + }), + }, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: () => + Effect.sync(() => { + postCalls += 1; + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + yield* repository.upsert(threadCreated); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(turnStatusStarted); + yield* Deferred.succeed(threadActivity, coordinates.threadId); + yield* Effect.yieldNow; + + expect(yield* Deferred.isDone(secondTurnStatusStarted)).toBe(false); + + yield* Deferred.succeed(releaseTurnStatus, undefined); + yield* Deferred.await(activityHandled); + + const posted = yield* waitForStoredState( + repository, + request.sourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + + expect(turnStatusCalls).toBe(1); + expect(postCalls).toBe(1); + expect(posted).toEqual( + toReplyPosted(toReplyPending(threadCreated, reply), replySourceUri), + ); + + yield* Fiber.interrupt(run); + }), + ); + }); + }); + + describe("reply delivery", () => { + it.effect("retries a transient reply-posting failure during later recovery", () => { + const firstPostFinished = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply after a transient posting failure", + }; + const replySourceUri = "test://reply/retried-post"; + let postCalls = 0; + + return withTestProcessor( + { + t3: {}, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: () => { + postCalls += 1; + + return postCalls === 1 + ? Effect.fail( + new AdapterError({ + reason: "Reply posting temporarily failed", + cause: "test failure", + }), + ).pipe(Effect.ensuring(Deferred.succeed(firstPostFinished, undefined))) + : Effect.succeed(replySourceUri); + }, + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const pending = toReplyPending( + toThreadCreated(makeRequestClaimed(request, coordinates)), + reply, + ); + yield* repository.upsert(pending); + + const firstRun = yield* processor.run.pipe( + Effect.forkChild({ startImmediately: true }), + ); + + yield* Deferred.await(firstPostFinished); + yield* Fiber.interrupt(firstRun); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(pending); + + const secondRun = yield* processor.run.pipe( + Effect.forkChild({ startImmediately: true }), + ); + const posted = yield* waitForStoredState( + repository, + request.sourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + + expect(postCalls).toBe(2); + expect(posted).toEqual(toReplyPosted(pending, replySourceUri)); + + yield* Fiber.interrupt(secondRun); + }), + ); + }); + + it.effect("retries reply discovery before posting during later recovery", () => { + const firstDiscoveryFinished = Deferred.makeUnsafe(); + const reply = { + type: "answer" as const, + text: "Reply after a transient discovery failure", + }; + const replySourceUri = "test://reply/retried-discovery"; + let findCalls = 0; + let postCalls = 0; + + return withTestProcessor( + { + t3: {}, + adapter: { + findPostedReply: () => { + findCalls += 1; + + return findCalls === 1 + ? Effect.fail( + new AdapterError({ + reason: "Reply discovery temporarily failed", + cause: "test failure", + }), + ).pipe(Effect.ensuring(Deferred.succeed(firstDiscoveryFinished, undefined))) + : Effect.succeed(null); + }, + postReply: () => + Effect.sync(() => { + postCalls += 1; + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const pending = toReplyPending( + toThreadCreated(makeRequestClaimed(request, coordinates)), + reply, + ); + yield* repository.upsert(pending); + + const firstRun = yield* processor.run.pipe( + Effect.forkChild({ startImmediately: true }), + ); + + yield* Deferred.await(firstDiscoveryFinished); + yield* Fiber.interrupt(firstRun); + + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(pending); + expect(postCalls).toBe(0); + + const secondRun = yield* processor.run.pipe( + Effect.forkChild({ startImmediately: true }), + ); + const posted = yield* waitForStoredState( + repository, + request.sourceUri, + (state): state is ReplyPosted => state.tag === "reply-posted", + ); + + expect(findCalls).toBe(2); + expect(postCalls).toBe(1); + expect(posted).toEqual(toReplyPosted(pending, replySourceUri)); + + yield* Fiber.interrupt(secondRun); + }), + ); + }); + + it.effect("records a reply already found on the platform without posting it again", () => { + const reply = { + type: "answer" as const, + text: "Already delivered", + }; + const discoveredReplySourceUri = "test://reply/already-posted"; + const findCalls: Array = []; + let postCalls = 0; + + return withTestProcessor( + { + t3: {}, + adapter: { + findPostedReply: (state) => + Effect.sync(() => { + findCalls.push(state); + return discoveredReplySourceUri; + }), + postReply: () => + Effect.sync(() => { + postCalls += 1; + return "test://reply/unexpected"; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + const replyPending = toReplyPending(threadCreated, reply); + yield* repository.upsert(replyPending); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + const expected = toReplyPosted(replyPending, discoveredReplySourceUri); + + expect(findCalls).toEqual([replyPending]); + expect(postCalls).toBe(0); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("records a definitively rejected reply as undeliverable", () => { + const reply = { + type: "answer" as const, + text: "Reply that cannot be delivered", + }; + const rejectionCause = { + message: "The originating discussion was deleted", + }; + const postCalls: Array = []; + + return withTestProcessor( + { + t3: {}, + adapter: { + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.gen(function* () { + postCalls.push(state); + return yield* new ReplyRejected({ cause: rejectionCause }); + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + const replyPending = toReplyPending(threadCreated, reply); + yield* repository.upsert(replyPending); + + const run = yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + const expected = toUndeliverable(replyPending, rejectionCause); + + expect(postCalls).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + + yield* Fiber.interrupt(run); + }), + ); + }); + + it.effect("delivers a failure reply when T3 rejects thread provisioning", () => { + const rejectionReason = "T3 cannot provision this request"; + const rejectionCause = { + message: "The selected project no longer exists", + }; + const replySourceUri = "test://reply/provisioning-failure"; + const postedReplies: Array = []; + let provisionCalls = 0; + let acknowledgementCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "missing" }), + provisionThread: () => + Effect.gen(function* () { + provisionCalls += 1; + return yield* new T3Rejected({ + reason: rejectionReason, + cause: rejectionCause, + }); + }), + }, + adapter: { + acknowledge: () => + Effect.sync(() => { + acknowledgementCalls += 1; + }), + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.sync(() => { + postedReplies.push(state); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + const claimed = makeRequestClaimed(request, coordinates); + const failureReply = { + type: "failure" as const, + text: rejectionReason, + cause: rejectionCause, + }; + const replyPending = toReplyPending(claimed, failureReply); + const expected = toReplyPosted(replyPending, replySourceUri); + + expect(provisionCalls).toBe(1); + expect(acknowledgementCalls).toBe(0); + expect(postedReplies).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + }), + ); + }); + + it.effect("delivers a failure reply when T3 rejects turn start", () => { + const rejectionReason = "T3 cannot start the turn"; + const rejectionCause = { + message: "The configured provider is unavailable", + }; + const replySourceUri = "test://reply/turn-start-failure"; + const acknowledgements: Array = []; + const postedReplies: Array = []; + let startTurnCalls = 0; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "present" }), + getTurnStatus: () => Effect.succeed({ turn: "missing" }), + startTurn: () => + Effect.gen(function* () { + startTurnCalls += 1; + return yield* new T3Rejected({ + reason: rejectionReason, + cause: rejectionCause, + }); + }), + }, + adapter: { + acknowledge: (state) => + Effect.sync(() => { + acknowledgements.push(state); + }), + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.sync(() => { + postedReplies.push(state); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + const failureReply = { + type: "failure" as const, + text: rejectionReason, + cause: rejectionCause, + }; + const replyPending = toReplyPending(threadCreated, failureReply); + const expected = toReplyPosted(replyPending, replySourceUri); + + expect(startTurnCalls).toBe(1); + expect(acknowledgements).toEqual([threadCreated]); + expect(postedReplies).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + }), + ); + }); + + it.effect("posts a completed T3 reply", () => { + const reply = { + type: "answer" as const, + text: "The bug is fixed.", + }; + const replySourceUri = "test://reply/1"; + const postedReplies: Array = []; + + return withTestProcessor( + { + t3: { + planCoordinates: () => Effect.succeed(coordinates), + getThreadStatus: () => Effect.succeed({ thread: "present" }), + getTurnStatus: () => + Effect.succeed({ + turn: "completed", + reply, + }), + }, + adapter: { + acknowledge: () => Effect.void, + findPostedReply: () => Effect.succeed(null), + postReply: (state) => + Effect.sync(() => { + postedReplies.push(state); + return replySourceUri; + }), + }, + }, + ({ processor, repository }) => + Effect.gen(function* () { + yield* processor.process(request, target); + + const threadCreated = toThreadCreated(makeRequestClaimed(request, coordinates)); + const replyPending = toReplyPending(threadCreated, reply); + const expected = toReplyPosted(replyPending, replySourceUri); + + expect(postedReplies).toEqual([replyPending]); + expect(yield* repository.findBySourceUri(request.sourceUri)).toEqual(expected); + }), + ); + }); + }); +}); diff --git a/apps/server/src/ntbs/processor.old.ts b/apps/server/src/ntbs/processor.old.ts new file mode 100644 index 000000000000..5eccf9d3d61f --- /dev/null +++ b/apps/server/src/ntbs/processor.old.ts @@ -0,0 +1,760 @@ +import { + type ChatAttachment, + CommandId, + DEFAULT_PROVIDER_INTERACTION_MODE, + MessageId, + OrchestrationCommand, + type OrchestrationEvent, + type ProjectId, + ThreadId, +} from "@t3tools/contracts"; +import type * as NTBS from "./exchange.ts"; +import { Context, Crypto, Data, DateTime, Effect, Stream, Semaphore } from "effect"; +import { NTBSAdapter } from "./adapter.ts"; +import type { T3Gateway } from "./t3gateway.ts"; +import type { ExchangeRepository } from "./ExchangeRepository.ts"; +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 { getAutoBootstrapDefaultModelSelection } from "../serverRuntimeStartup.ts"; +import { DEFAULT_THREAD_TITLE } from "@t3tools/shared/threadTitle"; +import { buildTemporaryWorktreeBranchName } from "@t3tools/shared/git"; + +/* +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. + +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. +*/ + +/** + * Describes _where_ the T3 works goes. Necessary for creating worktrees, threads and starting turns. + */ +export type T3Target = { + readonly projectId: ProjectId; + /** + * The starting point for the thread's worktree: the new branch is created from this ref. + * + * Usually a branch name such as `main`. Before use it is resolved against `origin`, so the worktree starts from the latest remote commit even when the local copy of the branch is behind. A commit SHA is also accepted and is used as-is. + * + * Set by the platform-specific inbound code. + */ + readonly baseRef: string; +}; + +export class NTBSProcessorError extends Data.TaggedError("NTBSProcessorError")<{ + reason: string; + cause: unknown; +}> {} + +export interface NTBSProcessor { + /** + * Claims one external request and drives its exchange. + * + * Does no filtering: the caller decides whether a request deserves T3 work, and everything passed here starts it. + * + * Returns once the exchange is claimed and under way, not once the request is answered: the reply is posted later, when T3 reports the turn finished. + * + * Idempotent per `sourceUri`: a redelivery of an already-claimed request is a no-op, whatever state that exchange has reached. Concurrent deliveries of the same request are serialized, so only the first claims it. + */ + readonly process: ( + request: NTBS.Request, + t3Target: 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 only moves when its T3 thread does. + * + * Never returns. It has no error channel: a failure on one exchange is logged and the next event is still processed. + */ + readonly run: Effect.Effect; +} + +export const makeNTBSProcessorTag = (key: string) => Context.Service(key); + +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; + +/** + * Builds a processor for the adapter found in the context. + * + * Build one per platform, each with its own adapter provided. + */ +export const makeNTBSProcessor: Effect.Effect = + Effect.gen(function* () { + const adapter = yield* NTBSAdapter; + + const orFail = (reason: string) => + Effect.mapError((cause: unknown) => new NTBSProcessorError({ reason, cause })); + + const crypto = yield* Crypto.Crypto; + const randomUUID = crypto.randomUUIDv4.pipe(orFail("Failed creating a UUID v4")); + + const projectionSnapshotQuery = yield* ProjectionSnapshotQuery; + const projectionTurnRepository = yield* ProjectionTurnRepository; + + const gitWorkflowService = yield* GitWorkflowService; + + const getNow = DateTime.now.pipe(Effect.map(DateTime.formatIso)); + + /** Keeps each user message's lock until its final response is recorded and no caller uses it. */ + const responseLocks = new Map< + MessageId, + { + readonly semaphore: Semaphore.Semaphore; + callers: number; + responsePosted: boolean; + } + >(); + + const getResponseLock = (userMessageId: MessageId) => { + let lock = responseLocks.get(userMessageId); + if (lock === undefined) { + lock = { + semaphore: Semaphore.makeUnsafe(1), + callers: 0, + responsePosted: false, + }; + responseLocks.set(userMessageId, lock); + } + return lock; + }; + + const markResponsePosted = (userMessageId: MessageId): void => { + const lock = responseLocks.get(userMessageId); + if (lock !== undefined) { + lock.responsePosted = true; + } + }; + + /** + * Prevents the turn started by one user message from producing competing + * final outcomes, such as both a normal response and a timeout. + */ + const ensureUniqueOutcome = ( + userMessageId: MessageId, + effect: Effect.Effect, + ): Effect.Effect => + Effect.suspend(() => { + const lock = getResponseLock(userMessageId); + lock.callers += 1; + + return lock.semaphore.withPermit(effect).pipe( + Effect.ensuring( + Effect.sync(() => { + lock.callers -= 1; + if ( + lock.callers === 0 && + lock.responsePosted && + responseLocks.get(userMessageId) === lock + ) { + responseLocks.delete(userMessageId); + } + }), + ), + ); + }); + + /** + * Starts the first turn in an existing T3 thread. + * + * Uses the user message ID recorded in `ThreadCreated` so the resulting turn + * and response can be matched to the external request. + */ + const startT3Turn = ( + threadId: ThreadId, + userMessageId: MessageId, + snapshot: string, + attachments: ReadonlyArray, + ): Effect.Effect => + Effect.gen(function* () { + const commandId = CommandId.make(yield* randomUUID); + const createdAt = yield* getNow; + + yield* orchestrationEngineService + .dispatch( + OrchestrationCommand.make({ + type: "thread.turn.start", + commandId, + threadId, + message: { + messageId: userMessageId, + role: "user", + text: snapshot, + attachments, + }, + runtimeMode: "full-access", + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + createdAt, + }), + ) + .pipe(orFail("Failed to start the first T3 turn")); + }); + + const getTurn = (threadId: ThreadId, userMessageId: MessageId) => + Effect.gen(function* () { + const turns = yield* projectionTurnRepository + .listByThreadId({ threadId }) + .pipe(orFail(`Failed loading turns for T3 thread ${threadId}`)); + /* + Using `.find` is safe: a userMessageId can never label more than one turn. + The UUID is minted once per request, and a turn start is only repeated + (by recovery) when no turn exists for it. + If the turn started, `.find` finds it. Finding none means the turn never + started (e.g. crash) or T3 discarded it before a provider picked it up. + */ + const turn = turns.find((turn) => turn.pendingMessageId === userMessageId); + return turn ?? null; + }); + + /** + * Reads the final outcome of the turn started by one NTBS user message. + * Returns `null` while that exact turn is still pending or running. + */ + const resolveT3Outcome = ( + threadId: ThreadId, + userMessageId: MessageId, + ): Effect.Effect => + Effect.gen(function* () { + const turn = yield* getTurn(threadId, userMessageId); + + if (!turn) { + return yield* new NTBSProcessorError({ + reason: `Turn for user message ${userMessageId} not found.`, + cause: { threadId, userMessageId }, + }); + } + + if (turn.state === "pending" || turn.state === "running") { + return null; + } + + const maybeThread = yield* projectionSnapshotQuery + .getThreadDetailById(threadId) + .pipe(orFail(`Failed loading T3 thread ${threadId}`)); + + const thread = yield* Effect.fromOption(maybeThread).pipe( + orFail(`Could not find T3 thread ${threadId}`), + ); + + if (turn.state === "completed") { + const assistantMessage = + turn.assistantMessageId === null + ? undefined + : thread.messages.find((message) => message.id === turn.assistantMessageId); + + const text = assistantMessage?.text.trim() ?? ""; + + return text.length > 0 + ? { type: "answer", text } + : { + type: "failure", + text: "T3 completed without producing a response.", + }; + } + + if (turn.state === "error") { + return { + type: "failure", + text: thread.session?.lastError ?? "T3 failed while processing this request.", + }; + } + + return { + type: "cancellation", + text: "T3 stopped processing this request.", + }; + }); + + /** + * Posts one final response and records it in the adapter lifecycle. + * + * The caller must have already confirmed that no response is recorded and + * must hold the outcome lock for this user message. If the platform already + * contains the response, it is recorded instead of reposted. + */ + const postResponse = ( + threadCreated: NTBS.ThreadCreated, + response: NTBSResponse, + ): Effect.Effect => + Effect.gen(function* () { + const existingResponseMessageId = yield* adapter + .findMatchingResponseMessage(threadCreated) + .pipe(orFail("Failed checking whether the NTBS response was already posted")); + + const responseMessageId = + existingResponseMessageId ?? + (yield* adapter + .postResponse(threadCreated, response) + .pipe(orFail("Failed posting the NTBS response"))); + + yield* adapter + .save({ + ...threadCreated, + state: "thread.response.posted", + responseMessageId, + }) + .pipe(orFail("Failed recording the posted NTBS response")); + + markResponsePosted(threadCreated.t3.userMessageId); + }); + + /** + * Posts the final response when a T3 session event ends an NTBS turn. + * Other T3 events and threads unknown to this adapter are ignored. + */ + const processT3Event = (event: OrchestrationEvent): Effect.Effect => + Effect.gen(function* () { + if (event.type !== "thread.session-set") { + return; + } + + const threadId = event.payload.threadId; + + /* + We may receive events for threads that are not related to the current + platform, and thus, adapter. + So we check if the thread in question exists in the adapter records. + */ + const recordedThread = yield* adapter.findByThreadId(threadId).pipe( + Effect.catchTag("ThreadNotFound", () => Effect.succeed(null)), + orFail("Failed loading the NTBS lifecycle for a T3 event"), + ); + + if (recordedThread === null) { + return; + } + + /* + At the same time a thread may have different messages. We're only interested + in the last user message that appears in the adapter records. + */ + const userMessageId = recordedThread.t3.userMessageId; + + yield* ensureUniqueOutcome( + userMessageId, + Effect.gen(function* () { + // Timeout handling may have posted a response while this event was + // waiting for the same user message's outcome lock. + const currentRecord = yield* adapter.findByThreadId(threadId).pipe( + Effect.catchTag("ThreadNotFound", () => Effect.succeed(null)), + orFail("Failed reloading the NTBS lifecycle before posting its outcome"), + ); + + if (currentRecord === null) { + return; + } + + if (currentRecord.state === "thread.response.posted") { + markResponsePosted(userMessageId); + return; + } + + const response = yield* resolveT3Outcome(threadId, userMessageId); + if (response === null) { + return; + } + + yield* postResponse(currentRecord, response); + }), + ); + }); + + /** + * Resolves where a new thread worktree starts from. + * + * Fetches `origin` and prefers the remote state of `baseRef`, so a branch name resolves to its latest remote commit even when the local copy is behind. + * + * When no remote branch with that name exists + * (a commit SHA, a tag, a local-only branch, or no reachable remote), + * the ref is returned as-is for git to resolve during worktree creation. + * + * Never fails: an unresolvable ref surfaces later as a worktree-creation error, + * which carries the real git cause. + * + */ + const resolveWorktreeBase = (input: { + readonly cwd: string; + readonly baseRef: string; + }): Effect.Effect<{ readonly refName: string; readonly baseRefName: string | null }> => + Effect.gen(function* () { + // A failed fetch only means we resolve against the last-known remote state + // The tracking ref may still exist locally + yield* gitWorkflowService + .fetchRemote({ + cwd: input.cwd, + remoteName: "origin", + }) + .pipe( + Effect.catch((cause) => + Effect.logDebug("NTBS fetch of origin failed; resolving against local state.", { + cwd: input.cwd, + cause, + }), + ), + ); + + return yield* gitWorkflowService + .resolveRemoteTrackingCommit({ + cwd: input.cwd, + refName: input.baseRef, + fallbackRemoteName: "origin", + }) + .pipe( + Effect.map((resolved) => ({ + refName: resolved.commitSha, + baseRefName: input.baseRef, + })), + Effect.catch((cause) => + Effect.logDebug("NTBS base ref is not a remote branch; using it as-is", { + baseRef: input.baseRef, + cwd: input.cwd, + cause, + }).pipe( + Effect.as({ + refName: input.baseRef, + baseRefName: null, + }), + ), + ), + ); + }); + + const orchestrationEngineService = yield* OrchestrationEngineService; + + const projectScriptRunner = yield* ProjectSetupScriptRunner; + + /** + * Semaphore-like behavior to avoid triggering multiple threads + * and turns for the same requests. + */ + const inFlightRequests = new Set(); + + /** + * Creates an isolated worktree and a new T3 thread. + * + * Uses the supplied project and base ref. The thread starts with T3's default + * title, the project's default model or T3's fallback model, `full-access` + * runtime mode, and `default` interaction mode. + * + * Does not start a turn, read platform data or call the adapter. + * + * The final title of the thread is generated by T3 after the first turn starts. + */ + const createT3Thread = (t3Context: T3Context): Effect.Effect => + Effect.gen(function* () { + const maybeProject = yield* projectionSnapshotQuery + .getProjectShellById(t3Context.projectId) + .pipe(orFail("Could not load the T3 Project.")); + + const project = yield* Effect.fromOption(maybeProject).pipe( + orFail(`T3 project ${t3Context.projectId} does not exist.`), + ); + + const threadUUID = yield* randomUUID; + const threadId = ThreadId.make(threadUUID); + + const createdAt = yield* getNow; + // TODO: Resolve the title in a better way + const title = DEFAULT_THREAD_TITLE; + const modelSelection = + project.defaultModelSelection ?? getAutoBootstrapDefaultModelSelection(); + + const commandId = CommandId.make(yield* randomUUID); + + // create the isolated branch and worktree + const branchName = buildTemporaryWorktreeBranchName(() => threadUUID); + + const base = yield* resolveWorktreeBase({ + cwd: project.workspaceRoot, + baseRef: t3Context.baseRef, + }); + + const gitWorktree = yield* gitWorkflowService + .createWorktree({ + cwd: project.workspaceRoot, + refName: base.refName, + ...(base.baseRefName !== null + ? { + baseRefName: base.baseRefName, + } + : {}), + newRefName: branchName, + path: null, + // we run setup scripts later + deferDependencyInstall: true, + }) + .pipe(orFail("Could not create the T3 worktree")); + + yield* orchestrationEngineService + .dispatch( + OrchestrationCommand.make({ + type: "thread.create", + branch: gitWorktree.worktree.refName, + worktreePath: gitWorktree.worktree.path, + threadId: threadId, + title: title, + modelSelection: modelSelection, + commandId: commandId, + createdAt: createdAt, + projectId: project.id, + runtimeMode: "full-access", + interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, + }), + ) + .pipe( + /* + Removes the worktree, but deliberately not its temporary + branch: GitWorkflowService has no branch-delete operation + (branch retention is an invariant of the thread worktree + lifecycle — see WorktreeLifecycle.cleanupThreadWorktree), so + the orphaned `t3/wt-…` ref is an accepted leak. It is a + dangling ref to an existing commit and costs nothing beyond + ref-listing noise. + */ + Effect.onError(() => + gitWorkflowService + .removeWorktree({ + path: gitWorktree.worktree.path, + cwd: project.workspaceRoot, + /* We also want garbage collection, we cannot rely + on the directory to be pristine. + */ + force: true, + }) + .pipe( + Effect.catch((cleanupErr) => + Effect.logWarning( + "Failed to remove worktree after thread.create did not complete", + { + threadId, + path: gitWorktree.worktree.path, + cause: cleanupErr, + }, + ), + ), + ), + ), + orFail("Failed to create a T3 thread"), + ); + + yield* projectScriptRunner + .runForThread({ + threadId, + projectId: project.id, + projectCwd: project.workspaceRoot, + worktreePath: gitWorktree.worktree.path, + }) + .pipe( + Effect.catch((err) => + Effect.logWarning("NTBS thread setup script failed.", { + threadId, + cause: err, + }), + ), + ); + + return threadId; + }); + + /** + * Resumes one stored NTBS thread after the processor starts. + * + * Starts the original turn when it is missing, leaves active turns to the + * live event listener, or posts the outcome when a turn already finished. + */ + const recoverThread = ( + threadCreated: NTBS.ThreadCreated, + ): Effect.Effect => + Effect.gen(function* () { + const { threadId, userMessageId } = threadCreated.t3; + + const turn = yield* getTurn(threadId, userMessageId); + + if (!turn) { + yield* startT3Turn( + threadId, + userMessageId, + threadCreated.snapshot, + threadCreated.attachments, + ); + return; + } + + if (turn.state === "pending" || turn.state === "running") { + return; + } + + yield* ensureUniqueOutcome( + userMessageId, + Effect.gen(function* () { + const currentRecord = yield* adapter + .findByThreadId(threadId) + .pipe(orFail("Failed reloading the NTBS lifecycle during recovery")); + + if (currentRecord.state === "thread.response.posted") { + markResponsePosted(userMessageId); + return; + } + + const response = yield* resolveT3Outcome(threadId, userMessageId); + if (response !== null) { + yield* postResponse(currentRecord, response); + } + }), + ); + }); + + /* + Handles an external request in this order: + + 1. Ask the adapter whether this platform request already has a recorded + `ThreadCreated` or `ResponsePosted`. + If yes - stop. . If no - continue + 2. Create the worktree and T3 thread. + 3. Generate the first user message ID and record it with ThreadCreated. + 4. Start the first T3 turn with that message ID, the snapshot, and attachments. + 5. Attempt to post the acknowledgement independently. + */ + const process = (request: NTBS.Request, t3Context: T3Context) => + Effect.gen(function* () { + /* + In-flight dedup first. We check if the processor is *currently* + working on this very request: it's being worked right now. + Later we check for the *durable* dedup: are we receiving a request + for work that has *already* completed. + */ + const key = request.sourceUri; + + const isBeingWorkedNow = inFlightRequests.has(key); + if (isBeingWorkedNow) { + yield* Effect.logDebug("NTBS request already being worked on; dropping duplicate", { + key, + }); + return; + } + inFlightRequests.add(key); + + yield* Effect.gen(function* () { + // durable dedup + const existingRequest = yield* adapter + .findByRequest(request) + .pipe(orFail("Error getting the existing request in process")); + + if (existingRequest) { + return; + } + // create the worktree and T3 thread + const threadId = yield* createT3Thread(t3Context); + + // generate the first user message ID and record it with ThreadCreated + const userMessageId = MessageId.make(yield* randomUUID); + + const threadCreated: NTBS.ThreadCreated = { + ...request, + state: "thread.created", + t3: { + threadId, + userMessageId, + }, + }; + + yield* adapter + .save(threadCreated) + .pipe(orFail("Failed to record the created NTBS thread")); + + // Start the first T3 turn with that message Id, the snapshot and attachments + yield* startT3Turn(threadId, userMessageId, request.snapshot, request.attachments); + + yield* adapter.acknowledge(threadCreated).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed posting the NTBS acknowledgement", { + userMessageId, + threadId, + cause, + }), + ), + ); + }).pipe(Effect.ensuring(Effect.sync(() => inFlightRequests.delete(key)))); + }); + + const consumeT3Events = Stream.runForEach( + orchestrationEngineService.streamDomainEvents, + (event) => + processT3Event(event).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed processing T3 event for NTBS", { + eventType: event.type, + cause, + }), + ), + ), + ); + + const recoverStoredThreads = adapter.loadThreadsAwaitingResponse.pipe( + orFail("Failed loading NTBS threads awaiting a response"), + Effect.flatMap((threads) => + Effect.forEach( + threads, + (threadCreated) => + recoverThread(threadCreated).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed recovering an NTBS thread", { + threadId: threadCreated.t3.threadId, + userMessageId: threadCreated.t3.userMessageId, + cause, + }), + ), + ), + { discard: true }, + ), + ), + Effect.catch((cause) => + Effect.logError("Failed starting NTBS thread recovery", { + cause, + }), + ), + ); + + const run = Effect.scoped( + Effect.gen(function* () { + yield* consumeT3Events.pipe(Effect.forkScoped({ startImmediately: true })); + yield* recoverStoredThreads; + return yield* Effect.never; + }), + ); + + return { + process, + run, + }; + }); diff --git a/apps/server/src/ntbs/processor.test.ts b/apps/server/src/ntbs/processor.test.ts new file mode 100644 index 000000000000..ff8cb263fd0f --- /dev/null +++ b/apps/server/src/ntbs/processor.test.ts @@ -0,0 +1,213 @@ +import { describe, it } from "@effect/vitest"; +import { NodeServices } from "@effect/platform-node"; +import { + OrchestrationProjectShell, + ProviderInstanceId, + ThreadId, + type OrchestrationCommand, + type OrchestrationEvent, +} from "@t3tools/contracts"; +import { DateTime, Deferred, Effect, Layer, PubSub, Stream } from "effect"; +import { makeNTBSAdapterTag, ThreadNotFound, type NTBSAdapter } from "./adapter.ts"; +import { makeNTBSProcessor, makeNTBSProcessorTag } from "./t3gateway.ts"; +import { OrchestrationEngineService } from "../orchestration/Services/OrchestrationEngine.ts"; +import { ProjectionSnapshotQuery } from "../orchestration/Services/ProjectionSnapshotQuery.ts"; +import { ProjectionTurnRepository } from "../persistence/Services/ProjectionTurns.ts"; +import { ProjectSetupScriptRunner } from "../project/ProjectSetupScriptRunner.ts"; +import { createAdapterRequest, createGitLayerMock } from "./test-helpers.ts"; +import { some } from "effect/Option"; + +const TestAdapter = makeNTBSAdapterTag("ntbs/TestAdapter"); + +const makeTestAdapter = Effect.gen(function* () { + const eventReceived = yield* Deferred.make(); + + const service: NTBSAdapter = { + save: () => Effect.void, + acknowledge: () => Effect.succeed("acknowledgement id"), + postResponse: () => Effect.succeed("response id"), + findByRequest: () => Effect.succeed(null), + findMatchingResponseMessage: () => Effect.succeed(null), + findByThreadId: (threadId) => + Effect.gen(function* () { + yield* Deferred.succeed(eventReceived, threadId); + return yield* new ThreadNotFound(); + }), + loadThreadsAwaitingResponse: Effect.succeed([]), + }; + + return { + eventReceived, + layer: Layer.succeed(TestAdapter, service), + }; +}); + +const TestProcessor = makeNTBSProcessorTag("ntbs/TestProcessor"); + +const makeTestOrchestrationEngine = Effect.gen(function* () { + const domainEvents = yield* PubSub.unbounded(); + const commands: OrchestrationCommand[] = []; + let sequence = 0; + + const service = OrchestrationEngineService.of({ + readEvents: () => Stream.empty, + dispatch: (command) => + Effect.sync(() => { + commands.push(command); + return { sequence: ++sequence }; + }), + streamDomainEvents: Stream.fromPubSub(domainEvents), + latestSequence: Effect.sync(() => sequence), + }); + + return { + layer: Layer.succeed(OrchestrationEngineService, service), + commands, + publish: (event: OrchestrationEvent) => PubSub.publish(domainEvents, event).pipe(Effect.asVoid), + }; +}); + +/* +processes a new request into a T3 thread, starts its first turn, persists lifecycle state, and posts an acknowledgement + + Assert that it: + + - fetches and resolves the requested base ref; + - creates the worktree; + - dispatches thread.create, then thread.turn.start; + - preserves the snapshot and attachments; + - saves thread.created with the generated thread/message IDs; + - runs setup; + - posts the acknowledgement; + - does not duplicate work. + + TODO: Add a recovery-path test for a stored thread with no matching projected + turn; recovery should start the original turn and monitor it. +*/ + +/* + TODO: Test processor-owned exchange serialization thoroughly. + + `process` first checks the repository and only then plans and persists a new + `RequestClaimed`. Without serialization, two concurrent deliveries carrying + the same `sourceUri` can both observe a missing exchange, mint different T3 + coordinates, and provision competing threads. Repository uniqueness alone + does not make that read-then-write sequence atomic. + + The processor should therefore serialize all work for one `sourceUri` while + allowing unrelated exchanges to run concurrently. A duplicate must wait for + the current caller and then perform the repository lookup again. It must not + merely be dropped: if the first caller fails before persisting its claim, the + waiting caller must get an opportunity to claim the request. + + Cover at least these cases using `Deferred` gates rather than sleeps: + + - Two simultaneous successful deliveries with the same `sourceUri`: block + the first during planning or persistence, start the second, and prove that + only one plan, claim, thread, turn, and acknowledgement are produced. Once + the first finishes, the second must re-read the repository and return as a + no-op. + - The first same-source caller fails before `RequestClaimed` is persisted: + the queued caller must acquire the lock afterward, observe no exchange, + and successfully claim and advance it. + - The first caller fails after persisting `RequestClaimed`: the queued caller + must observe the durable claim and return without planning new coordinates + or trying to repair the exchange. + - Different `sourceUri`s: block one request and prove that another request can + still plan and advance. The lock must be keyed, not global. + - Cancellation or interruption while holding the lock: the permit must be + released and the next caller must proceed. + - Cancellation or interruption while waiting for the lock: caller tracking + must be cleaned up without deleting a lock still used by another caller. + - Lock cleanup after the last caller exits, on both success and failure. The + lock map must not retain every `sourceUri` seen during the server lifetime. + - A platform redelivery racing with startup recovery or T3 thread activity: + every entry point must use the same source-keyed lock so stale state cannot + overwrite a newer transition and provisioning, turn start, or reply posting + cannot happen twice. + + Assert observable behavior rather than internal lock implementation wherever + possible. Count gateway, adapter, and repository calls; capture the exact T3 + coordinates and persisted states; and prove queued fibers are still blocked + by polling their completion before releasing each `Deferred` gate. +*/ + +/* + The processor's direct requirements are mocked one by one with `Layer.mock`: + a method left out simply dies if the test path reaches it, so each test only + fills in what it actually exercises. +*/ +const makeTestProcessorLive = ( + orchestrationEngine: Layer.Layer, + adapter: Layer.Layer, +) => { + const gitLayer = createGitLayerMock(); + return { + layer: Layer.effect(TestProcessor, makeNTBSProcessor(TestAdapter)).pipe( + Layer.provide(adapter), + Layer.provide(orchestrationEngine), + Layer.provide( + Layer.mock(ProjectionSnapshotQuery)({ + getProjectShellById: (projectId) => + Effect.gen(function* () { + const now = yield* DateTime.now.pipe(Effect.map(DateTime.formatIso)); + return some( + OrchestrationProjectShell.make({ + createdAt: now, + id: projectId, + title: "project title", + workspaceRoot: "workspaceRoot", + defaultModelSelection: { + model: "gpt-does-not-exist-v2", + instanceId: ProviderInstanceId.make("gpt-does-not-exist-v2"), + }, + updatedAt: now, + scripts: [], + }), + ); + }), + }), + ), + Layer.provide(Layer.mock(ProjectionTurnRepository)({})), + Layer.provide(gitLayer.layer), + Layer.provide( + Layer.mock(ProjectSetupScriptRunner)({ + runForThread: (_) => + Effect.sync(() => { + return { status: "no-script" }; + }), + }), + ), + // Provides Crypto (plus FileSystem/Path) for UUID generation. + Layer.provide(NodeServices.layer), + ), + gitCalls: gitLayer.gitCalls, + }; +}; + +const createProcessor = Effect.gen(function* () { + const testEngine = yield* makeTestOrchestrationEngine; + const testAdapter = yield* makeTestAdapter; + + const processorLive = makeTestProcessorLive(testEngine.layer, testAdapter.layer); + + const processor = yield* TestProcessor.pipe(Effect.provide(processorLive.layer)); + return { + processor, + testEngine, + testAdapter, + gitCalls: processorLive.gitCalls, + }; +}); + +describe("Basic happy case", () => { + it.effect("receives a T3 event", () => + Effect.gen(function* () { + const { processor } = yield* createProcessor; + + const request = createAdapterRequest("someRequestId"); + + yield* processor.process(request.request, request.t3Context); + }), + ); +}); diff --git a/apps/server/src/ntbs/processor.ts b/apps/server/src/ntbs/processor.ts new file mode 100644 index 000000000000..ed50310c57f6 --- /dev/null +++ b/apps/server/src/ntbs/processor.ts @@ -0,0 +1,422 @@ +import { type ProjectId, type ThreadId } from "@t3tools/contracts"; +import * as NTBS from "./exchange.ts"; +import { 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. + +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. +*/ + +/** + * Describes _where_ the T3 works goes. Necessary for creating worktrees, threads and starting turns. + */ +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. + * + * Set by the platform-specific inbound code. + */ + readonly startBranchName: string; +}; + +export class NTBSProcessorError extends Data.TaggedError("NTBSProcessorError")<{ + reason: string; + cause: unknown; +}> {} + +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 exchange is claimed and under way, not once the request is answered: the reply is posted later, when T3 reports the turn finished. + * + * Idempotent per `sourceUri`: a redelivery of an already-claimed request is a no-op, whatever state that exchange has reached. Concurrent deliveries of the same request are serialized, so only the first claims it. + */ + readonly process: ( + request: NTBS.Request, + t3Target: 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 only moves when its T3 thread does. + * + * Never returns. It has no error channel: a failure on one exchange is logged and the next event is still processed. + */ + 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. + */ +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 createReplyFailure = (failure: { + readonly reason: string; + readonly cause: unknown; + }): NTBS.ReplyFailure => ({ + type: "failure", + text: failure.reason, + cause: failure.cause, + }); + + const persist = (state: State) => + repo.upsert(state).pipe(orFail("Failed to persist the exchange state"), Effect.as(state)); + + 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 processRequestClaimed = Effect.fn("NTBSProcessor.processRequestClaimed")(function* ( + state: NTBS.RequestClaimed, + ) { + const context = yield* t3 + .getThreadStatus(state) + .pipe(orFail("Failed to get the T3 thread status")); + const decision = NTBS.fromRequestClaimed(state, context); + + switch (decision.type) { + case "provision-thread": { + const rejection = yield* t3.provisionThread(state).pipe( + Effect.as(null), + Effect.catchTag("T3Rejected", (error) => Effect.succeed(error)), + orFail("Failed to provision the T3 thread"), + ); + + if (rejection !== null) { + const next = yield* persist(NTBS.toReplyPending(state, createReplyFailure(rejection))); + return transitionedTo(next); + } + + break; + } + + case "record-thread-created": + break; + } + + const next = yield* persist(NTBS.toThreadCreated(state)); + yield* adapter.acknowledge(next).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed to post the NTBS acknowledgement", { + sourceUri: next.sourceUri, + threadId: next.t3.threadId, + cause, + }), + ), + ); + return transitionedTo(next); + }); + + const processThreadCreated = Effect.fn("NTBSProcessor.processThreadCreated")(function* ( + state: NTBS.ThreadCreated, + ) { + const context = yield* t3 + .getTurnStatus(state) + .pipe(orFail("Failed to get the T3 turn status")); + const decision = NTBS.fromThreadCreated(state, context); + + switch (decision.type) { + case "start-turn": { + const rejection = yield* t3.startTurn(state).pipe( + Effect.as(null), + Effect.catchTag("T3Rejected", (error) => Effect.succeed(error)), + orFail("Failed to start the T3 turn"), + ); + + if (rejection !== null) { + const next = yield* persist(NTBS.toReplyPending(state, createReplyFailure(rejection))); + return transitionedTo(next); + } + + return unchanged; + } + + case "wait": + return unchanged; + + case "record-reply-pending": { + const next = yield* persist(NTBS.toReplyPending(state, decision.reply)); + return transitionedTo(next); + } + } + }); + + const processReplyPending = Effect.fn("NTBSProcessor.processReplyPending")(function* ( + state: NTBS.ReplyPending, + ) { + 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); + + switch (decision.type) { + 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) + : NTBS.toUndeliverable(state, delivery.cause), + ); + return transitionedTo(next); + } + + case "record-reply-posted": { + const next = yield* persist(NTBS.toReplyPosted(state, decision.replySourceUri)); + 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-claimed": + result = yield* processRequestClaimed(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: 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 get the t3 coordinates, save them and advance the exchange. + */ + + const existing = yield* repo + .findBySourceUri(request.sourceUri) + .pipe(orFail("Failed to find the exchange for the platform request")); + + if (existing !== null) { + return; + } + + const coordinates = yield* t3 + .planCoordinates(t3Target.projectId, t3Target.startBranchName) + .pipe(orFail("Failed to plan the T3 work")); + const claimed = NTBS.makeRequestClaimed(request, coordinates); + yield* persist(claimed); + yield* advanceExchange(claimed); + }), + ); + }); + + 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.catch((cause) => + Effect.logWarning("Failed to advance an exchange after T3 thread activity", { + threadId, + cause, + }), + ), + ), + ); + + const resumeNonTerminalExchanges = repo.findNonTerminalExchanges.pipe( + orFail("Failed to load non-terminal exchanges"), + Effect.flatMap((exchanges) => + Effect.forEach( + exchanges, + (exchange) => + advanceSavedExchange(exchange.sourceUri).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed to recover an exchange", { + sourceUri: exchange.sourceUri, + threadId: exchange.t3.threadId, + cause, + }), + ), + ), + { discard: true }, + ), + ), + Effect.catch((cause) => + Effect.logError("Failed to start exchange recovery", { + cause, + }), + ), + ); + + const run = Effect.scoped( + Effect.gen(function* () { + yield* subscribeToThreadActivity.pipe(Effect.forkScoped({ startImmediately: true })); + yield* resumeNonTerminalExchanges; + return yield* Effect.never; + }), + ); + + return { + process, + run, + }; + }); diff --git a/apps/server/src/ntbs/processor2.test.ts b/apps/server/src/ntbs/processor2.test.ts new file mode 100644 index 000000000000..57fea161f4ec --- /dev/null +++ b/apps/server/src/ntbs/processor2.test.ts @@ -0,0 +1,281 @@ +import { assert, describe, it } from "@effect/vitest"; +import { NodeServices } from "@effect/platform-node"; +import { + CommandId, + EventId, + MessageId, + ProjectId, + ThreadId, + type OrchestrationCommand, + type OrchestrationEvent, +} from "@t3tools/contracts"; +import { Context, DateTime, Effect, Layer, PubSub, Queue, Stream } from "effect"; +import { + makeNTBSAdapterTag, + ThreadNotFound, + type NTBSAdapter, + type NTBSResponse, +} from "./adapter.ts"; +import type { Request, Exchange, ThreadCreated } from "./exchange.ts"; +import { makeNTBSProcessor, makeNTBSProcessorTag } from "./t3gateway.ts"; +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"; + +/* + Harness layout: + + The fakes' observable state lives in dedicated context services (TestEngine, + TestAdapterState). The real service tags (OrchestrationEngineService, the + adapter) get thin layers derived from that state. `Layer.provideMerge` keeps + the state services visible to the tests, so a test pulls its handles from + context instead of building fakes inline and closing over them. + + Everything is provided per test with `Effect.provide(Harness)`: the processor + holds internal mutable state (dedup keys, outcome locks, monitor baselines), + so a shared `layer(...)` block would leak state across tests. +*/ + +class TestEngine extends Context.Service< + TestEngine, + { + /** Every command the processor dispatched, in order. */ + readonly commands: Array; + /** Emits a domain event as if the orchestration engine produced it. */ + readonly publish: (event: OrchestrationEvent) => Effect.Effect; + readonly domainEvents: PubSub.PubSub; + } +>()("t3/ntbs/processor2.test/TestEngine") { + static readonly layer = Layer.effect( + TestEngine, + Effect.gen(function* () { + const domainEvents = yield* PubSub.unbounded(); + return { + commands: [], + domainEvents, + publish: (event: OrchestrationEvent) => + PubSub.publish(domainEvents, event).pipe(Effect.asVoid), + }; + }), + ); +} + +const OrchestrationEngineFromTestEngine = Layer.effect( + OrchestrationEngineService, + Effect.gen(function* () { + const engine = yield* TestEngine; + let sequence = 0; + + return OrchestrationEngineService.of({ + readEvents: () => Stream.empty, + dispatch: (command) => + Effect.sync(() => { + engine.commands.push(command); + return { sequence: ++sequence }; + }), + streamDomainEvents: Stream.fromPubSub(engine.domainEvents), + latestSequence: Effect.sync(() => sequence), + }); + }), +); + +class TestAdapterState extends Context.Service< + TestAdapterState, + { + /** Lifecycle records keyed by T3 thread — seed before acting, inspect after. */ + readonly records: Map; + readonly postedAcks: Array; + readonly postedResponses: Array<{ + readonly record: ThreadCreated; + readonly response: NTBSResponse; + }>; + /** One entry per findByThreadId call; taking from it awaits event delivery. */ + readonly threadLookups: Queue.Queue; + } +>()("t3/ntbs/processor2.test/TestAdapterState") { + static readonly layer = Layer.effect( + TestAdapterState, + Effect.gen(function* () { + return { + records: new Map(), + postedAcks: [], + postedResponses: [], + threadLookups: yield* Queue.unbounded(), + }; + }), + ); +} + +const TestAdapter = makeNTBSAdapterTag("ntbs/TestAdapter"); + +const AdapterFromState = Layer.effect( + TestAdapter, + Effect.gen(function* () { + const state = yield* TestAdapterState; + + const adapter: NTBSAdapter = { + save: (lifecycleEvent) => + Effect.sync(() => { + state.records.set(lifecycleEvent.t3.threadId, lifecycleEvent); + }), + acknowledge: (record) => + Effect.sync(() => { + state.postedAcks.push(record); + }), + postResponse: (record, response) => + Effect.sync(() => { + state.postedResponses.push({ record, response }); + return `response-${state.postedResponses.length}`; + }), + findByRequest: (request) => + Effect.sync( + () => + [...state.records.values()].find((record) => record.sourceUri === request.sourceUri) ?? + null, + ), + findMatchingResponseMessage: () => Effect.succeed(null), + findByThreadId: (threadId) => + Queue.offer(state.threadLookups, threadId).pipe( + Effect.flatMap(() => { + const record = state.records.get(threadId); + return record === undefined + ? Effect.fail(new ThreadNotFound()) + : Effect.succeed(record); + }), + ), + loadThreadsAwaitingResponse: Effect.sync(() => + [...state.records.values()].filter( + (record): record is ThreadCreated => record.state === "thread.created", + ), + ), + }; + + return adapter; + }), +); + +const TestProcessor = makeNTBSProcessorTag("ntbs/TestProcessor"); + +const Harness = Layer.effect(TestProcessor, makeNTBSProcessor(TestAdapter)).pipe( + Layer.provide(AdapterFromState), + Layer.provide(OrchestrationEngineFromTestEngine), + Layer.provideMerge(TestAdapterState.layer), + Layer.provideMerge(TestEngine.layer), + Layer.provide(Layer.mock(ProjectionSnapshotQuery)({})), + Layer.provide(Layer.mock(ProjectionTurnRepository)({})), + Layer.provide(Layer.mock(GitWorkflowService)({})), + Layer.provide(Layer.mock(ProjectSetupScriptRunner)({})), + // Provides Crypto (plus FileSystem/Path) for UUID generation. + Layer.provide(NodeServices.layer), +); + +const sessionSetEvent = (threadId: ThreadId): Effect.Effect => + Effect.map(DateTime.now, (nowDateTime) => { + const now = DateTime.formatIso(nowDateTime); + return { + type: "thread.session-set", + eventId: EventId.make(`event-for-${threadId}`), + occurredAt: now, + commandId: CommandId.make("someCommand"), + aggregateId: threadId, + aggregateKind: "thread", + sequence: 0, + causationEventId: EventId.make("someOtherEvent"), + correlationId: null, + metadata: {}, + payload: { + threadId, + session: { + threadId, + status: "running", + providerName: null, + runtimeMode: "full-access", + activeTurnId: null, + lastError: null, + updatedAt: now, + }, + }, + }; + }); + +const makeRequest = (platformMessageId: string): Request => ({ + sourceUri: platformMessageId, + snapshot: "please look into this", + attachments: [], +}); + +const recordedThread = (request: Request, threadId: ThreadId): ThreadCreated => ({ + ...request, + state: "thread.created", + t3: { threadId, userMessageId: MessageId.make(`message-for-${threadId}`) }, +}); + +describe("NTBSProcessor (layer harness)", () => { + it.effect("delivers T3 session events to the adapter and ignores unknown threads", () => + Effect.gen(function* () { + const engine = yield* TestEngine; + const adapterState = yield* TestAdapterState; + const processor = yield* TestProcessor; + + yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + + const threadId = ThreadId.make("unknown-thread"); + yield* engine.publish(yield* sessionSetEvent(threadId)); + + // Taking the recorded lookup proves the event crossed the stream into + // the adapter; an unknown thread must produce no further activity. + assert.strictEqual(yield* Queue.take(adapterState.threadLookups), threadId); + assert.deepStrictEqual(adapterState.postedResponses, []); + assert.deepStrictEqual(engine.commands, []); + }).pipe(Effect.provide(Harness)), + ); + + it.effect("drops a redelivered request that already has a recorded thread", () => + Effect.gen(function* () { + const engine = yield* TestEngine; + const adapterState = yield* TestAdapterState; + const processor = yield* TestProcessor; + + const request = makeRequest("platform-message-1"); + const threadId = ThreadId.make("existing-thread"); + adapterState.records.set(threadId, recordedThread(request, threadId)); + + yield* processor.process(request, { + projectId: ProjectId.make("some-project"), + startBranchName: "main", + }); + + // Durable dedup: no new thread or turn, no second acknowledgement. + assert.deepStrictEqual(engine.commands, []); + assert.deepStrictEqual(adapterState.postedAcks, []); + }).pipe(Effect.provide(Harness)), + ); + + it.effect("records an already-posted response without posting again", () => + Effect.gen(function* () { + const engine = yield* TestEngine; + const adapterState = yield* TestAdapterState; + const processor = yield* TestProcessor; + + const request = makeRequest("platform-message-2"); + const threadId = ThreadId.make("answered-thread"); + adapterState.records.set(threadId, { + ...recordedThread(request, threadId), + state: "thread.response.posted", + responseMessageId: "already-posted", + }); + + yield* processor.run.pipe(Effect.forkChild({ startImmediately: true })); + yield* engine.publish(yield* sessionSetEvent(threadId)); + + // The processor loads the record twice: once to route the event and once + // under the outcome lock before deciding what to post. + yield* Queue.take(adapterState.threadLookups); + yield* Queue.take(adapterState.threadLookups); + + assert.deepStrictEqual(adapterState.postedResponses, []); + }).pipe(Effect.provide(Harness)), + ); +}); diff --git a/apps/server/src/ntbs/t3gateway.test.ts b/apps/server/src/ntbs/t3gateway.test.ts new file mode 100644 index 000000000000..2ed4080e7e57 --- /dev/null +++ b/apps/server/src/ntbs/t3gateway.test.ts @@ -0,0 +1,163 @@ +import { describe, it, expect } from "@effect/vitest"; +import { t3GatewayLive, T3Gateway } from "./t3gateway.ts"; +import { Effect, Layer, Option } 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 { Crypto } from "effect/Crypto"; +import { OrchestrationProjectShell, ProjectId } from "@t3tools/contracts"; + +/* +T3 Gateway Live is a layer, a constructor of a dependency. + +But it needs its own dependencies and we need to provide/mock them. + +Let's try using Layer.mock to provide those. +*/ + +const CryptoMock = Layer.mock(Crypto, { + "~effect/platform/Crypto": "~effect/platform/Crypto", + randomUUIDv4: Effect.succeed("randomUUIDv4"), + nextDoubleUnsafe: () => 0, + nextIntUnsafe: () => 0, +}); + +const OrchestrationEngineServiceMock = Layer.mock(OrchestrationEngineService, {}); + +type PSQMInput = { + getProjectShellById: + | { + success: Partial; + } + | { failure: unknown }; +}; + +const createPSQM = (input?: PSQMInput) => { + return Layer.mock(ProjectionSnapshotQuery, { + getProjectShellById: (projectId) => + Effect.option( + input && "failure" in input.getProjectShellById + ? Effect.fail(String(input.getProjectShellById.failure)) + : Effect.succeed({ + id: projectId, + workspaceRoot: "root", + title: "project-title", + createdAt: new Date().toUTCString(), + updatedAt: new Date().toUTCString(), + defaultModelSelection: null, + scripts: [], + ...(input && + "success" in input.getProjectShellById && { ...input.getProjectShellById.success }), + }), + ), + }); +}; + +const ProjectionTurnRepositoryMock = Layer.mock(ProjectionTurnRepository, {}); + +// TODO: Can't we simplify it by leveraging default values in params? +// we can pass default arguments in JS +const createGitWorkflowServiceMock = (input?: { + remoteExists?: boolean; + resolvedRemoteSha?: string; +}) => + Layer.mock(GitWorkflowService, { + remoteExists: () => Effect.succeed(!input ? true : !!input.remoteExists), + fetchRemote: () => Effect.void, + resolveRemoteTrackingCommit: (_input) => + Effect.succeed({ + commitSha: input?.resolvedRemoteSha ?? "sha123", + remoteRefName: "remoteRefName", + }), + }); + +const ProjectSetupScriptRunnerMock = Layer.mock(ProjectSetupScriptRunner, {}); + +const t3Dependencies = (input?: { + pqsm?: { + getProjectShellById?: {}; + }; + gwfs?: { + remoteExists?: boolean; + }; +}) => + Layer.mergeAll( + OrchestrationEngineServiceMock, + createPSQM({ + getProjectShellById: { success: {} }, + }), + ProjectSetupScriptRunnerMock, + createGitWorkflowServiceMock(input?.gwfs), + ProjectionTurnRepositoryMock, + CryptoMock, + ); + +describe("T3Gateway", () => { + describe("planCoordinates", () => { + describe("successful planning", () => { + const t3GatewayTest = t3GatewayLive.pipe( + Layer.provide( + t3Dependencies({ + gwfs: { remoteExists: true }, + }), + ), + ); + + it.layer(t3GatewayTest)((it) => + it.effect("pins the selected branch to the commit fetched from origin", () => + Effect.gen(function* () { + // TODO: Continue from here + const t3Gateway = yield* T3Gateway; + + const projectId = ProjectId.make("test-1"); + + /* + 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 retryable T3GatewayError + - 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 + */ + const coordinates = yield* t3Gateway.planCoordinates(projectId, "main"); + + expect(coordinates).toEqual({ + projectId, + startBranchName: "main", + startCommitSha: "sha123", + threadId: "randomUUIDv4", // why? + userMessageId: "randomUUIDv4", // why? + worktreeBranchName: "t3code/add4", // why is it this specific value? + }); + }), + ), + ); + + it.todo( + "returns the project, branch and commit with distinct thread and message IDs and a worktree branch derived from the thread ID", + ); + }); + + describe("rejected planning", () => { + it.todo("rejects a project that does not exist without performing provisioning work"); + + it.todo("rejects a project whose repository has no origin remote"); + + it.todo( + "rejects a selected branch that is absent after a successful fetch without performing provisioning work", + ); + }); + + describe("operational failures", () => { + it.todo("fails retryably when the project lookup fails"); + + it.todo("fails retryably when checking for the origin remote fails"); + + it.todo("fails retryably when fetching origin fails"); + + it.todo("fails retryably when the exchange IDs cannot be minted"); + }); + }); +}); diff --git a/apps/server/src/ntbs/t3gateway.ts b/apps/server/src/ntbs/t3gateway.ts new file mode 100644 index 000000000000..011a458b5bd3 --- /dev/null +++ b/apps/server/src/ntbs/t3gateway.ts @@ -0,0 +1,292 @@ +/* +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 { MessageId, type ProjectId, ThreadId } from "@t3tools/contracts"; +import type * as NTBS from "./exchange.ts"; +import { Context, Crypto, Data, Effect, Layer, Option, 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 { interrupt } from "effect/Cause"; + +/* + 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. + + TODO: Better description of the whole architecture. +*/ + +export class T3GatewayError extends Data.TaggedError("T3GatewayError")<{ + reason: string; + cause: unknown; +}> {} + +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; + +/** T3 will never accept this work; the processor records a failure reply. */ +export class T3Rejected extends Data.TaggedError("T3Rejected")<{ + reason: string; + cause: unknown; +}> {} + +/** A branch on `origin` and the commit it pointed at when it was resolved. */ +interface RemoteBranchTip { + readonly branchName: string; + readonly commitSha: string; +} + +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; + + readonly getThreadStatus: ( + state: NTBS.RequestClaimed, + ) => Effect.Effect; + + /** Reentrant: worktree, thread creation and setup scripts, each skipped if already done. */ + readonly provisionThread: ( + state: NTBS.RequestClaimed, + ) => Effect.Effect; + + /** Reports turn progress, interpreting a finished turn into a verbatim `Reply`. */ + readonly getTurnStatus: ( + state: NTBS.ThreadCreated, + ) => Effect.Effect; + + 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 = (reason: string) => + Effect.mapError( + (cause: unknown) => + new T3GatewayError({ + reason, + cause, + }), + ); + + const orReject = (reason: string) => + Effect.mapError( + (cause: unknown) => + new T3Rejected({ + reason, + cause, + }), + ); + + const projectionSnapshotQuery = yield* ProjectionSnapshotQuery; + + /* + 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("Could not load project " + projectId), + Effect.flatMap( + Option.match({ + onSome: Effect.succeed, + onNone: () => + Effect.fail( + new T3Rejected({ + 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("Could not check whether the remote 'origin' exists"), + Effect.filterOrFail( + (exists) => exists, + () => + new T3Rejected({ + 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("Could not fetch origin. try again")); + + // TODO: Is this correct? It doesn't look like this reads the tracking ref locally. + /* + This reads the tracking ref locally, with no network involved. The two calls above + already proved the repository is usable and the remote reachable, so what is left + fails only when the branch is absent from origin. A repository broken badly enough + to fail the read some other way is reported as a missing branch; the trade buys us + a permanent rejection for the case that actually happens, a mistyped branch name. + */ + return yield* gitWorkflowService + .resolveRemoteTrackingCommit({ + cwd, + refName: startBranchName, + fallbackRemoteName: "origin", + }) + .pipe( + Effect.map((resolved) => ({ + branchName: startBranchName, + commitSha: resolved.commitSha, + })), + ) + .pipe(orReject("Branch '" + startBranchName + "' does not exist on origin")); + }); + + const crypto = yield* Crypto.Crypto; + const randomUUID = crypto.randomUUIDv4.pipe(orFail("Failed creating a UUID v4")); + + 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; + }); + + const getThreadStatus = ( + _state: NTBS.RequestClaimed, + ): Effect.Effect => + Effect.succeed({ + thread: "missing", + }); + + const getTurnStatus = ( + _state: NTBS.ThreadCreated, + ): Effect.Effect => + Effect.succeed({ + turn: "missing", + }); + + const startTurn = (_state: NTBS.ThreadCreated) => Effect.void; + + const threadActivity = Stream.never; + + const provisionThread = ( + _state: NTBS.RequestClaimed, + ): Effect.Effect => Effect.void; + + return { + startTurn, + getTurnStatus, + threadActivity, + planCoordinates, + getThreadStatus, + provisionThread, + }; + }, +); + +export const t3GatewayLive = Layer.effect(T3Gateway, T3GatewayLive); diff --git a/apps/server/src/ntbs/test-helpers.ts b/apps/server/src/ntbs/test-helpers.ts new file mode 100644 index 000000000000..89c4ae5c77a6 --- /dev/null +++ b/apps/server/src/ntbs/test-helpers.ts @@ -0,0 +1,270 @@ +import { Context, Effect, Layer, PubSub, Queue, Stream } from "effect"; +import { GitWorkflowService } from "../git/GitWorkflowService.ts"; +import { + OrchestrationCommand, + OrchestrationEvent, + ProjectId, + ThreadId, + VcsCreateWorktreeResult, +} from "@t3tools/contracts"; +import type { Request, Exchange, ThreadCreated } from "./exchange.ts"; +import type { T3Context } from "./t3gateway.ts"; +import { OrchestrationEngineService } from "../orchestration/Services/OrchestrationEngine.ts"; +import { makeNTBSAdapterTag, ThreadNotFound, type NTBSResponse } from "./adapter.ts"; + +export const createGitLayerMock = () => { + const gitCalls = { + createWorktree: [] as unknown[], + fetchRemote: [] as unknown[], + resolveRemoteTrackingCommit: [] as unknown[], + removeWorktree: [] as unknown[], + }; + + const layer = Layer.mock(GitWorkflowService, { + fetchRemote: (input) => + Effect.sync(function () { + gitCalls.fetchRemote.push(input); + }), + resolveRemoteTrackingCommit: (input) => + Effect.sync(() => { + gitCalls.resolveRemoteTrackingCommit.push(input); + + return { commitSha: "input-sha", remoteRefName: input.refName }; + }), + createWorktree: (input) => + Effect.sync(() => { + gitCalls.createWorktree.push(input); + return VcsCreateWorktreeResult.make({ + worktree: { + path: "createworktreepath", + refName: input.refName, + }, + }); + }), + removeWorktree: (input) => + Effect.sync(() => { + gitCalls.removeWorktree.push(input); + + return; + }), + }); + + return { + gitCalls, + layer, + }; +}; + +export const createAdapterRequest = ( + uniqueId: string, +): { + request: Request; + t3Context: T3Context; +} => ({ + request: { + snapshot: "This is an ongoing discussion", + attachments: [], + sourceUri: uniqueId, + }, + t3Context: { + startBranchName: "fork/dev", + projectId: ProjectId.make("project"), + }, +}); + +/* + # Testing strategy + + At its core the processor provides the business logic implementation that interacts with external services. + + The two core *behavioral* boundaries that are coordinated by the processor are the: + + - T3 orchestration engine. Emits event that the processor reads, receives commands (such as `thread.create` or `thread.turn.start` from the processor. + + - Adapter. The software responsible for the external platform (such as Jira or Discord) integration. It records acknowledgements and responses, stores lifecycle records, and answers deduplication and thread lookup queries. + + The other boundaries that communicate with the processor are: + - ProjectionSnapshotQuery: reads project and thread state + - ProjectionTurnRepository: reads turn progress + - GitWorkflowService: fetches refs and creates worktrees + - ProjectSetupScriptRunner: runs project setup + + Focusing on the behavioral boundary allows to quickly test the happy cases. + + The test simulates T3 events entering the processor, and the commands that the processor sends to T3 via `TestT3Engine`. + + It inspects the acknowledgements, responses and lifecycle events of the adapter via `TestAdapter`. +*/ + +class TestEngine extends Context.Service< + TestEngine, + { + /** + * Every command the processor dispatched, in order. + */ + readonly commandsReceived: Array; + /** + * Emits a domain event as if the orchestration engine produced it. + */ + readonly publish: (event: OrchestrationEvent) => Effect.Effect; + + readonly domainEvents: PubSub.PubSub; + } +>()("t3/ntbs/test-helpers/TestEngine") { + static readonly layer = Layer.effect( + TestEngine, + Effect.gen(function* () { + /** + * In production `OrchestrationEngineService.streamDomainEvents` is the live + * feed of domain events the engine emits as it processes commands. + * + * It is a Stream. + * + * Here, `domainEvents` is the PubSub where events are published. + * + * The main purpose of this PubSub and related code is enabling tests to say "the engine just emitted event X for thread Y" without any real engine, persistence or provider session existing. + */ + const domainEvents = yield* PubSub.unbounded(); + return { + commandsReceived: [], + domainEvents, + /** + * The entry point for emulating orchestration engine published events in test. + * + * Call `TestEngine.publish`. + * It will publish the event to `domainEvents`. + * Then, the `OrchestrationEngineService` will stream that event out of `streamDomainEvents`, in the very same fashion the + */ + publish: (event: OrchestrationEvent) => PubSub.publish(domainEvents, event), + }; + }), + ); +} + +const OrchestrationEngineFromTestEngine = Layer.effect( + OrchestrationEngineService, + Effect.gen(function* () { + const engine = yield* TestEngine; + let sequence = 0; + + return OrchestrationEngineService.of({ + dispatch: (command) => + Effect.sync(() => { + engine.commandsReceived.push(command); + sequence += 1; + return { sequence }; + }), + latestSequence: Effect.sync(() => sequence), + readEvents: () => Stream.empty, + streamDomainEvents: Stream.fromPubSub(engine.domainEvents), + }); + }), +); + +// TODO: Continue from here +class TestAdapterState extends Context.Service< + TestAdapterState, + { + /** + * Lifecycle records keyed by T3 thread. + */ + readonly records: Map; + readonly postedAcks: Map; + readonly postedResponses: Map< + string, + { + readonly record: ThreadCreated; + readonly response: NTBSResponse; + } + >; + /** + * One entry per findByThreadId call; + * taking from it awaits event delivery. + */ + readonly threadLookups: Queue.Queue; + } +>()("t3/ntbs/test-helpers/TestAdapterState") { + static readonly layer = Layer.effect( + TestAdapterState, + Effect.gen(function* () { + return { + // lifecycleEvents: [], + records: new Map(), + postedAcks: new Map(), + postedResponses: new Map< + string, + { + readonly record: ThreadCreated; + readonly response: NTBSResponse; + } + >(), + threadLookups: yield* Queue.unbounded(), + }; + }), + ); +} + +const TestAdapter = makeNTBSAdapterTag("test/ntbs/TestAdapter"); + +const TestAdapterFromState = Layer.effect( + TestAdapter, + Effect.gen(function* () { + const adapterState = yield* TestAdapterState; + + return { + save: (event) => + Effect.sync(() => { + adapterState.records.set(event.t3.threadId, event); + }), + acknowledge: (state) => + Effect.sync(() => { + const acknowledgementId = `acknowledgementId-${adapterState.postedAcks.size}`; + adapterState.postedAcks.set(acknowledgementId, state); + return acknowledgementId; + }), + + postResponse: (state, response) => + Effect.sync(() => { + const messageId = `messageid-${adapterState.postedResponses.size}`; + adapterState.postedResponses.set(messageId, { + record: state, + response, + }); + return messageId; + }), + findByRequest: (request) => + Effect.sync(function () { + return ( + adapterState.records + .entries() + .map((el) => el[1]) + .find((entry) => { + return entry.sourceUri === request.sourceUri; + }) ?? null + ); + }), + findByThreadId: (threadId) => + Effect.suspend(() => { + const maybeRecord = adapterState.records.get(threadId); + + return maybeRecord ? Effect.succeed(maybeRecord) : new ThreadNotFound(); + }), + findMatchingResponseMessage: (state) => + Effect.sync(() => { + const maybeResponse = adapterState.postedResponses + .entries() + .find(([_id, posted]) => posted.record.sourceUri === state.sourceUri); + return maybeResponse ? maybeResponse[0] : null; + }), + loadThreadsAwaitingResponse: Effect.sync(() => { + const awaitingResponse: ThreadCreated[] = []; + adapterState.records.forEach((state) => { + if (state.state === "thread.created") { + awaitingResponse.push(state); + } + }); + return awaitingResponse; + }), + }; + }), +); 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/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..3b32bb5d6107 --- /dev/null +++ b/docs/planning/ntbs-todos.md @@ -0,0 +1,244 @@ +# NTBS exchange lifecycle + +**Status:** decided 2026-08-16 · supersedes the two-state `ExchangeState` + +The old model stored only `ThreadCreated | ResponsePosted` through a generic `save`, so the processor had to re-derive "what already happened" from adapter storage, T3 projections, process-local locks, and the external platform on every step. The settled design replaces it with a claimed, forward-only exchange machine with one reconciler. + +## Ports + +Three ports, one per thing the exchange has to touch. Each knows only what it wraps, and none of them knows about the others. + +- **Exchange repository** — the durable record of where each exchange got to. Stores, looks up by `sourceUri` or `threadId`, and lists the incomplete ones for startup recovery. Nothing in it reaches T3 or the platform. +- **Adapter** — the originating platform. Posts the acknowledgement and the reply, and answers with certainty whether its reply for this exchange is already there. The only piece that may parse a `sourceUri`. +- **T3 gateway** — T3 and the VCS lifecycle behind it. Plans the identifiers, provisions the thread and worktree, starts the turn, reports thread and turn status, and signals which threads have moved. + +The processor sits above them and holds the orchestration none of them have: it reads the state, asks the relevant port what is true now, decides, executes, and records the transition. It is the only writer of exchange state, and the only place the three ports meet. + +No leases and no multi-process machinery anywhere: the real deployment is one server process. How each port enforces the invariants is implementation, decided during the build. + +## States + +```text +RequestClaimed -> ThreadCreated -> ReplyPending -> ReplyPosted + \ + -> Undeliverable +``` + +- **`RequestClaimed`** — the platform inbound code admitted the request (trigger and actor checks passed) and the processor recorded the claim; from here the processor alone drives the exchange to a terminal state, and redeliveries change nothing. Carries the full request (`sourceUri`, snapshot, attachments), the T3 context (`projectId`, `baseRef`), and pre-minted planned IDs (`threadId`, `userMessageId`, branch name) so a cold start can redo provisioning without the original webhook and detect an already-created thread instead of duplicating it. +- **`ThreadCreated`** — the planned thread exists; the IDs are confirmed facts. No turn state is stored: turn existence and progress are T3-owned. +- **`ReplyPending`** — T3 reached a terminal outcome; the exact reply payload is stored verbatim so every posting attempt sends the same content. +- **`ReplyPosted`** — terminal. The platform accepted the reply; its message ID is stored. +- **`Undeliverable`** — terminal, entered only from `ReplyPending`: a finished reply exists but the platform definitively rejected delivery. Stores the undelivered payload and a serializable explanation. The tombstone keeps dedup intact and stops the processor from retrying forever. + +## Invariants + +- One exchange per `sourceUri`, for its whole life. Duplicate deliveries join it, never create another; they are pure dedup and trigger no repair. +- Forward-only lifecycle: moving backwards is an error, not a write. Failure may jump ahead to `ReplyPending`. +- States track delivery, not outcome quality. Answer, failure, or cancellation is data in the reply payload, never a state. Every exchange ends in `ReplyPosted` or `Undeliverable`. +- T3 stays authoritative for T3-owned facts — no `TurnStarted` or `AwaitingOutcome` copies in the stored exchange. +- Turn-start idempotency rests on `getTurn` being read-your-writes at reconcile time; provisioning recovery must treat worktree creation as reentrant (the branch may already exist from a pre-crash attempt). + +## Recovery + +An exchange can be cut in half by the server stopping: a thread was provisioned, but a turn never started, a reply was computed but not posted, etc. + +The stored state say how far did the exchange go, so work can be easily resumed. + +On startup the processor loads every incomplete exchange and continues it. While running, T3 events tell it when a turn has finished so the reply can be posted. + +An optional, additional recovery method can be envisioned by a periodic function checking whether any non-terminal exchange hung and can be resumed. Any of the non-terminal states can strand: provisioning that keeps failing, a turn that was never started, a turn T3 still calls active but that will never finish (the agent died, hit its token limit, the provider hung), a reply whose posting keeps being rejected. None of these produce an event, so nothing wakes the exchange up. + +It first requires proper invariants and heuristics to be defined — chiefly, how long a state may legitimately sit before it counts as stuck, which differs per state and is not knowable from the exchange alone. + +## Pure decider + +The branching rules are pure: `stored state + retrieved observation -> next action`. Decision and transition stay separate so no state records an external effect before it happens. The processor retrieves the one observation relevant to the current state and interprets the returned action: + +```text +RequestClaimed planned thread missing | present Provision | RecordThreadCreated +ThreadCreated turn missing | active | completed(r) StartTurn | Wait | RecordReplyPending(r) +ReplyPending my reply missing | posted(id) PostReply | RecordReplyPosted(id) +ReplyPosted / Undeliverable Done +``` + +The state-specific contexts contain only plain, already-retrieved data—never adapters, repositories, clocks, `Effect`s, or query functions: + +```ts +type RequestClaimedContext = { readonly thread: "missing" } | { readonly thread: "present" }; + +type ThreadCreatedContext = + | { readonly turn: "missing" } + | { readonly turn: "active" } + | { readonly turn: "completed"; readonly reply: Reply }; + +type ReplyPendingContext = + | { readonly platformReply: "missing" } + | { + readonly platformReply: "posted"; + readonly replySourceUri: string; + }; +``` + +Temporary failures do not change the exchange state, so the processor can try the same operation again later. If creating the thread or starting the turn has permanently failed, the processor creates a failure reply and moves the exchange to `ReplyPending`. If the platform permanently refuses to post that reply, the processor moves the exchange to `Undeliverable`. The processor decides whether an error is temporary or permanent when the operation fails; the decider only sees the plain result it needs. + +Pure transition constructors preserve the forward-only lifecycle: + +```ts +declare const toThreadCreated: (state: RequestClaimed) => ThreadCreated; + +declare const toReplyPending: (state: RequestClaimed | ThreadCreated, reply: Reply) => ReplyPending; + +declare const toReplyPosted: (state: ReplyPending, replySourceUri: string) => ReplyPosted; + +declare const toUndeliverable: (state: ReplyPending, cause: UndeliverableCause) => Undeliverable; +``` + +`StartTurn` persists no exchange transition—turn existence is T3's fact. Orchestration proper—scheduling triggers, fetching observations, executing actions, classifying operational failures, applying transitions, and persisting them—stays in the processor. + +## Reply delivery + +Identity and content are separate: + +- **Identity**: the adapter must answer with certainty whether its reply for this exact exchange exists on the platform — structural attribution (Discord reply referencing the trigger message, Jira comment linkage) or an embedded exchange UUID as last resort. Never content matching; identical texts legitimately recur. +- **Content**: the verbatim payload stored in `ReplyPending`, so retries post the same thing. + +Delivery is: check existence -> post if absent -> record posted. + +## Failure path + +Any definitive failure while provisioning, starting a turn, or recovering a lost thread becomes a failure-typed reply through the normal delivery pipe; delivering it ends the exchange in `ReplyPosted`—a completed job from the processor's view. Transient failures leave the current state unchanged and are retried in place. Only a definitive rejection of reply delivery ends the exchange in `Undeliverable`. + +## Acknowledgement + +The processor never learns whether the ack succeeded; no exchange state waits on it. The adapter records the ack message ID locally and may deliver the final reply by editing that ack instead of posting fresh — a rendering choice it owns. An adapter doing so must count the edited ack as the existing reply in its certainty check. A crash before the ack means it is simply never posted; the final reply is unaffected. + +## Build order + +Model → contract → orchestration; each phase leaves the previous one settled. + +1. **Exchange (the model).** The five states with their decided contents. Transition constructors as the only way to build each state from its predecessor plus an effect result. The observation and action vocabularies, and the pure decider. Pure table tests for decider and transitions—no Effect scaffolding. + +2. **Ports (the contracts).** The exchange repository, the adapter and the T3 gateway, each shaped around the model per "Ports" above. Update the in-memory test adapter. + +3. **Processor (orchestration).** Collapse `process` / `recoverThread` / `processT3Event` into `process` plus one internal reconciler; the exposed surface stays `process` and `run`. `process` claims, then provisions and starts the turn. `run` calls the reconciler — load → observe → decide → execute → persist — on startup and on T3 events. Serialize per `sourceUri`; the outcome lock and `inFlightRequests` collapse into that. Crash-window tests drive the real loop against the real in-memory repository, with the adapter and T3 gateway faked. + +4. **Jira port** (ntbs-plan step 3) as the first real adapter on the settled contract, replacing the legacy bridge path. + +## Remaining processor tests + +The processor tests are grouped by responsibility: `process`, source serialization, `run`, and reply delivery. + +Lifecycle and retry behavior: + +- [x] A transient `postReply` failure leaves the exchange in `ReplyPending`; a later recovery retries and reaches `ReplyPosted`. +- [x] A transient `findPostedReply` failure leaves the exchange in `ReplyPending`; a later recovery repeats discovery before posting. +- [x] A failed acknowledgement is best-effort: processing still persists `ThreadCreated` and starts the turn. +- [x] An active turn leaves `ThreadCreated` unchanged and performs no delivery work. +- [x] A transient `provisionThread` failure leaves `RequestClaimed`; later recovery provisions the thread successfully. +- [x] A transient `getTurnStatus` failure leaves `ThreadCreated`; later activity retries and records the completed reply. +- [x] A transient `startTurn` failure leaves `ThreadCreated`; later recovery retries the start. +- [x] Extend the post-persistence failure test to prove the retained `RequestClaimed` can later be resumed by `run`. + +`run` robustness: + +- [x] One exchange failing during startup recovery does not prevent another exchange from advancing. +- [x] One thread-activity event failing does not stop later activity events from being processed. +- [x] Activity subscription starts before recovery: an event arriving while startup recovery is blocked is not missed. +- [x] Startup recovery racing with activity for the same exchange posts only one reply. + +After these cases, stop expanding the processor suite unless its contract changes. Do not add tests for every `NTBSProcessorError.reason` string, every lifecycle tag already covered by the pure model tests, repository behavior already covered by `ExchangeRepository.test.ts`, internal lock-map deletion with no observable behavior, or every `Reply` subtype that the processor handles identically. + +## T3 gateway implementation and tests + +There is currently no T3 gateway implementation or focused gateway test file: `t3gateway.ts` only defines the port. The processor tests mock that port and already cover how its contexts and errors drive the exchange lifecycle. The old processor tests exercise an obsolete adapter-owned design and are not useful gateway coverage. + +Gateway tests should build the real gateway with fake `ProjectionSnapshotQuery`, `ProjectionTurnRepository`, `GitWorkflowService`, `ProjectSetupScriptRunner`, `OrchestrationEngineService`, and deterministic clock/UUID services. They should assert the gateway result and its calls to those boundaries. They should not construct the real orchestration engine, run Git, or repeat processor recovery and persistence tests. + +### Settled gateway contracts + +Planning is branch-only. The platform selects a branch name; `planT3Work` fetches `origin` and pins that branch to the fetched commit. Store the selected branch and immutable commit separately from the new worktree branch: + +```ts +type T3WorkCoordinates = { + readonly projectId: ProjectId; + readonly baseBranchName: string; + readonly baseCommitSha: string; + readonly worktreeBranchName: string; + readonly threadId: ThreadId; + readonly userMessageId: MessageId; +}; +``` + +Worktree creation uses `baseCommitSha` as `refName`, `baseBranchName` as `baseRefName`, and `worktreeBranchName` as `newRefName`. A fetch or other Git/network failure is operational and becomes `T3GatewayError`, so the exchange remains retryable. A project that does not exist, or a branch that is absent after a successful fetch, is `T3Rejected`. + +Provisioning is complete only after the thread and worktree exist and the setup script has completed successfully. The T3 thread projection is the durable readiness marker: + +1. Create the thread first with the stored `threadId`, `worktreeBranchName`, and `worktreePath: null`. +2. Ensure the worktree exists for `worktreeBranchName`. +3. Run the setup script and wait for successful completion. +4. Only then dispatch `thread.meta.update` with the branch and final worktree path. +5. `getThreadStatus` reports `present` only when the thread exists and has a non-null `worktreePath`. An absent thread or a thread with a null path remains incomplete and causes provisioning to resume. + +Worktree recovery uses an exact, refreshed local-ref lookup for `worktreeBranchName`: reuse its live worktree, attach the existing branch when it has no worktree, recreate a stale/missing worktree, or create the branch from `baseCommitSha` when it does not exist. Put those Git-specific cases behind an `ensureWorktree` operation on `GitWorkflowService`; the gateway should not reproduce Git worktree bookkeeping. + +The existing `ProjectSetupScriptRunner.runForThread` only launches a terminal command. Add a blocking `runForThreadAndWait` operation, backed by `ProcessRunner` in the same style as `ProjectLifecycleScriptRunner`. It returns only after no script is needed or the setup command exits successfully; failure or timeout becomes `T3GatewayError`. + +Setup execution is at least once. If setup succeeds and the process dies before `thread.meta.update`, recovery reuses the worktree and runs setup again. Setup scripts must therefore be idempotent. Exactly-once setup would require another durable record and is outside v1. + +Error classification is fixed: + +- Missing project or missing selected branch after a successful fetch: `T3Rejected`. +- A worktree-branch collision inconsistent with the stored coordinates: `T3Rejected`. +- Projection/database, Git/network, setup, UUID, and orchestration persistence failures: `T3GatewayError`. +- Duplicate thread creation with the matching projected thread: successful recovery. +- Duplicate thread creation without the matching projection: `T3GatewayError`, because T3 state is inconsistent. +- Every gateway error preserves the original value as `cause`; classification never inspects arbitrary error text. + +Terminal reply conversion is fixed: + +- A completed turn with nonblank assistant text produces an answer containing the original text exactly. Trimming is used only to detect blank output. +- Missing or blank assistant output produces `"T3 completed without producing a reply."` with a serializable `missing-assistant-reply` cause containing the thread, user-message, and assistant-message IDs. +- An errored turn uses `session.lastError` or `"T3 failed while processing this request."`, with a serializable `turn-error` cause containing the thread ID, user-message ID, and recorded error. +- An interrupted turn produces `"T3 stopped processing this request."` with a serializable `turn-interrupted` cause containing the thread and user-message IDs. + +`threadActivity` emits only `thread.session-set` events, using `payload.threadId`, and preserves repeats. This relies on the orchestration invariant that a terminal session event is observed only after the final turn state and assistant output are readable from the projections. + +### Implementation prerequisites + +- [ ] Rename the coordinate fields to `baseBranchName`, `baseCommitSha`, and `worktreeBranchName`, and update their construction and consumers. +- [ ] Add `GitWorkflowService.ensureWorktree` with the exact-branch recovery behavior above. +- [ ] Add `ProjectSetupScriptRunner.runForThreadAndWait` with an explicit timeout and bounded diagnostic output. +- [ ] Implement the real `T3Gateway` constructor using the settled contracts before adding its focused tests. + +### Focused gateway test checklist + +`planT3Work`: + +- [ ] For an existing project and branch, fetch `origin`, resolve the remote branch once, and return its exact commit SHA alongside the selected branch, requested project, distinct minted thread/message IDs, and worktree branch derived from the thread ID. +- [ ] A fetch/network failure is `T3GatewayError`; a branch absent after a successful fetch is `T3Rejected`. Neither case returns unpinned coordinates or performs provisioning work. +- [ ] A missing project is `T3Rejected`; project-query and UUID failures are `T3GatewayError`. Every case retains its cause and performs no provisioning side effects. + +Thread observation and provisioning: + +- [ ] `getThreadStatus` queries the stored `threadId`: an absent thread or one with `worktreePath: null` maps to `{ thread: "missing" }`, while a non-null path maps to `{ thread: "present" }`. Projection failure becomes `T3GatewayError`. +- [ ] The normal `provisionThread` path dispatches `thread.create` with a null path, ensures a worktree from the stored branch/SHA pairing, waits for setup, and finally dispatches `thread.meta.update`. It uses the project's model selection or standard fallback, mints no replacement exchange IDs, and does not start a turn. +- [ ] A retry after `thread.create` reuses the matching incomplete thread instead of dispatching another. A conflicting duplicate without a matching projection fails as inconsistent T3 state. +- [ ] Cover `ensureWorktree` recovery through the gateway: reuse a live matching worktree, attach an existing branch without a worktree, recreate a stale/missing worktree, and create an absent branch from the stored SHA. A conflicting branch is `T3Rejected`. +- [ ] Setup failure or timeout returns `T3GatewayError` and leaves the thread projection incomplete. A later retry reuses the worktree, runs setup again, records the final path, and then reports the thread present. +- [ ] Failure to persist the final `thread.meta.update` remains retryable. Recovery reruns setup under the documented at-least-once rule and does not create another thread or worktree. + +Turn observation and start: + +- [ ] `startTurn` dispatches exactly one `thread.turn.start` using the stored `threadId`, `userMessageId`, snapshot, and attachments, with the agreed runtime/interaction defaults and a fresh command ID/timestamp. It does no project, Git, setup, or turn-status work. +- [ ] Classify representative permanent dispatch rejection as `T3Rejected` and operational dispatch failure as `T3GatewayError`, preserving each cause. The processor tests already cover what happens after either result. +- [ ] `getTurnStatus` selects the turn whose `pendingMessageId` equals the exchange's stored `userMessageId`, even when the thread contains newer or unrelated turns. +- [ ] No matching turn maps to `{ turn: "missing" }`; `pending` and `running` map to `{ turn: "active" }`. These cases must not load the heavier thread-detail snapshot. +- [ ] A completed turn with its matching assistant message preserves the original nonblank text exactly. Missing or blank output produces the fixed failure text and `missing-assistant-reply` cause. +- [ ] An errored turn produces the recorded error or fixed fallback with a `turn-error` cause; an interrupted turn produces the fixed cancellation text and `turn-interrupted` cause. +- [ ] Turn-list and terminal thread-detail query failures, or a terminal turn whose thread projection is missing, become `T3GatewayError` with the original diagnostic cause. + +Activity stream: + +- [ ] `threadActivity` emits `payload.threadId` for `thread.session-set`, filters every other orchestration event, and preserves repeated session events. The processor owns lookup, serialization, and terminal-state deduplication. + +Stop there unless the gateway contract grows. Do not retest UUID generation, the branch-name helper, Git command behavior, orchestration projection internals, setup-script internals, or processor state transitions; those belong to their existing modules and suites. 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