closed by dirtydishes · Pull Request #7635 · pingdotgg/t3code

dirtydishes · 2026-08-20T04:24:13Z

Opened in error. Closed immediately.

coderabbitai · 2026-08-20T04:24:21Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a1293485-d9fa-497f-9af2-9459124827cd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

macroscopeapp · 2026-08-20T04:27:05Z

+    }),
+  );
+  const projection = useThreadProjection(scopeThreadRef(props.environmentId, props.threadId));
+  if (query.data === null) return null;


🟡 Medium chat/ThreadProgramAttemptPanel.tsx:179

When programAttempt fails, ThreadProgramAttemptPanel returns null, so the Dirtyloops section disappears without showing query.error or any stale-state warning. Handle query.error before the no-data check and render an error state.

- if (query.data === null) return null; + if (query.error) { + return <p className="p-3 text-[11px] text-warning">Unable to load Dirtyloops attempt details: {query.error}</p>; + } + if (query.data === null) return null;

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/web/src/components/chat/ThreadProgramAttemptPanel.tsx around line 179: When `programAttempt` fails, `ThreadProgramAttemptPanel` returns `null`, so the Dirtyloops section disappears without showing `query.error` or any stale-state warning. Handle `query.error` before the no-data check and render an error state.

macroscopeapp · 2026-08-20T04:27:05Z

+        yield* programAttempts.retainProcessInterruptions;
        yield* providerSessions.shutdown;


🟠 High src/serverRuntimeStartup.ts:401

A provider run that finishes during shutdown is persisted as interrupted, so observers can permanently see an interrupted attempt even though the run completed successfully. retainProcessInterruptions runs before providerSessions.shutdown, and its COALESCE update preserves that stale terminal result; shut down provider sessions before retaining the remaining nonterminal attempts.

- yield* programAttempts.retainProcessInterruptions; yield* providerSessions.shutdown; + yield* programAttempts.retainProcessInterruptions;

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/serverRuntimeStartup.ts around lines 401-402: A provider run that finishes during shutdown is persisted as `interrupted`, so observers can permanently see an interrupted attempt even though the run completed successfully. `retainProcessInterruptions` runs before `providerSessions.shutdown`, and its `COALESCE` update preserves that stale terminal result; shut down provider sessions before retaining the remaining nonterminal attempts.

macroscopeapp · 2026-08-20T04:27:05Z

        if (workerFiber !== null) {
          yield* Fiber.interrupt(workerFiber).pipe(Effect.ignore);
        }
+        yield* programAttempts.retainProcessInterruptions;


🟠 High src/serverRuntimeStartup.ts:401

A failure in programAttempts.retainProcessInterruptions aborts the shutdown finalizer before providerSessions.shutdown and providerRuntimeRecovery.reconcile run, leaving provider sessions and runtime state unreconciled. Handle or log this retention failure independently so the remaining cleanup always proceeds.

Suggested change

yield* programAttempts.retainProcessInterruptions;

yield* programAttempts.retainProcessInterruptions.pipe(

Effect.catchCause((cause) =>

Effect.logWarning("Failed to retain process interruptions during shutdown", {

cause: Cause.pretty(cause),

}),

),

);

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/serverRuntimeStartup.ts around line 401: A failure in `programAttempts.retainProcessInterruptions` aborts the shutdown finalizer before `providerSessions.shutdown` and `providerRuntimeRecovery.reconcile` run, leaving provider sessions and runtime state unreconciled. Handle or log this retention failure independently so the remaining cleanup always proceeds.

macroscopeapp · 2026-08-20T04:27:05Z

    updateStateAtom,
    settingsValueAtom,
    providersValueAtom,
