Skip to content

fix(orchestration-v2): make stuck and resurrected run states unrepresentable - #5203

Closed
Iweisc wants to merge 246 commits into
pingdotgg:mainfrom
delogx:feat/orchestration-run-lifecycle-guarantees
Closed

Iweisc wants to merge 246 commits into
pingdotgg:mainfrom
delogx:feat/orchestration-run-lifecycle-guarantees

Conversation

@Iweisc

@Iweisc Iweisc commented Aug 1, 2026 •

Copy link
Copy Markdown

Problem

State-management bugs in orchestration all share one root cause: a persisted status that claims something ("this run is working") with nothing forcing the claim to stay true. Concretely at HEAD:

  • A fiber dying without finalizing its run left the thread at "Working …" forever; recovery only ran at boot, so every in-process leak persisted until a restart.
  • The effect outbox wrote lease_expires_at but never read it — a hung executor wedged its thread's effect queue for the life of the process.
  • Nothing (not the event log, not the projections) prevented a buggy write from moving a terminal run back to a live status.
  • The thread shell derived its status from the newest run row, so a cancelled queued run (e.g. after promoting a queued message to a steer) could outrank the actually-running turn and anchor the "Working for Xm" timer to the queued message's stale requestedAt.

Fix

Three mechanisms enforce one invariant — every non-terminal run is owned by live work, covered by durable queued work, or on a bounded clock toward terminalization; terminal runs stay terminal:

  1. Transition gate — RunLifecycle.ts is the single source of truth for legal status transitions; EventSink validates every status-bearing event inside the commit transaction (folding statuses through the batch) and rejects illegal commits. Migration 045 adds matching SQLite triggers as a storage backstop, with a replay exemption so projection rebuilds of pre-gate history still work. The policy was derived from an exhaustive sweep of every status writer in the codebase: provider-owned subagent rows and subagent/root_turn nodes are deliberately reopenable and stay ungated; checkpoint rollback's completed → rolled_back stays legal.
  2. Liveness leases — the provider-event ingestion fiber (and launch preparation) holds a renewed lease row per run; the renewal fiber is scoped to the work, so it dies with its owner on any exit path and the lease expires on its own. No code path has to "remember to clear" anything.
  3. Continuous janitor — started right after boot recovery (which stays as the sweep's synchronous first tick): settles expired outbox claims (requeue replay-safe, fail process-bound, per-claim lease tokens so an abandoned claimer can't settle a reclaimed row) and cancels orphaned runs using the boot sweep's cascade, sparing leased, queued-behind, effect-covered, and recently-active work — and never touching live idle provider sessions.

The thread shell now picks the live run (lowest-ordinal non-terminal) over the newest row, fixing the queued-steer "Working 17m" symptom at the derivation layer as well.

Timings are env-tunable (PIKU_ORCH_RUN_LEASE_TTL_MS, PIKU_ORCH_RUN_LEASE_RENEW_MS, PIKU_ORCH_RUN_JANITOR_INTERVAL_MS, PIKU_ORCH_RUN_JANITOR_GRACE_MS, PIKU_ORCH_EFFECT_LEASE_MS). Internals doc: docs/internals/run-lifecycle-guarantees.md.

Tests

  • RunLifecycle.test.ts: transition-policy unit coverage (absorbing terminals, rollback/reopen exemptions).
  • RunLifecycleGuarantees.test.ts: gate rejection (single event + folded batch) with no partial persistence, rollback retirement, storage-trigger rejection + replay exemption, live-run shell derivation, janitor sweep (orphan terminalized; leased/effect-covered/fresh work spared; shield removal makes work sweepable), outbox expired-lease settlement with stale-token refusal.
  • Existing orchestration-v2 suites (foundation persistence, effect worker, recovery, projection store, run execution, thread launch, checkpoint rollback, runtime layer, migrations) pass.

Known follow-up: a replay fixture for a successful queued-message promote-to-steer (the only existing promote test covers the rejected/archived path).

Implemented by Claude Fable 5 via Claude Code.

🤖 Generated with Claude Code


Note

High Risk
Release and deploy workflows lose Windows builds, relay auto-config, and stable-release paths—operational behavior changes beyond naming. Broad env var and home-dir renames can break local/CI setups until secrets and docs are aligned.

