Agent-runtime audit fixes (Phase 1 complete; 2 & 3 in progress)#16
Merged
Conversation
…-time from startedAt Phase-1 safety fixes for the goal/execution-plan loop (from the agent-runtime audit): - cascadeReadiness / transitionStepToReady now early-return unless the plan is RUNNING. A BLOCKED (budget/wall-time) or CANCELED (abandon/replan) plan no longer cascades finishing steps into fresh dispatches — closing the hole where a blocked plan kept spending the exact budget the operator was asked to approve. - recordVerdict rejects a verdict on a settled step (DONE/BLOCKED/CANCELED) with CONFLICT. A stale/duplicate reviewer webhook can no longer move a DONE step back to TODO and re-dispatch, un-completing finished work. - ExecutionPlan.startedAt (migration 0091) stamped in activatePlan; wall-time budget measured from it, not createdAt, so planning + approval-wait isn't charged against the execution clock. - Add @@index([status,lastEventAt]) + @@index([assignmentEventId]) on AgentRun so the cross-tenant 5s/60s worker sweeps stop sequential-scanning. 4 new regression tests; all 24 orchestration tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pagate approve/stop failures
Phase-1 runtime-backend safety fixes (agent-runtime audit):
- pollActiveRuns treats getStatus 'unknown' as NON-terminal (leaves the run
ACTIVE, lets the stale watchdog arbitrate) instead of false-STALLing. This is
the worker-restart / deploy false-STALL for Codex, and any momentary blip.
- hermes-runs mapStatus defaults an unrecognized non-empty lifecycle word
('initializing', 'preparing', …) to 'running', not 'unknown'.
- On a live 'running' poll always bump lastEventAt, even when the step label is
unchanged, so a quiet-but-alive run (mid long tool call) isn't watchdog-STALLED.
- hermes-runs approve/stop now inspect res.ok and throw on failure; the agent-run
approve/reject mutation surfaces it as an error instead of clearing the block
and returning ok — no more false 'approved' while the provider run stays blocked.
- runtime.register now calls assertEndpointTransport like create/update, so a
plaintext public endpoint can't slip in via the register path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t blip Phase-1 daemon reliability fixes (agent-runtime audit): - acquirePidLock() writes the daemon pid via an atomic O_CREAT|O_EXCL open BEFORE the SSE stream opens, closing the TOCTOU where two racing 'forge daemon start' invocations both passed the liveness check and both dispatched every event. A loser that finds a live holder refuses; a stale holder is reclaimed. - refreshLinkedAgent distinguishes a transient failure (undefined) from a genuine unlink (null); the heartbeat loop only overwrites linkedAgent on a definitive answer, so one soft agents.me blip no longer nulls the linkage and silently drops all chat + assignment dispatch for up to 60s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ngs dead-end Phase-1 runtime/orchestration UX fixes (agent-runtime audit): - Goals list/detail and Plans list/detail now render a themed 'couldn't load … Retry' state on a fetch error, instead of masking it as 'No goals yet' / 'Goal not found — may have been abandoned', which read as though work was deleted. - global.runtimes returns each runtime's home workspace; the global Runtimes settings page routes the settings gear via workspacesInUse[0] ?? homeWorkspace, so a freshly registered LOCAL_DAEMON (no workspacesInUse yet) is no longer a read-only dead-end you can't self-test or add secrets to. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… wall-time watchdog Phase-2 lifecycle + budget integrity (agent-runtime audit): - reapPlanRuns(): abandonGoal + re-decompose/re-generate now cancel non-terminal steps and stop/abandon their in-flight AgentRuns (best-effort connector.stop), so an abandoned goal or a superseded plan stops spending immediately instead of burning tokens until the stale watchdog reaps it. A superseded prior RUNNING plan is CANCELED, not just demoted, so two plans can't drive one goal. - assertNoStepCycles(): plan creation (insertStepsTx + createExecutionPlan) rejects a dependency cycle with BAD_REQUEST via Kahn's algorithm — a mutually-dependent step pair no longer leaves the plan RUNNING forever with nothing ready. - sweepOrchestrationBudget(): new 60s worker watchdog runs checkAndBlockBudget on every RUNNING plan with a wall-time cap (enforcing time independent of cost reporting — was only ever checked on a cost record) and logs wedged plans. Tests: cycle rejection + abandon-cancels-steps; 26 orchestration tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sabled/unresolvable handling
Phase-2 dispatch fixes (agent-runtime audit):
- Poll-mirrored RUNS cost now flows into applyRunCostToPlan (delta vs the run's
last-known cost, persisted each tick), so a Hermes/Codex RUNS agent that reports
cost only via poll actually trips the plan's maxTotalCostUsd instead of burning
past it.
- Enforce the agent's maxConcurrent on the RUNS dispatch path (startNewRuns), not
just in auto-dispatch selection — a maxConcurrent=1 agent can no longer hold N
concurrent provider runs from manual/mention/watcher/rule assignments.
- A disabled runtime's in-flight runs are left alone (disable blocks NEW dispatch
only) instead of being false-STALLed by the sentinel connector.
- An unresolvable connector annotates the run ("runtime unresolvable — check
config") instead of silently spinning, and doesn't bump lastEventAt so the
watchdog can still reap it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hen no admin exists Phase-2 security fixes (agent-runtime audit): - setMemberRole: only an OWNER may grant the OWNER role or change an existing owner's role. Closes the privilege-escalation where any workspace ADMIN could set its own row to OWNER (the server accepted any Role) and then delete the founder's tenant, making the OWNER-only archive/delete gate meaningless. - ADMIN_EMAIL env bootstrap (instanceAdminProcedure + requireInstanceAdmin) is now honored ONLY while no INSTANCE_ADMIN exists yet. auth.ts no longer re-stamps INSTANCE_ADMIN on every credentials login (only bootstraps when none exists). Together: a demoted operator stays demoted, and a shared / recycled / SSO- asserted ADMIN_EMAIL can't silently regain full instance control. Test: non-owner admin can't self-promote to OWNER; owner can. 18 members tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase-2 daemon reliability (agent-runtime audit): - On every SSE (re)connect the daemon now pulls its agent's UNACKED inbox (agent.inbox.list) and drives each pending assignment through the same handler as a live event, deduped against seenEvents by the triggering event id. This recovers AGENT_ASSIGNED events dropped while the daemon was stopped or mid- reconnect — the live-only SSE has no replay, so they were previously lost. - Fatal-auth backstop: consecutive 401/403 SSE errors past a threshold log 'run forge login' and exit non-zero, so a revoked/expired token surfaces to a supervisor instead of looping forever while 'daemon status' reads healthy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase-2 budget integrity (agent-runtime audit): recordUsage read the run's prior cost outside any lock, so two concurrent calls (or a call racing the poll sweep) could both compute a delta from the same base and double-count against the plan budget. The prev-read + update now run in one tx behind a row lock on the AgentRun, so the prev→new delta applied to the plan/goal totals is correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…licates Phase-2 (agent-runtime audit): runtimes.register always CREATEd, so a daemon that lost its cached runtime id (fresh clone / config wipe) stacked a duplicate row per re-register. It now reuses a non-archived match on (workspaceId, name, kind), refreshing endpoint/providers/heartbeat, and only creates when none exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y-run Implements the operator's explicit ask #2: move issues to another workspace as an instance admin. Issue is the hub of ~14 workspace-scoped child tables with per-workspace unique constraints, so a naive workspaceId re-stamp corrupts data — this does it safely: - planIssueMove() (read-only dry-run): renumber map (AXI-42 → WRK-108), status remap by category, label remap by name (drops unmatched), FKs nulled across the tenant boundary (project/cycle/agents), parent kept only if also moving, dropped cross-workspace relations, attachment-quota delta, and per-issue BLOCK reasons. - executeIssueMove(): runs the plan in one transaction — re-tenants the issue + its workspaceId-bearing content children (comments/attachments/time/watchers/ external links), renumbers, remaps status + labels, drops cross-ws relations, and writes MOVE audit events in BOTH workspaces. - Safety: REFUSES to move an entangled issue (agent runs, orchestration links, action requests, artifacts) rather than risk a cross-tenant reference; the preview reports which and why. instanceAdmin-gated (references two tenants). - MCP-exposed as instanceAdmin.previewIssueMove / moveIssues. Tests: preview+block, execute (re-tenant/renumber/remap/audit), refuse-all-blocked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds /admin/move-issues (instance-admin): pick source + target workspace + paste issue ids, Preview shows the full remap table (new keys, status/label remap, blocked-issue reasons, quota warning), then a type-confirmed Move executes it. Uses themed primitives (Combobox, useConfirm, admin panels) — no native controls. New 'Move issues' nav entry in the admin shell. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eam-D jargon Phase-3 governance + housekeeping (agent-runtime audit): - Governance: MAX_WORKSPACES_PER_USER env caps self-service tenant creation for non-instance-admins (default 0 = unlimited; no behavior change until set). - Docs: rewrote CLAUDE.md's stale CLI 'v1 gaps' (all closed — chat.getThread, full issue-loop, runtimes.list/agents.list shipped; reconcile + fatal-auth added; real remaining gaps noted). Corrected run-dispatcher's 'restart-safe' claim to Hermes-only (Codex 'unknown'-after-restart now non-terminal). - Copy: stripped internal 'Stream D' jargon from the runtime detail settings page. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase-3 slices (agent-runtime audit): - P3.3: cached-token usage now flows end-to-end for RUNS agents — RunStatus.usage + the 'usage' RunEvent carry tokensCached, the Hermes connector extracts it from the provider response (cached_tokens / cache_read_input_tokens), and the poll loop's terminal write persists it onto AgentRun.tokensCached (was always null). - P3.2: the workspace members role picker is now the themed Combobox instead of a native <select> (pairs with the P2.5 owner-only guard, which toasts a clear error if a non-owner tries to grant OWNER). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Finish the P3.2 token sweep across the orchestration + members surfaces — replace ad-hoc emerald-*/amber-* tailwind with the semantic --success / --warning tokens (which already flip light/dark, so the redundant dark: variants collapse). No visual change intended; brings these files in line with the run-row/step-node retry chips that already use the tokens. - plans + goals pages (in-flight): judge verdicts, progress bar, plan status dots → bg-success / bg-warning - orchestration-ui/status.ts: goal PLANNING/ACHIEVED tones - orchestration/crew-roster-panel.tsx: REVIEWER/OPERATOR_PROXY role tones - orchestration/step-node.tsx: awaiting-approval ring + approval chip - settings/runtimes: serves-chat badge, disabled badge, tier-1 explainer Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P3.2 observability items — make the orchestration surfaces update in place and let the operator act without a detour to Command Center. - Plan detail: wire the P1.7 `startedAt` into the budget meter's wall-time bar (elapsed / cap), mirroring the goal cockpit. `hasBudget` now also trips on a wall-time cap so a time-only budget still renders. - Plans index: subscribe to realtime (execution-plan/step/agent-run) so background dispatch progress no longer looks frozen until manual Refresh. Mirrors the goals index. - Review inbox: realtime on review-gate events (a crew member requesting approval, or another operator resolving) + tokenize the status tones (amber/emerald/destructive → warning/success/danger — REJECTED now actually reads red instead of the undefined `destructive` class). - RunRow: replace the "open Command Center to approve" dead-end notice with the shared RunApprovalCard (Approve session/once + Reject inline), which its own doc comment already claimed it hosted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lects P3.2 native-select sweep for the orchestration + runtime-settings surfaces — replace every raw <select> in these files with the themed Combobox primitive (portaled popover, keyboard nav, no native control): - settings/runtimes: Adapter, Codex Sandbox mode, Approval policy - goals/[goalId]: crew picker (allowNone → "No crew") - plans/[planId]: plan-status + both step-status transition pills These three files are now 100% native-select-free (the app-wide backlog guard stays a warn; other surfaces untouched). e2e: runtime-management.spec.ts drove the sandbox-mode + adapter selects via Playwright selectOption, which only works on native <select>. Rewire both to the Combobox contract — click the role="combobox" trigger by its aria-label, then click the role="option" row; assert the selected label via toContainText instead of toHaveValue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
P3.2 modal item — replace the three hand-rolled orchestration modals with the shared QuickForm primitive: one graphite scrim + backdrop-blur (was a mix of bg-black/40 and bg-foreground/20), consistent header/footer, Enter -to-submit + Esc-to-close, draft-safe fields, and an inline error banner. - plans: New-plan modal (title + template) — onSubmit reuses submit(); empty title now banners instead of a disabled button. - goals: New-goal modal (objective + planner context + CrewSelector). - goals/[goalId]: GoalEditModal (title/description/crew/cost+wall caps) — onSave is now awaited so a failing update keeps the modal open with the server message in-banner rather than a fire-and-forget toast. The goal-router shim (use-goal-trpc) exposes `mutateAsync` too — it already wraps the real tRPC mutation, so this only surfaces what exists at runtime, letting the create/edit forms await the call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the roadmap from the agent-runtime audit (73 findings; report artifact). Landing in phases on this branch; Phase 1 is complete, Phase 2 + 3 in progress.
Phase 1 — safety / lifecycle / false-terminal / dead-ends (done)
Orchestration loop
plan.status===RUNNING— a BLOCKED (budget) or CANCELED (abandon/replan) plan stops dispatching finishing steps into fresh spend.recordVerdictrejects a verdict on a settled step (DONE/BLOCKED/CANCELED) → a stale/duplicate reviewer webhook can't reopen completed work.ExecutionPlan.startedAt(migration0091) → wall-time budget measured from execution start, not decompose.Runtime backend
getStatus 'unknown'is now non-terminal (stale watchdog arbitrates) → fixes the Codex worker-restart / deploy false-STALL; unrecognized Hermes statuses map torunning.runningrun so a quiet-but-alive run isn't watchdog-STALLED.approve/stopinspectres.ok+ throw; the approve/reject mutation surfaces failure instead of a false success.runtime.registerenforces the TLS transport guard like create/update.AgentRunindexes for the cross-tenant 5s/60s sweeps.CLI daemon
O_EXCLpid lock closes the double-daemon TOCTOU.agents.meblip (was: dropped all dispatch ~60s).UI
Verification:
pnpm typecheckclean ·pnpm lint0 errors · orchestration suite 24 passed (4 new regression tests) ·pnpm build:cliclean.Phase 2 (in progress)
Budget integrity (poll-cost → plan budget), abandon/replan run teardown, dep-cycle detection + stuck-plan watchdog, maxConcurrent on the RUNS path, the two security holes (admin→owner self-promotion; ADMIN_EMAIL bootstrap), Codex restart reconciliation, daemon reconcile-on-reconnect + fatal-auth.
Phase 3 (in progress)
Cross-workspace move (instance-admin, dry-run remap), native-
<select>migration + token colors, Hermes usage frames / ACP / multi-agent-per-runtime, view-hierarchy legibility, housekeeping.🤖 Generated with Claude Code