+    programAttempt: createEnvironmentQueryAtomFamily(runtime, {


🟡 Medium state/server.ts:740

programAttempt is fetched only once while the panel remains mounted, so an attempt that is running at mount never updates to its terminal snapshot; terminalResult.failure.message and other terminal fields remain stale until remount or reconnect. staleTimeMs does not trigger re-fetching—add a refresh interval or subscribe/invalidate this query when the attempt changes.

🤖 Copy this AI Prompt to have your agent fix this:

In file @packages/client-runtime/src/state/server.ts around line 740: `programAttempt` is fetched only once while the panel remains mounted, so an attempt that is running at mount never updates to its terminal snapshot; `terminalResult.failure.message` and other terminal fields remain stale until remount or reconnect. `staleTimeMs` does not trigger re-fetching—add a refresh interval or subscribe/invalidate this query when the attempt changes.

macroscopeapp · 2026-08-20T04:27:05Z

+            ),
+          ),
+        );
+      const run = launched.projection.runs.toSorted(


🟠 High orchestration-v2/ProgramAttemptService.ts:414

On an idempotent retry after the thread has a later follow-up run, launch selects that follow-up run as the attempt receipt and can return invalid_record or bind the attempt to the wrong run. Sorting the entire projection.runs does not identify the run created by this launch; derive the receipt from the launch-specific run instead.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/orchestration-v2/ProgramAttemptService.ts around line 414: On an idempotent retry after the thread has a later follow-up run, `launch` selects that follow-up run as the attempt receipt and can return `invalid_record` or bind the attempt to the wrong run. Sorting the entire `projection.runs` does not identify the run created by this launch; derive the receipt from the launch-specific run instead.

macroscopeapp

Effect service conventions review of the new ProgramAttemptService / PreparedWorktreeVerifier services and their call sites. Three findings, all in apps/server/src/orchestration-v2/ProgramAttemptService.ts; the rest of the touched Effect code (namespace imports, environment-based dependency acquisition, layer wiring, ProviderTurnStartService.fail error passthrough) follows the conventions.

^{Posted via Macroscope — Effect Service Conventions}

macroscopeapp · 2026-08-20T04:27:23Z

+    reason: Schema.Literals([
+      "not_found",
+      "request_conflict",
+      "launch_incomplete",
+      "run_missing",
+      "not_terminal",
+      "persistence_failed",
+      "launch_failed",
+      "projection_failed",
+      "cancel_failed",
+      "invalid_record",
+    ]),
+    attemptId: ProgramAttemptId,
+    detail: Schema.String,


reason here is not diagnostic-only: programAttemptHttp.ts switches on it to choose 404 / 409 / 400 / 500, and detail is surfaced verbatim as the caller-visible HTTP message. Per the conventions these are semantically distinct failures and should be separate Schema.TaggedErrorClass types (e.g. ProgramAttemptNotFoundError, ProgramAttemptRequestConflictError, ProgramAttemptStateError, ProgramAttemptPersistenceError), each deriving message from its structural attributes rather than carrying a free-form detail string as the only payload. The HTTP mapper can then use Effect.catchTags({ ... }) instead of a reason switch.

^{Posted via Macroscope — Effect Service Conventions}

macroscopeapp · 2026-08-20T04:27:23Z

+function error(
+  attemptId: ProgramAttemptId,
+  reason: ProgramAttemptError["reason"],
+  detail: string,
+  cause?: unknown,
+) {
+  return new ProgramAttemptError({
+    attemptId,
+    reason,
+    detail,
+    ...(cause === undefined ? {} : { cause }),
+  });
+}


error(...) is a helper whose only behavior is new ProgramAttemptError({ ...args }); it performs no normalization, no domain-error passthrough, and adds no control flow. Consider constructing the error inline at each failure boundary so its attributes and cause stay visible, or replacing it with static factories on the error class(es) that actually classify the incoming failure.

^{Posted via Macroscope — Effect Service Conventions}

macroscopeapp · 2026-08-20T04:27:23Z

+  const run = projection.runs.find((candidate) => candidate.id === runId);
+  if (run === undefined || !ThreadManagementService.isTerminalRunStatus(run.status)) {
+    throw new Error(`Run ${runId} is not terminal.`);


Throwing a raw Error inside service code turns a domain condition into an untyped defect (it escapes snapshot/persistTerminal as a die rather than a ProgramAttemptError). Consider making terminalResult return an Effect that fails with the typed not_terminal failure (or having the caller keep the terminal check and fail typed) so the failure stays in the declared error channel.

^{Posted via Macroscope — Effect Service Conventions}

macroscopeapp

UI consistency review found 2 issues in the changed web UI: an internal navigation rendered as a raw anchor in the new thread-details section, and a compact composer surface that was not updated for the new runtime mode.

^{Posted via Macroscope — UI Consistency}

macroscopeapp · 2026-08-20T04:27:40Z

+  "read-only": {
+    label: "Read only",
+    description: "Allow inspection but deny commands and file changes that need write access.",
+    icon: LockIcon,
+  },


read-only is added to runtimeModeConfig, but CompactComposerControlsMenu still hard-codes its own four MenuRadioItems (approval-required … full-access). At compact widths a thread already in read-only mode renders a radio group with no matching item, so nothing shows as selected and the mode cannot be chosen there — the two composer surfaces now disagree about which runtime modes exist. Suggest driving that menu from the exported runtimeModeConfig/runtimeModeOptions (or, minimally, adding <MenuRadioItem value="read-only">Read only</MenuRadioItem>).

Optional: read-only and approval-required share LockIcon, so the two rows in the select popup are visually identical apart from their text; a distinct glyph (e.g. EyeIcon) would keep the mode ladder readable.

^{Posted via Macroscope — UI Consistency}

macroscopeapp · 2026-08-20T04:27:40Z

+          <a
+            href={threadHref}
+            className="inline-flex max-w-full items-center gap-1 text-foreground/85 underline decoration-border underline-offset-2 hover:decoration-foreground/70"
+          >
+            <span className="truncate font-mono text-[10px]">{attempt.runId}</span>
+            <ExternalLinkIcon aria-hidden className="size-3 shrink-0 text-muted-foreground" />
+          </a>


The T3 run row navigates an app route through a raw <a href>. Every other internal navigation in apps/web goes through TanStack Router (Link, or useNavigate + buildThreadRouteParams, as the sibling ThreadRelationshipsPanel/ThreadAutomationsPanel do); the only raw anchors are external URLs. In Electron the router runs on hash history (main.tsx), so a plain path href takes the shell away from the loaded app instead of routing, and in the browser it forces a full reload that drops SPA state. ExternalLinkIcon also advertises an external target for an in-app route.

Smallest fix: render the router link and an in-app affordance (imports for Link from @tanstack/react-router, buildThreadRouteParams from ../../threadRoutes, and e.g. ArrowRightIcon are needed, and threadHref can go away).

- <a - href={threadHref} + <Link + to="/$environmentId/$threadId" + params={buildThreadRouteParams(scopeThreadRef(props.environmentId, attempt.threadId))} className="inline-flex max-w-full items-center gap-1 text-foreground/85 underline decoration-border underline-offset-2 hover:decoration-foreground/70" > <span className="truncate font-mono text-[10px]">{attempt.runId}</span> - <ExternalLinkIcon aria-hidden className="size-3 shrink-0 text-muted-foreground" /> - </a> + <ArrowRightIcon aria-hidden className="size-3 shrink-0 text-muted-foreground" /> + </Link>

^{Posted via Macroscope — UI Consistency}

dirtydishes added 6 commits August 19, 2026 21:09

feat(server): add exact program attempt launch seam

4997dea

fix(server): bind program attempt effects exactly

b7de725

fix(server): terminalize exhausted provider starts

5b66b32

seal review recovery paths

49eb064

feat(web): show dirtyloops task summary

5bab8e2

fix s6 program observability gaps

56157e7

github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Aug 20, 2026

macroscopeapp Bot reviewed Aug 20, 2026

View reviewed changes

dirtydishes changed the title ~~add dirtyloops program attempts to t3 code~~ closed Aug 20, 2026

dirtydishes closed this Aug 20, 2026

		yield* programAttempts.retainProcessInterruptions;
		yield* providerSessions.shutdown;

-        yield* programAttempts.retainProcessInterruptions;
+        yield* programAttempts.retainProcessInterruptions.pipe(
+          Effect.catchCause((cause) =>
+            Effect.logWarning("Failed to retain process interruptions during shutdown", {
+              cause: Cause.pretty(cause),
+            }),
+          ),
+        );

Uh oh!

Conversation

dirtydishes commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 20, 2026

Review skipped

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot left a comment

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot left a comment

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dirtydishes commented Aug 20, 2026 •

edited

Loading