Overview
This diff rebrands the repo from T3 Code to Piku Code across agent skills, devcontainer/cursor notes, .env.example, .gitignore (.pikucode), and planning docs—replacing T3CODE_* / ~/.t3 with PIKU_* / ~/.pikucode, t3-sqlite-state with piku-sqlite-state, and hosted defaults like app.pikucode.dev.

Agent workflows drop the OpenAI iOS debugger/simulator-browser skills and the test-t3-app / test-t3-mobile skills, and add test-piku-app for isolated web dev, pairing-token recovery, and SQLite fixture seeding.

CI and release shift package filters to @piku/* and piku, add build-essential on the test job so ACP process-tree fixtures compile, and remove mobile native lint, EAS preview/production, showcase screenshots, and the T3 Connect relay deploy workflow. The release workflow becomes nightly-only (scheduled + manual dispatch): no stable tag path, no Windows desktop builds or WSL node-pty prebuild, no relay deploy job for public Clerk/relay config (uses PIKU_* GitHub vars instead), no post-release version-bump finalize job, and CLI publish always uses the nightly npm tag; resource monitor binaries rename to piku-resource-monitor.

Issue templates no longer list apps/mobile. A new plan doc adds thread lineage / lazy context transfer for orchestration v2 (separate from the run-lifecycle work described in the PR title/description if that lives in other commits).

Reviewed by Cursor Bugbot for commit 990ecc0. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add orchestration-v2 run lifecycle with unrepresentable stuck and resurrected states

  • Introduces a new orchestration-v2 subsystem under apps/server/src/orchestration-v2/ with services for run lifecycle management, provider session management, thread forking, checkpointing, and recovery — designed so stuck and resurrected run states cannot be represented in the type system.
  • Adds 10+ new SQLite migrations (036–045) covering orchestration-v2 schema, subagents, effect cancellation, scheduled tasks, and run lifecycle guarantees.
  • Adds new contract modules (orchestrationV2.ts, scheduledTask.ts, rpc.ts, applicationEvent.ts, etc.) and a ./legacy-orchestration subpath export on @piku/contracts for the old orchestration schema.
  • Renames all internal package namespaces, service identifiers, cookie names, event names, and URNs from t3/t3tools to piku/@piku across the entire monorepo.
  • Behavioral Change: session cookie name changes from t3_session to piku_session, the environment descriptor endpoint moves from /.well-known/t3/environment to /.well-known/piku/environment, and the WebAssembly import key changes from t3_write_pty to piku_write_pty — all of which are breaking for existing clients.
📊 Macroscope summarized 990ecc0. 185 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

juliusmarminge and others added 30 commits April 17, 2026 17:29
Co-authored-by: codex <codex@users.noreply.github.com>
- Initialize provider as unchecked in a pending state
- Update initial probe message to reflect session-local status
- Type the runtime effect with `Scope`
- Build the ACP session runtime without wrapping it in `Effect.scoped`
- Use strict TurnId and ProviderItemId parsing in Codex session routing
- Decode in-memory stdio chunks in streaming mode to avoid split UTF-8 corruption
- Transfer session-owned scopes into adapter state
- Ensure runtime scopes close on stop and startup failure
- Add regression coverage for scoped lifecycle cleanup
- Close the managed native event logger when the adapter layer tears down
- Make session runtime close idempotent with an atomic closed flag
- Add coverage for flushing thread native logs on shutdown
- Use codex app-server snapshots for auth, models, and skills
- Remove legacy CLI/config discovery paths and related helpers
- Update tests for the new provider status flow
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- Document the target orchestration graph, IDs, lifecycles, and capability model
- Add Codex app-server probe fixtures and update the probe test harness
- Introduce orchestration v2 service interfaces and error types
- Add replay runtime, fixtures, and integration coverage
- Update shared contracts and probe transcripts

Co-authored-by: codex <codex@users.noreply.github.com>
- Add Codex adapter and replay harness wiring
- Introduce in-memory orchestration projections and provider registry
- Expand orchestration contracts for turn and runtime events
Co-authored-by: codex <codex@users.noreply.github.com>
- Add context transfer IDs, schemas, and projections
- Support cheap fork creation and Codex native fork rollback
- Cover fork idempotency and replay behavior in tests
- Track remaining projection, context transfer, rollback, capability, and subagent work
- Clarify current V2 baseline and debugger-only follow-ups
- Map fork and merge-back turns into stored handoffs and transfer resolutions
- Add shell snapshot projection support plus coverage tests
- Update replay fixtures and web contracts for the new turn flow
Co-authored-by: codex <codex@users.noreply.github.com>
- Move Codex replay recording into `apps/server`
- Add Claude Agent SDK replay fixtures and test harness
- Update orchestration-v2 fixture scenarios and docs
- Move Claude provider runtime logic into its own module
- Share the SDK query runner between live and replay paths
- Add replay driver error wrapping for unexpected failures
Port orchestration V2 provider adapter wiring to the provider-instance driver registry.

Co-authored-by: codex <codex@users.noreply.github.com>
- persist the selected model on run records
- surface run model selection in the debug UI
- update replay fixtures and contracts for the new field
- Record Claude SDK transcripts across multiple prompts and restart/query modes
- Add approval and tool-call replay coverage for new orchestration fixtures
- Update Claude adapter testkit to model open/prompt/permission frames
- Derive Claude SDK query options from runtime policy
- Add read-only replay fixture and policy mapping tests
- Reuse shared approval-policy fixtures across orchestrator tests

Co-authored-by: codex <codex@users.noreply.github.com>
- add active steering and interrupt-restart replay fixtures
- update Claude adapter/orchestrator turn handling for steering
- refresh replay and integration test coverage
- add interrupt and mid-tool replay fixtures for Claude and Codex
- log Claude Agent SDK protocol frames to native event traces
- project Codex commandExecution start events into orchestration updates
- Map Cursor SDK agents and runs to V2 thread and turn lifecycles
- Update MCP capability, tool, and testing guidance for SDK-based injection
Claude Code and others added 19 commits July 31, 2026 18:44
Azure DevOps, Bitbucket, and GitLab each carried a CLI/API adapter, a
provider, PR/MR listing and creation, auth probing, clone sources, icons,
and settings rows. Removed all three end to end: server providers and
layers, the narrowed SourceControlProviderKind contract, shared
presentation, the command-palette Sources picker rows, publish targets,
and the user docs. Local folder, Git URL, and GitHub remain.

Also finished the nightly-only pass over docs (npx piku@nightly, since
nightly is the only npm dist-tag we publish) and dropped the remaining
prose for the deleted infra/relay deployment and SSH launch paths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was vendored only to mirror the relay's Alchemy dependency, and its
version source (infra/relay/package.json) went away with the relay, so
`sync:repos` would fail on it. The JSON-version-source test coverage it
provided now uses a synthetic repo entry instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pnpm-workspace.yaml no longer declares the expo/react-native overrides or
patches, so the lockfile's copies of them made `pnpm install
--frozen-lockfile` fail outright. Regenerate it.

Also drop the imports, locals, and spreads left unreferenced by the
platform and provider removals, and format two stragglers.
PikuWordmark carried aria-label="Piku" but its SVG path still traced the
literal "T3" letterforms, so the running app rendered "T3 Code" in the
sidebar header. Found by launching the real app rather than grepping.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
refactor: rebrand to Piku Code and trim to desktop-only
All four tracked brand SVGs still drew the old "T3" wordmark as raw path
data, so the app icons and favicons carried the pre-rebrand mark. The "T"
subpath is replaced with a "P" matching the adjacent "3" glyph: cap 37 to
baseline 93, stem and bowl at 13.04 units (measured off the "3" as
109.925 - 96.8853), counter wound opposite the outer contour so it punches
through under nonzero fill. The "3" subpath is byte-identical in all four.

assets/prod/app-icon.icon/Assets/text.svg is a pre-scaled 77x47 variant;
its P is derived through the transform that maps the 128-unit glyph onto
it, x' = 0.8125(x - 15.5309), y' = 0.8125(y - 37), rather than redrawn.

The tracked PNG/ICO exports still show T3. Every one of them, including
the Linux and web favicons, renders through Icon Composer's ictool, which
is macOS-only, so they cannot be regenerated here and need a follow-up
`vp run icons:export` pass on macOS. icons:check is not wired into CI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(assets): replace the T3 brand glyph with P3
PR #2 updated the vector glyph sources to P3 but left the tracked PNG/ICO
exports untouched, so the shipped app icon still rendered "T3" in the dock
and in browser tabs.

The exports normally come from Icon Composer's ictool, which is macOS-only
and unavailable in this environment, so these were produced by editing the
1024px masters directly and deriving the rest:

  - the "T" region of each master was regenerated as a "P" via a masked
    image edit, then composited back over the original pixels so the
    per-brand artwork (nightly starry sky, prod black/chrome, dev
    blueprint grid) and the original alpha channel are preserved verbatim
  - 180/32/16 web sizes and the 7-size .ico were downscaled from the
    corrected masters
  - macOS masters keep their required 824x824 opaque body inset 100px
  - apps/web/public copies re-synced from the dev exports, matching
    DEVELOPMENT_PUBLIC_ICON_OVERRIDES

Note this bypasses the documented "do not edit the generated PNG or ICO
files directly" rule in assets/README.md. A subsequent `vp run
icons:export` on macOS should supersede these bitmaps with true ictool
renders; until then the shipped icons at least match the brand.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(brand): regenerate raster icon exports with the P3 mark
Merges pingdotgg#2829 ("feat(orchestrator): introduce new
orchestrator") into the Piku Code fork. The PR replaces the V1
session/turn orchestration core with an event-sourced V2 layer:
run/attempt semantics, ProjectionStoreV2, a durable effect outbox,
checkpoint capture/rollback, provider adapter drivers, thread
fork/merge-back/context handoff, scheduled tasks, and orchestrator +
worktree MCP toolkits.

Resolving 211 conflicts came down to four decisions:

- Upstream naming. Our rebrand rewrote nearly every import line, so a
  plain merge conflicts on any line the PR also touched. The upstream
  side of each conflict was rebranded first (@t3tools/* -> @piku/*,
  T3CODE_/T3_ -> PIKU_, refs/t3 -> refs/piku, t3_* metrics -> piku_*,
  .t3 -> .pikucode, T3ProjectFile* -> PikuProjectFile*) and then
  re-merged, which resolved 82 of the 98 content conflicts as pure
  renames. The same pass covers the PR's new files.

- Surfaces this fork dropped. Files the trim deleted stay deleted:
  the mobile app, the marketing site, tailscale/SSH remote
  environments, and Windows-only spawn handling. New PR files landing
  in those areas were dropped too, along with the Cursor SDK dep and
  its Connect RPC transport.

- Providers. This fork ships Claude and Codex, so the PR's Cursor,
  Grok and OpenCode V2 adapters, settings, fixtures and driver
  registrations are dropped. The PR's new acpRegistry provider is
  kept: it is a generic ACP-agent bridge over the ACP plumbing we
  already carry, not one of the dropped three. Tests keyed to a
  dropped provider were retargeted to a provider we ship
  (acpRegistry for ACP capability shapes, claudeAgent for the
  model-change and maintenance fixtures) rather than deleted, and the
  ACP replay fixtures the acpRegistry entries reuse were kept.

- V1 removal. Where the PR deletes a V1 module our main had since
  modified (reactors, provider service/adapter registry,
  threadReducer, orchestrationEventEffects), the deletion wins.

Validation: pnpm fmt, pnpm lint (0 errors), pnpm typecheck, pnpm test
(5121 passed, 3 skipped, 0 failed).
merge: upstream t3code PR pingdotgg#2829 (orchestrator V2)
Claude Code's dynamic workflows (the Workflow tool) previously fell
through the generic subagent path and rendered as a bare subagent card
with a single progress line. This teaches the full pipeline about them:

- contracts: new `workflow` turn item carrying the workflow name,
  description, script, declared phases, live per-agent state (label,
  phase, model, status, tokens, tool uses, duration, previews), run
  usage, and the subagent linkage fields so run-owned lifecycle
  tracking keeps working.
- ClaudeAdapterV2: tasks with task_type `local_workflow` keep the
  subagent lifecycle (node, wake continuation, child thread) but emit
  the rich workflow item instead. Declared phases are recovered from
  the script's `export const meta` literal at task_started; live state
  comes from the undocumented `workflow_progress` snapshots on
  task_progress frames; terminal task_notification folds still-open
  agent rows to the outcome the terminal implies.
- web: V2WorkflowCard renders the run as a persistent timeline card —
  header with name/status/agent totals/tokens/duration, phase list
  with per-phase agent counts, per-agent rows with model + usage, a
  live progress strip while running, a result disclosure once settled,
  and an open-child-thread affordance.

Wire format verified against a captured claude CLI stream (v2.1.220).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rn settles

A workflow usually outlives its launch turn: Claude answers "launched in
background" within seconds, the turn settles, and every subsequent
task_progress frame used to be dropped by the wake buffer (only the
terminal notification counts as wake evidence). The card froze at
whatever snapshot arrived before settle and jumped straight to
completed — observed live with a run whose Analysis and Summary agents
never appeared.

The session registry entry now records the provider linkage of the turn
that last projected it, and a settled-progress path re-emits the
subagent record and turn item straight from the registry when progress
arrives with no active turn context. Applies to plain subagent progress
too. RunExecutionService already keeps the run's ingestion fiber alive
while a background-capable item is open, so the events flow end to end.

Also strips the dated suffix from provider model ids in the workflow
card's agent rows (claude-haiku-4-5-20251001 → "Haiku 4.5").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Design-consistency pass over V2WorkflowCard, picked from a four-variant
comparison (see PR comment):

- header follows the subagent card grammar: normal-weight description as
  the title, muted stats, and the timeline's neutral mono status pill
  instead of a colored one; mono is reserved for data
- phases render as plan-sidebar steps: the same size-5 status chips, the
  same blue tint on the live step only, 13px step titles
- agent rows use the plan palette (success/primary/destructive tokens
  rather than raw emerald), inline result previews, and right-aligned
  mono stats
- the live progress strip uses the standard LoaderIcon working signal
  instead of a bespoke pulsing dot; failed runs tint the card with the
  event-row destructive treatment

Verified in light and dark themes; timeline tests unchanged and green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dynamic workflow UI for Claude Code runs
The stage artwork behind the sidebar header and send button was fixed to
the environment channel: Blueprint on Dev, Night Sky on Nightly, nothing
anywhere else. Now a Sidebar artwork picker in Settings → Appearance
offers those two scenes plus six new illustrated banners (Aurora, Dusk
Tide, Midnight City, Misty Pines, Nebula, Neon Horizon). An explicit
choice shows on every channel; the default Auto keeps the exact
channel-gated behavior, and the Auto tile previews what it resolves to
on the current environment.

The new sidebarArtwork client setting is a literal union in contracts,
so unknown ids from newer builds fail decoding into the default rather
than persisting garbage. Raster scenes ship as ~20 KB WebP assets that
Vite emits as separate hashed files, fetched only when shown.

Built with Claude Fable 5 on Claude Code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(web): choose the sidebar artwork in Settings → Appearance
…entable

Threads could stay "Working" forever when a fiber died without finalizing
its run, a hung effect wedged its thread's queue for the life of the
process, and nothing prevented a buggy write from moving a terminal run
back to a live status. Recovery only ran at boot, so every in-process leak
persisted until a restart.

Three mechanisms now enforce one invariant — every non-terminal run is
owned by live work, covered by durable queued work, or on a bounded clock
toward terminalization, and terminal runs stay terminal:

- A pure transition policy (RunLifecycle) gates every status-bearing event
  inside the EventSink commit transaction, with SQLite triggers as a
  storage-level backstop (migration 045; projection rebuilds replay with
  the guard relaxed).
- Non-terminal runs hold liveness leases renewed by the fiber that drives
  them (provider-event ingestion, launch preparation), so an orphaned run
  is provably orphaned once its lease expires.
- A continuous janitor settles expired effect-outbox claims (requeue
  replay-safe, fail process-bound, per-claim lease tokens) and cancels
  orphaned runs with the boot sweep's cascade, sparing leased, queued-
  behind, effect-covered, and recently-active work.

The thread shell now derives its status from the live run instead of the
newest row, so a cancelled queued run (e.g. after promoting a queued
message to a steer) no longer shows "Working for…" anchored to a stale
timestamp, and a terminal tail row no longer hides a running turn.

Implemented by Claude Fable 5 via Claude Code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

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: 87402f57-0b84-43f1-8bc1-4f5677717f19

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.

@github-actions 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 1, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 990ecc0. Configure here.

version:
description: "Release version (for example 1.2.3 or v1.2.3)"
required: false
type: string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual release dispatch skips jobs

Medium Severity

After this change, workflow_dispatch is the only manual release trigger, but preflight still needs check_changes, which runs only on schedule. GitHub Actions skips dependent jobs when a needed job is skipped, so a manual run never reaches build or publish.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 990ecc0. Configure here.

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions review of the orchestration run-lifecycle changes. The new RunJanitorService.ts, RunLifecycle.ts and the EventSink gate wiring largely follow the conventions (inline service interface, make + layer, exported Schema.is predicate, multi-value operation discriminator). Four findings, all in RunLeaseService.ts and EventSink.ts.

Posted via Macroscope — Effect Service Conventions

@macroscopeapp

macroscopeapp Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Effect Service Conventions — findings

Inline anchoring failed, so the findings are listed here with file/line references.

1. apps/server/src/orchestration-v2/RunLeaseService.ts:55-70 — standalone RunLeaseServiceV2Shape

The service interface should be declared inline in the Context.Service call rather than kept as a standalone shape type (the sibling RunJanitorV2 added in this PR already does this). Consider moving the withRunLease member (with its doc comment) into the Context.Service<RunLeaseServiceV2, { ... }>() type argument, dropping the RunLeaseServiceV2Shape export, and referring to the inferred shape as RunLeaseServiceV2["Service"]["withRunLease"] at line 133.

2. apps/server/src/orchestration-v2/RunLeaseService.ts:72 — no exported make

This module owns construction, so it should export a real make and keep the canonical order (tag, make, then layer), as RunJanitorService.ts does in this same PR:

export const make = Effect.gen(function* () { /* current layer body */ });

