feat: run Discord coding tasks in isolated worktrees - #5467
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the new Effect service code in this PR against the repo's Effect service conventions.
One clear violation: the Discord channel task-start effect fails with plain strings instead of a Schema.TaggedErrorClass with structured attributes, so the failures carry no structural context and are indistinguishable at the Promise boundary (the catch in handleText reports "could not create an isolated worktree" for every case, including configuration failures).
Everything else checked out: subpath namespace imports for effect/*, the Effect.runPromiseWith(yield* Effect.context<never>()) bridge is confined to the imperative channel adapter (same pattern as ClaudeAdapter) and is not injected into another Effect service, dependencies are acquired from the environment with yield*, and the serverSettings.ts secret plumbing reuses the existing structured ServerSettingsError with a valid operation literal.
Posted via Macroscope — Effect Service Conventions
| const botToken = discord.botTokenRedacted | ||
| ? yield* readSecret(DISCORD_BOT_TOKEN_SECRET_NAME) | ||
| : discord.botToken; | ||
| return { | ||
| ...settings, | ||
| channelIntegrations: { | ||
| ...settings.channelIntegrations, | ||
| discord: { | ||
| ...discord, | ||
| botToken, | ||
| }, | ||
| }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🟡 Medium src/serverSettings.ts:394
When botTokenRedacted is true but the secret store returns Option.none (e.g. after copying settings.json to a machine without its secret store), materializeChannelSecrets sets botToken to "" but leaves botTokenRedacted: true. redactServerSettingsForClient preserves that marker, so the Channels UI treats the integration as configured even though no credential exists. Clear botTokenRedacted when the secret is absent.
const botToken = discord.botTokenRedacted
? yield* readSecret(DISCORD_BOT_TOKEN_SECRET_NAME)
: discord.botToken;
+ const botTokenRedacted = discord.botTokenRedacted && botToken.length > 0;
return {
...settings,
channelIntegrations: {
...settings.channelIntegrations,
discord: {
...discord,
botToken,
+ botTokenRedacted,
},
},
};🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/serverSettings.ts around lines 394-407:
When `botTokenRedacted` is `true` but the secret store returns `Option.none` (e.g. after copying `settings.json` to a machine without its secret store), `materializeChannelSecrets` sets `botToken` to `""` but leaves `botTokenRedacted: true`. `redactServerSettingsForClient` preserves that marker, so the Channels UI treats the integration as configured even though no credential exists. Clear `botTokenRedacted` when the secret is absent.
| projectId !== null && | ||
| baseBranch.trim().length > 0 && | ||
| branchPrefix.trim().length > 0 && | ||
| applicationId.trim().length > 0 && | ||
| hasBotToken; | ||
| const selectedProject = projects.find((project) => project.id === projectId) ?? null; |
There was a problem hiding this comment.
🟠 High settings/ChannelSettings.tsx:73
setupComplete treats any non-null projectId as valid, but when the configured project was deleted or belongs to a different environment, selectedProject is null. The UI still labels the integration "Configured" and allows saving, so Discord requests target a project that no longer exists and cannot start. Consider requiring selectedProject !== null in setupComplete so the status badge and save button reflect the missing project.
const setupComplete =
- projectId !== null &&
+ selectedProject !== null &&
baseBranch.trim().length > 0 &&
branchPrefix.trim().length > 0 &&
applicationId.trim().length > 0 &&
hasBotToken;🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ChannelSettings.tsx around lines 73-78:
`setupComplete` treats any non-null `projectId` as valid, but when the configured project was deleted or belongs to a different environment, `selectedProject` is `null`. The UI still labels the integration "Configured" and allows saving, so Discord requests target a project that no longer exists and cannot start. Consider requiring `selectedProject !== null` in `setupComplete` so the status badge and save button reflect the missing project.
| ); | ||
| return { value: "", redacted: true } as const; | ||
| }); | ||
| const botToken = yield* persistSecret({ |
There was a problem hiding this comment.
🟠 High src/serverSettings.ts:568
persistChannelSecrets writes to the secret store before normalizeServerSettings and writeSettingsAtomically commit the settings file. If normalization or the file write fails, the error is returned but the secret mutation is not rolled back — a replaced Discord bot token is already changed in the store, and a cleared token is already deleted, even though the old settings file still references it. The API reports failure while leaving credentials modified or lost. Consider staging secret mutations and committing them only after the settings write succeeds, or rolling them back on failure.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/serverSettings.ts around line 568:
`persistChannelSecrets` writes to the secret store before `normalizeServerSettings` and `writeSettingsAtomically` commit the settings file. If normalization or the file write fails, the error is returned but the secret mutation is not rolled back — a replaced Discord bot token is already changed in the store, and a cleared token is already deleted, even though the old settings file still references it. The API reports failure while leaving credentials modified or lost. Consider staging secret mutations and committing them only after the settings write succeeds, or rolling them back on failure.
| config.enabled && | ||
| config.projectId !== null && | ||
| config.baseBranch.trim().length > 0 && | ||
| config.branchPrefix.trim().length > 0 && |
There was a problem hiding this comment.
🟠 High channels/T3CodeDiscordChannel.ts:83
isDiscordChannelConfigured accepts branchPrefix values like "/" or strings containing spaces, which then cause every task to fail during worktree creation. After channelBranchName strips surrounding slashes, a branchPrefix of "/" produces a branch like /task-<suffix>, and prefixes with spaces pass through into the branch name. Git rejects refs that begin with / or contain spaces, so these settings are accepted as valid but always fail at gitWorkflow.createWorktree. Consider trimming and validating branchPrefix more strictly — for example, rejecting it when nothing remains after stripping slashes or when it contains whitespace.
| config.branchPrefix.trim().length > 0 && | |
| config.branchPrefix.trim().length > 0 && | |
| !/^\/+|\s|\/+$/u.test(config.branchPrefix) && |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around line 83:
`isDiscordChannelConfigured` accepts `branchPrefix` values like `"/"` or strings containing spaces, which then cause every task to fail during worktree creation. After `channelBranchName` strips surrounding slashes, a `branchPrefix` of `"/"` produces a branch like `/task-<suffix>`, and prefixes with spaces pass through into the branch name. Git rejects refs that begin with `/` or contain spaces, so these settings are accepted as valid but always fail at `gitWorkflow.createWorktree`. Consider trimming and validating `branchPrefix` more strictly — for example, rejecting it when nothing remains after stripping slashes or when it contains whitespace.
| ].join("\u0000"); | ||
| } | ||
|
|
||
| export const layer = Layer.effectDiscard( |
There was a problem hiding this comment.
🟡 Medium channels/T3CodeDiscordChannel.ts:427
notifyCompleted silently drops the completion card whenever the channel is restarted or reconfigured while a task is running. linkedThreads is an in-memory Map on a single channel instance; each reconcile that changes the fingerprint replaces the instance and discards all prior Discord↔T3 thread links. When thread.turn-diff-completed later fires, linkedThreads.get(completion.threadId) returns undefined, so notifyCompleted exits at if (!thread) return and the completion reply is never posted. Persisting thread links or keying them independently of the channel instance would prevent the loss.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around line 427:
`notifyCompleted` silently drops the completion card whenever the channel is restarted or reconfigured while a task is running. `linkedThreads` is an in-memory `Map` on a single channel instance; each `reconcile` that changes the fingerprint replaces the instance and discards all prior Discord↔T3 thread links. When `thread.turn-diff-completed` later fires, `linkedThreads.get(completion.threadId)` returns `undefined`, so `notifyCompleted` exits at `if (!thread) return` and the completion reply is never posted. Persisting thread links or keying them independently of the channel instance would prevent the loss.
| return yield* Effect.fail("Discord channel branch must differ from its base branch"); | ||
| } | ||
|
|
||
| const worktree = yield* gitWorkflow.createWorktree({ |
There was a problem hiding this comment.
🟡 Medium channels/T3CodeDiscordChannel.ts:342
After createWorktree succeeds, a failure in either thread.create or thread.turn.start rejects startTaskEffect without removing the worktree, so the worktree and its branch are leaked on disk. The Discord user sees a "Task did not start" error, but the worktree remains registered and the branch name stays occupied. Repeated transient dispatch failures accumulate orphaned worktrees and storage. Consider wrapping the post-createWorktree dispatch logic in an Effect.ensuring or Effect.catchAll that calls gitWorkflow.removeWorktree on failure.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around line 342:
After `createWorktree` succeeds, a failure in either `thread.create` or `thread.turn.start` rejects `startTaskEffect` without removing the worktree, so the worktree and its branch are leaked on disk. The Discord user sees a "Task did not start" error, but the worktree remains registered and the branch name stays occupied. Repeated transient dispatch failures accumulate orphaned worktrees and storage. Consider wrapping the post-`createWorktree` dispatch logic in an `Effect.ensuring` or `Effect.catchAll` that calls `gitWorkflow.removeWorktree` on failure.
| useEffect(() => { | ||
| setEnabled(settings.enabled); | ||
| setProjectId(settings.projectId); | ||
| setBaseBranch(settings.baseBranch); | ||
| setBranchPrefix(settings.branchPrefix); | ||
| setApplicationId(settings.applicationId); | ||
| setGuildId(settings.guildId); | ||
| }, [settings]); |
There was a problem hiding this comment.
🟠 High settings/ChannelSettings.tsx:62
The useEffect at line 62 that syncs local state when settings changes resets applicationId, guildId, projectId, etc., but never clears botToken or botTokenChanged. If the primary environment/settings changes after a token is typed but before Save, the form repopulates with the new environment's fields while still showing the previously typed secret. Pressing Save then sends the old environment's botToken to the new environment — leaking the credential across environments. Consider resetting botToken to "" and botTokenChanged to false inside the same effect so secrets don't persist across environment switches.
useEffect(() => {
setEnabled(settings.enabled);
setProjectId(settings.projectId);
setBaseBranch(settings.baseBranch);
setBranchPrefix(settings.branchPrefix);
setApplicationId(settings.applicationId);
setGuildId(settings.guildId);
+ setBotToken("");
+ setBotTokenChanged(false);
}, [settings]);🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ChannelSettings.tsx around lines 62-69:
The `useEffect` at line 62 that syncs local state when `settings` changes resets `applicationId`, `guildId`, `projectId`, etc., but never clears `botToken` or `botTokenChanged`. If the primary environment/settings changes after a token is typed but before Save, the form repopulates with the new environment's fields while still showing the previously typed secret. Pressing Save then sends the old environment's `botToken` to the new environment — leaking the credential across environments. Consider resetting `botToken` to `""` and `botTokenChanged` to `false` inside the same effect so secrets don't persist across environment switches.
| ); | ||
| }; | ||
|
|
||
| const handleText = async (thread: ChannelThread, rawText: string) => { |
There was a problem hiding this comment.
🟡 Medium channels/T3CodeDiscordChannel.ts:229
Two messages in the same Discord thread that arrive before the first startTask resolves both start separate agent runs and worktrees, and the second setState call silently overwrites the first link — so a rapid duplicate mention or command launches unintended duplicate coding tasks. handleText reads thread.state() and later calls thread.setState(...) without any per-thread serialization, so concurrent handlers both observe no active task and proceed to startTask. Consider serializing handleText per thread (e.g., with a per-thread lock or in-flight queue) so the second message waits for the first startTask/setState to complete.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around line 229:
Two messages in the same Discord thread that arrive before the first `startTask` resolves both start separate agent runs and worktrees, and the second `setState` call silently overwrites the first link — so a rapid duplicate mention or command launches unintended duplicate coding tasks. `handleText` reads `thread.state()` and later calls `thread.setState(...)` without any per-thread serialization, so concurrent handlers both observe no active task and proceed to `startTask`. Consider serializing `handleText` per thread (e.g., with a per-thread lock or in-flight queue) so the second message waits for the first `startTask`/`setState` to complete.
There was a problem hiding this comment.
One convention issue remains in apps/server/src/channels/T3CodeDiscordChannel.ts: the new DiscordChannelTaskError carries an unstructured message string as its only attribute, so the four distinct start-up failures it represents carry no structural context and cannot be discriminated by callers.
Posted via Macroscope — Effect Service Conventions
| class DiscordChannelTaskError extends Schema.TaggedErrorClass<DiscordChannelTaskError>()( | ||
| "DiscordChannelTaskError", | ||
| { message: Schema.String }, | ||
| ) {} |
There was a problem hiding this comment.
This tagged error stores an unstructured message as its only attribute, which is the one shape the repo's error convention rules out: failures should carry stable structural fields (project id, branch/base branch) and derive message from them. It is also used for four semantically distinct failures (project not configured, project not found, no default model, branch equals base branch), each with its own user-facing wording, so callers cannot tell them apart.
Consider one error class per failure, each with the context available at the failure site and a derived message — matching e.g. WorkspacePaths.ts / ProjectSetupScriptRunner.ts:
| class DiscordChannelTaskError extends Schema.TaggedErrorClass<DiscordChannelTaskError>()( | |
| "DiscordChannelTaskError", | |
| { message: Schema.String }, | |
| ) {} | |
| class DiscordChannelProjectNotConfiguredError extends Schema.TaggedErrorClass<DiscordChannelProjectNotConfiguredError>()( | |
| "DiscordChannelProjectNotConfiguredError", | |
| {}, | |
| ) { | |
| override get message(): string { | |
| return "Discord channel project is not configured"; | |
| } | |
| } | |
| class DiscordChannelProjectNotFoundError extends Schema.TaggedErrorClass<DiscordChannelProjectNotFoundError>()( | |
| "DiscordChannelProjectNotFoundError", | |
| { projectId: ProjectId }, | |
| ) { | |
| override get message(): string { | |
| return `Discord channel project ${this.projectId} was not found`; | |
| } | |
| } | |
| class DiscordChannelProjectModelMissingError extends Schema.TaggedErrorClass<DiscordChannelProjectModelMissingError>()( | |
| "DiscordChannelProjectModelMissingError", | |
| { projectId: ProjectId }, | |
| ) { | |
| override get message(): string { | |
| return `Discord channel project ${this.projectId} has no default model`; | |
| } | |
| } | |
| class DiscordChannelBranchCollisionError extends Schema.TaggedErrorClass<DiscordChannelBranchCollisionError>()( | |
| "DiscordChannelBranchCollisionError", | |
| { branch: Schema.String, baseBranch: Schema.String }, | |
| ) { | |
| override get message(): string { | |
| return `Discord channel branch '${this.branch}' must differ from its base branch '${this.baseBranch}'`; | |
| } | |
| } |
(ProjectId is already exported from @t3tools/contracts; the four new DiscordChannelTaskError({ message }) sites would construct the matching class instead.)
Posted via Macroscope — Effect Service Conventions
| void persistDiscordPatch({ botToken, botTokenRedacted: false }, ["botToken"]).then( | ||
| (saved) => { | ||
| if (!saved) return; | ||
| setBotTokenChanged(false); | ||
| setBotTokenStored(botToken.length > 0); | ||
| }, | ||
| ); | ||
| }} |
There was a problem hiding this comment.
🟠 High settings/ChannelSettings.tsx:238
If the user types a new bot token while a previous token save is still pending, the stale request's success callback unconditionally calls setBotTokenChanged(false), overwriting the newer edit's dirty state. A subsequent save() then sends botToken: "", silently dropping the newly entered token. In SecretInput's onBlur handler, the .then callback should only clear botTokenChanged when botToken still equals the value that was actually persisted.
| void persistDiscordPatch({ botToken, botTokenRedacted: false }, ["botToken"]).then( | |
| (saved) => { | |
| if (!saved) return; | |
| setBotTokenChanged(false); | |
| setBotTokenStored(botToken.length > 0); | |
| }, | |
| ); | |
| }} | |
| void persistDiscordPatch({ botToken, botTokenRedacted: false }, ["botToken"]).then( | |
| (saved) => { | |
| if (!saved) return; | |
| if (botToken !== value) return; | |
| setBotTokenChanged(false); | |
| setBotTokenStored(botToken.length > 0); | |
| }, | |
| ); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ChannelSettings.tsx around lines 238-245:
If the user types a new bot token while a previous token save is still pending, the stale request's success callback unconditionally calls `setBotTokenChanged(false)`, overwriting the newer edit's dirty state. A subsequent `save()` then sends `botToken: ""`, silently dropping the newly entered token. In `SecretInput`'s `onBlur` handler, the `.then` callback should only clear `botTokenChanged` when `botToken` still equals the value that was actually persisted.
| hasBotToken; | ||
| const selectedProject = projects.find((project) => project.id === projectId) ?? null; | ||
|
|
||
| const persistDiscordPatch = useCallback( |
There was a problem hiding this comment.
🟠 High settings/ChannelSettings.tsx:98
After a blur-save starts for a field, further edits to that same field are silently overwritten by the useEffect that syncs from settings. For example, blur applicationId to start saving value A, then refocus and type B before the A request completes. When the A response arrives, persistDiscordPatch deletes the applicationId dirty marker even though the field now holds B. The next settings update runs the sync effect at lines 76–87, and with the marker removed it overwrites the unsaved B value with A. The completion handler must only clear the dirty marker for the exact revision it persisted, not unconditionally for the field name.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ChannelSettings.tsx around line 98:
After a blur-save starts for a field, further edits to that same field are silently overwritten by the `useEffect` that syncs from `settings`. For example, blur `applicationId` to start saving value A, then refocus and type B before the A request completes. When the A response arrives, `persistDiscordPatch` deletes the `applicationId` dirty marker even though the field now holds B. The next `settings` update runs the sync effect at lines 76–87, and with the marker removed it overwrites the unsaved B value with A. The completion handler must only clear the dirty marker for the exact revision it persisted, not unconditionally for the field name.
| useEffect(() => { | ||
| setEnabled(settings.enabled); | ||
| setProjectId(settings.projectId); | ||
| setThreadEnvMode(settings.threadEnvMode); | ||
| if (!dirtyFieldsRef.current.has("baseBranch")) setBaseBranch(settings.baseBranch); | ||
| if (!dirtyFieldsRef.current.has("branchPrefix")) setBranchPrefix(settings.branchPrefix); | ||
| if (!dirtyFieldsRef.current.has("applicationId")) setApplicationId(settings.applicationId); | ||
| if (!dirtyFieldsRef.current.has("guildId")) setGuildId(settings.guildId); | ||
| if (!dirtyFieldsRef.current.has("botToken")) { | ||
| setBotTokenStored(settings.botTokenRedacted); | ||
| } | ||
| }, [settings]); |
There was a problem hiding this comment.
🟠 High settings/ChannelSettings.tsx:78
The useEffect that syncs state from settings skips fields tracked in dirtyFieldsRef, so an unsaved draft value (e.g. baseBranch) is carried over when the primary environment changes and is then persisted to the wrong environment. Switching environments does not reset dirtyFieldsRef or the draft state, so the next blur/save uses the stale value from the previous environment with the new environment's persistServerSettings. This silently cross-contaminates configuration across environments. Consider clearing dirtyFieldsRef and resetting draft state when primaryEnvironment.environmentId changes.
useEffect(() => {
+ const environmentId = primaryEnvironment?.environmentId ?? null;
+ if (environmentId !== dirtyFieldsRef.current["__envId"] as string | undefined) {
+ dirtyFieldsRef.current.clear();
+ (dirtyFieldsRef.current as any)["__envId"] = environmentId;
+ }
setEnabled(settings.enabled);🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/settings/ChannelSettings.tsx around lines 78-89:
The `useEffect` that syncs state from `settings` skips fields tracked in `dirtyFieldsRef`, so an unsaved draft value (e.g. `baseBranch`) is carried over when the primary environment changes and is then persisted to the wrong environment. Switching environments does not reset `dirtyFieldsRef` or the draft state, so the next blur/save uses the stale value from the previous environment with the new environment's `persistServerSettings`. This silently cross-contaminates configuration across environments. Consider clearing `dirtyFieldsRef` and resetting draft state when `primaryEnvironment.environmentId` changes.
| const linked = linkedTasks.get(completion.threadId); | ||
| if (!linked) return; | ||
| if (completion.changedFileCount !== undefined) { | ||
| linked.changedFileCount = completion.changedFileCount; | ||
| } | ||
| const task = await input.operations.getTaskStatus(completion.threadId); | ||
| if (!task) return; | ||
| linked.messageRef = await linked.thread.update( | ||
| linked.messageRef, | ||
| taskStatusText(task, linked.changedFileCount), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🟡 Medium channels/T3CodeDiscordChannel.ts:351
Entries added to linkedTasks are never removed after a task reaches done or failed, so each completed Discord task permanently retains its Thread object and MessageRef. Over the lifetime of a long-running server this map grows without bound and leaks memory. Consider deleting the entry from linkedTasks once the task reaches a terminal state, e.g. inside refreshTask after the final status update.
const linked = linkedTasks.get(completion.threadId);
if (!linked) return;
if (completion.changedFileCount !== undefined) {
linked.changedFileCount = completion.changedFileCount;
}
const task = await input.operations.getTaskStatus(completion.threadId);
if (!task) return;
linked.messageRef = await linked.thread.update(
linked.messageRef,
taskStatusText(task, linked.changedFileCount),
);
+ if (task.state === "done" || task.state === "failed") {
+ linkedTasks.delete(completion.threadId);
+ }🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around lines 351-362:
Entries added to `linkedTasks` are never removed after a task reaches `done` or `failed`, so each completed Discord task permanently retains its `Thread` object and `MessageRef`. Over the lifetime of a long-running server this map grows without bound and leaks memory. Consider deleting the entry from `linkedTasks` once the task reaches a terminal state, e.g. inside `refreshTask` after the final status update.
There was a problem hiding this comment.
Effect service conventions review: two findings on the new Discord channel wiring, plus one previously reported issue that is still present.
apps/server/src/channels/T3CodeDiscordChannel.ts— the layer consumes the Promise adapter (operations.listModels) from inside Effect code, round-trippingProviderRegistrythroughrunPromiseand back throughEffect.tryPromise.apps/web/src/hooks/useSettings.ts—AsyncResultis imported as a named export from theeffect/unstable/reactivitybarrel instead of as a module namespace from its subpath.- Still open from an earlier review (not re-commented):
DiscordChannelTaskErrorcarries only an unstructuredmessageand is reused for four semantically distinct failures.
Posted via Macroscope — Effect Service Conventions
| import { resolveSidebarV2Enabled } from "~/branding.logic"; | ||
| import { ensureLocalApi } from "~/localApi"; | ||
| import * as Struct from "effect/Struct"; | ||
| import { AsyncResult } from "effect/unstable/reactivity"; |
There was a problem hiding this comment.
Effect library modules should be imported as namespaces from their own subpath rather than as named exports of a barrel; the rest of the app uses import * as AsyncResult from "effect/unstable/reactivity/AsyncResult" (e.g. apps/web/src/state/desktopUpdate.ts).
| import { AsyncResult } from "effect/unstable/reactivity"; | |
| import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; |
Posted via Macroscope — Effect Service Conventions
| const models = yield* Effect.tryPromise(() => operations.listModels()).pipe( | ||
| Effect.orElseSucceed(() => []), | ||
| ); |
There was a problem hiding this comment.
listModels is only consumed here, from Effect code, so ProviderRegistry.getProviders is run through Effect.runPromiseWith inside makeOperations and then bridged back with Effect.tryPromise — the Promise adapter becomes a dependency of an Effect service, and orElseSucceed erases the typed failure. The other operations members genuinely need the Promise shape for the channel callbacks; this one does not.
Consider acquiring ProviderRegistry in this layer (const providerRegistry = yield* ProviderRegistry;) and reading the models directly in Effect:
- const models = yield* Effect.tryPromise(() => operations.listModels()).pipe(
- Effect.orElseSucceed(() => []),
- );
+ const models = yield* providerRegistry.getProviders.pipe(
+ Effect.map(discordModelOptions),
+ Effect.orElseSucceed(() => []),
+ );Posted via Macroscope — Effect Service Conventions
| } | ||
| } | ||
|
|
||
| try { |
There was a problem hiding this comment.
🟡 Medium channels/T3CodeDiscordChannel.ts:325
When startTask succeeds but a later step (such as the message PATCH in postStatus) fails, the catch block posts "T3 Code could not start" even though the agent task is already running. This misleads users into resubmitting duplicate tasks. The try/catch wraps thread.setState, postText, and postStatus in addition to startTask, so any notification failure is reported as a startup failure. Consider narrowing the try/catch to startTask only and handling notification failures separately.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around line 325:
When `startTask` succeeds but a later step (such as the message `PATCH` in `postStatus`) fails, the catch block posts "T3 Code could not start" even though the agent task is already running. This misleads users into resubmitting duplicate tasks. The `try`/`catch` wraps `thread.setState`, `postText`, and `postStatus` in addition to `startTask`, so any notification failure is reported as a startup failure. Consider narrowing the `try`/`catch` to `startTask` only and handling notification failures separately.
| ), | ||
| Effect.catchCause(() => Effect.succeed(false)), | ||
| ); | ||
| if (!connected) { |
There was a problem hiding this comment.
🟠 High channels/T3CodeDiscordChannel.ts:632
When ɵruntime.start() fails or times out, reconcile stops the runtime and returns without scheduling a retry or recording an active instance. Reconciliation is only triggered by settings changes, so a transient Discord or network outage during startup leaves the configured integration offline indefinitely until settings change or the server restarts. Consider scheduling a retry on connect failure so the channel can recover from transient outages.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around line 632:
When `ɵruntime.start()` fails or times out, `reconcile` stops the runtime and returns without scheduling a retry or recording an active instance. Reconciliation is only triggered by settings changes, so a transient Discord or network outage during startup leaves the configured integration offline indefinitely until settings change or the server restarts. Consider scheduling a retry on connect failure so the channel can recover from transient outages.
| const completeLinkedTask = async (threadId: ThreadId, status: ChannelTaskStatus) => { | ||
| const linked = linkedTasks.get(threadId); | ||
| if (!linked || linked.terminal || status.assistantResponse === null) return; | ||
| await updateLinkedTask(threadId, status); | ||
| await discordClient.reply(linked.messageRef, taskResponseUi(status.assistantResponse)); | ||
| linked.terminal = true; | ||
| }; |
There was a problem hiding this comment.
🟡 Medium channels/T3CodeDiscordChannel.ts:376
completeLinkedTask sets linked.terminal = true only after awaiting both updateLinkedTask and discordClient.reply, so a concurrent /status request and the refreshPendingTasks poll can both pass the initial linked.terminal guard. Both calls then await the same network operations and each sends the assistant response to Discord, producing duplicate completion replies for the same task. Mark the task terminal before awaiting the network operations so the second concurrent caller exits early.
const completeLinkedTask = async (threadId: ThreadId, status: ChannelTaskStatus) => {
const linked = linkedTasks.get(threadId);
if (!linked || linked.terminal || status.assistantResponse === null) return;
+ linked.terminal = true;
await updateLinkedTask(threadId, status);
await discordClient.reply(linked.messageRef, taskResponseUi(status.assistantResponse));
- linked.terminal = true;
};🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/channels/T3CodeDiscordChannel.ts around lines 376-382:
`completeLinkedTask` sets `linked.terminal = true` only *after* awaiting both `updateLinkedTask` and `discordClient.reply`, so a concurrent `/status` request and the `refreshPendingTasks` poll can both pass the initial `linked.terminal` guard. Both calls then await the same network operations and each sends the assistant response to Discord, producing duplicate completion replies for the same task. Mark the task terminal *before* awaiting the network operations so the second concurrent caller exits early.
1 participant
Discord coding requests currently have to be copied into T3 Code by hand, and there is no safe way to start a run from a team channel. This adds a Discord demo powered by CopilotKit Channels so a mention or
/t3command can start a T3 Code task and receive status and completion in the same Discord thread.Every channel task creates a unique branch and worktree before the agent turn starts. Worktree creation is fail-closed: the base branch is only the starting ref, and a failure never falls back to running on
main. The new Channels settings page keeps project selection, Discord credentials, and branch policy in T3 Code; the bot token is persisted through the server secret store and redacted from the browser andsettings.json.The Discord adapter posts native start, status, and completion cards. Existing orchestration and
thread.turn-diff-completedevents drive the run, so the Discord conversation remains linked to the real T3 Code thread and diff.Screenshots and demo walkthrough: https://ki136pbimw6u.postplan.dev
Verification:
The PR is intentionally draft for the demo. A live Discord round trip still requires maintainers to supply a Discord application and bot token in Settings → Channels.
Model: GPT-5.6-Sol
Harness: Codex (T3 Code)
Note
Add Discord channel integration that runs coding tasks in isolated git worktrees
T3CodeDiscordChannellayer that listens for Discord mentions or/t3commands, creates an isolated git worktree viaGitWorkflowService.createWorktree, and dispatches a new orchestration thread for each task.@copilotkit/channels-*packages; the channel lifecycle is driven byServerSettingsand restarts when config changes./settings/channelsroute with aChannelSettingsUI for configuring Discord app credentials, project, base branch, and branch prefix.settings.jsonand is redacted in client-facing settings responses.📊 Macroscope summarized d7ee184. 8 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.