export const layer: Layer.Layer<RunLeaseServiceV2, never, SqlClient.SqlClient> = Layer.effect(
  RunLeaseServiceV2,
  make,
);

3. apps/server/src/orchestration-v2/RunLeaseService.ts:37-53 — process.env read in a module-scope helper

envMs bypasses the server's effect/Config convention (see apps/server/src/cli/config.ts, telemetry/AnalyticsService.ts) and keeps the timings out of the Effect environment. Consider a Config.all({ ttlMs: Config.integer("PIKU_ORCH_RUN_LEASE_TTL_MS").pipe(Config.withDefault(30_000)), ... }) descriptor yielded where the timings are produced, keeping the Context.Reference as the test/override seam. The same applies to the module-load IIFE at apps/server/src/orchestration-v2/EffectWorker.ts:336-341.

4. apps/server/src/orchestration-v2/EventSink.ts:45-61 — EventSinkIllegalTransitionError

  • detail is prose derived from the attributes the error already carries (kind, from, to), and message is built from it. Consider dropping detail and deriving the message from the structural fields directly, e.g. return `Rejected orchestration V2 event: a ${this.kind} ${this.entityId} in terminal status "${this.from}" cannot move to "${this.to}".`;
  • The error is not part of the declared error channel (EventSinkV2Error = Schema.Union([EventSinkWriteError, EventSinkStreamError])), so the boundary mappers at lines 578-627 bury this structured domain failure inside EventSinkWriteError.cause as an opaque defect — which is why RunLifecycleGuarantees.test.ts has to inspect .cause with a predicate instead of using Effect.catchTags. Consider adding EventSinkIllegalTransitionError to the union and passing it through the boundary mappers, wrapping only genuinely lower-level failures.

Posted via Macroscope — Effect Service Conventions

@macroscopeapp

macroscopeapp Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Unable to determine

Macroscope's correctness review was unable to post its findings for this PR. Approvability cannot proceed without a successful correctness review.

You can customize Macroscope's approvability policy. Learn more.

@Iweisc Iweisc closed this Aug 2, 2026
@Iweisc

Iweisc commented Aug 2, 2026

Copy link
Copy Markdown
Author

gh pr create silently resolved the fork's upstream remote as the base repo.....this was meant for my own fork. Sorry guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants