Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 34 additions & 28 deletions .plan.md
Original file line number Diff line number Diff line change
@@ -1,39 +1,45 @@
# Reviewer Memory Ledger Plan
# ADA-128 Plan-Usage Dashboard Regression Plan

## Intent

Add a backward-compatible `--extra-context <file>` input to the shared autoreview helper, then have the factory pre-commit gate synthesize reviewer memory from prior audit records for the current repository worktree and branch. The prompt must reproduce the framing validated by `~/.openclaw/experiments/mkledger-validated.py`: prior findings and reworded equivalents are settled, reviewers should seek new issues, and the ledger does not imply that the code is safe.
Restore the plan-usage dashboard contract after the app-server usage migration: omit Codex Spark limits, present provider/window rows in deterministic order with an explicit Codex/Anthropic separator, and make the server continually refresh a snapshot that is already available when a client connects. The connect path must read only the current snapshot; provider probes must run exclusively in the background at a bounded interval.

## Blast radius
## Trace and blast radius

- Shared infrastructure: `~/.claude/skills/autoreview/scripts/autoreview` argument parsing and prompt assembly only. Calls without the new flag remain byte-for-byte equivalent in prompt content.
- Repository gate: `scripts/factory/precommit-gate.sh` builds a temporary ledger immediately before a non-cached autoreview invocation and conditionally adds `--extra-context`.
- Repository configuration: `scripts/factory/factory.conf` supplies the ledger history limit as a documented knob.
- Tests: autoreview stub-engine coverage plus `scripts/factory-gate.test.ts` integration coverage. No application/runtime code changes.
- Current client path: `usePlanUsage` waits for `usePreparedConnection`, then separately fetches `/api/plan-usage`; the first render can therefore show context usage before plan usage.
- Current server path: `/api/plan-usage` calls `loadPlanUsageSnapshot`; a cold cache launches Codex app-server and Claude CLI probes in the request path (up to their 10s/30s timeouts).
- `apps/server/src/usage/PlanUsage.ts`: separate probe/refresh from snapshot reads, preserve last-good values, filter Spark, and define deterministic provider/window ordering. Risk is stale or missing data after failures, duplicate background probes, or excessive probe frequency.
- Server startup/layer wiring and WebSocket initial exchange: own one scoped refresher process, expose its current aggregate snapshot, and include it with the initial client state that already carries context usage. Risk is process leaks, startup blocking, or schema/version skew.
- `packages/contracts`: extend only the existing initial-exchange schema if required. New server/old client and old server/new client must continue decoding safely.
- `apps/web`: hydrate plan usage from the connect snapshot, retain minute-scale background updates without a cold on-demand fetch, filter/sort defensively, and render an explicit provider boundary. Risk is stale state crossing environment changes or keyed rows reordering.
- No provider-account discovery, multi-account aggregation redesign, live-server deployment, or ADA-116 scope expansion.

## Edge-case matrix

| Case | Expected behavior |
| ---------------------------------------------- | ---------------------------------------------------------------------------------------- |
| `--extra-context` absent | Existing autoreview prompt and behavior are unchanged. |
| Context file empty | No extra prompt section is added. |
| Context file missing/unreadable | Fail closed with a clear CLI error. |
| First review on branch/worktree | Ledger is empty; gate passes no `--extra-context` flag. |
| Prior finding on another branch or worktree | Excluded. |
| Prior finding on current branch/worktree | Included with title, priority, file, and “addressed in a subsequent commit”. |
| More than configured finding history | Keep only the most recent approximately 15 unique findings. |
| Audited dismissal on current branch/worktree | Always included and marked “DISMISSED with audited rationale (settled design decision)”. |
| Duplicate finding/dismissal | Deduplicate stable display entries. |
| Growing ledger with unchanged HEAD/tree/config | Does not alter `GATE_ID`; cached result remains reusable. |
| Temp-file lifecycle / interruption | File is created under gate state/temp storage and removed by a trap. |
| Malformed audit JSON line | Ignore the malformed line; do not make the gate unavailable. |

## Red-first test plan

1. Extend the autoreview stub-engine fixture to capture its prompt and assert non-empty extra context appears after fixed reviewer instructions and before the change bundle; assert absent/empty context does not appear.
2. Add gate integration tests that are red on the old gate: first round has no `--extra-context`; after an audited finding, the next staged tree receives a ledger containing it; after a valid audited dismissal, the next staged tree receives the settled dismissal framing.
3. Run the focused factory-gate test, the autoreview self/stub test, then required `vp check` and `vp run typecheck`.
| Case | Input/event | Expected result |
| --------------------------------------------- | --------------------------------------------------------- | --------------------------------------------------------------------------------- |
| Spark-only Codex bucket | `limitName`/title identifies gpt-5.3 Codex Spark | Bucket is absent; empty provider may be omitted |
| Mixed Codex buckets | Spark interleaved with ordinary primary/secondary windows | Only plan windows remain, in fixed key order |
| Provider results resolve in either order | Claude first on one refresh, Codex first on the next | Codex section then Anthropic section every render |
| Same provider windows arrive shuffled | Equal data with reversed source order | Identical ordered ids and React keys |
| Both providers visible | One or more windows each | Clear horizontal divider occurs exactly at provider transition |
| Client connects after refresh | Context and plan snapshot already cached | Same initial exchange contains both; no probe starts from connect handler |
| Client connects before first successful probe | Background warm-up still pending | Initial exchange contains an empty snapshot immediately; later refresh updates it |
| Probe fails after prior success | Provider temporarily unavailable | Last-good snapshot remains readable; refresher retries at the normal interval |
| Many clients connect together | N initial handshakes | Zero additional provider probes; all read the same current snapshot |
| Settings/provider scope changes | Enabled instances or credentials change | Next bounded refresh recomputes the correct scope without hammering APIs |
| Polling disabled | Environment flag set | Empty snapshot, no background probes |
| Disconnect/reconnect/environment switch | Hook changes connection key | No old-environment snapshot flashes in the new environment |

## Red-first guardrails

1. Unit RED: a Codex app-server response containing normal and Spark buckets currently exposes Spark; assert only normal Codex plan windows survive.
2. Unit RED: feed provider/window collections in opposite orders and assert the flattened display model has the same fixed provider/window order.
3. Component RED: render Codex plus Claude rows and assert an accessible/styled separator exists at the provider boundary.
4. Integration RED: connect a WebSocket client while the probe double is deliberately blocked; assert the initial exchange already includes context usage and the current plan-usage snapshot, and assert the connect handler did not invoke the probe.
5. Background-service tests: use fake time/probe counters to assert immediate warm-up, sane interval refresh, one in-flight refresh, last-good retention, and scoped shutdown.
6. E2E: on a disposable T3 server, connect through the real UI and capture evidence that context and plan usage appear from the initial connection state; verify stable ordering and the provider divider on rerender/reconnect.

## Smallest-change argument

The helper gains one optional file-backed prompt segment and no new review semantics. The gate derives memory directly from its existing append-only audit source at invocation time, avoiding a second state store or migration. The ledger is deliberately excluded from `GATE_ID`: cache identity remains HEAD + staged tree + reviewer configuration, while the history is advisory context that grows after a review and must not invalidate an already-bound result.
Reuse the existing official app-server/Claude probe and parsing code from the #162 path. Add one server-owned snapshot/refresher boundary around it rather than inventing another provider API or polling mechanism. Keep filtering and canonical ordering close to normalization, with a defensive pure display-model helper for UI stability. Extend the existing connection snapshot/stream rather than adding another request on connect. Preserve the HTTP/MCP readers as snapshot consumers where compatibility requires them, but make them read-only so no user request can launch a probe.
10 changes: 3 additions & 7 deletions apps/server/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import * as HttpApiBuilder from "effect/unstable/httpapi/HttpApiBuilder";
import { OtlpTracer } from "effect/unstable/observability";

import * as ServerConfig from "./config.ts";
import * as ServerSettings from "./serverSettings.ts";
import {
ASSET_ROUTE_PREFIX,
FALLBACK_PROJECT_FAVICON_SVG,
Expand All @@ -56,7 +55,7 @@ import {
normalizeCorsOrigin,
} from "./httpCors.ts";
import { buildElevenLabsRequest, resolveVoiceId, validateTtsText } from "./tts/ttsRequest.logic.ts";
import { loadPlanUsageSnapshot } from "./usage/PlanUsage.ts";
import * as PlanUsageSnapshot from "./usage/PlanUsageSnapshot.ts";
import {
SUBAGENT_PEER_MCP_TOKEN_PATH,
SubagentPeerMcpTokenRequest,
Expand Down Expand Up @@ -467,11 +466,8 @@ export const planUsageRouteLayer = HttpRouter.add(
const providerInstanceId = rawProviderInstanceId
? Option.getOrNull(decodedProviderInstanceId)
: null;
const serverSettings = yield* ServerSettings.ServerSettingsService;
const settings = yield* serverSettings.getSettings;
const snapshot = yield* Effect.promise(() =>
loadPlanUsageSnapshot({ providerInstanceId, settings }),
);
const planUsage = yield* PlanUsageSnapshot.PlanUsageSnapshotStore;
const snapshot = yield* planUsage.read(providerInstanceId);

return HttpServerResponse.jsonUnsafe(snapshot, {
headers: {
Expand Down
7 changes: 7 additions & 0 deletions apps/server/src/mcp/McpOfficialClientConformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { ProjectionSnapshotQuery } from "../orchestration/Services/ProjectionSna
import { ProviderRegistry } from "../provider/Services/ProviderRegistry.ts";
import { makeProviderRegistryMock } from "../provider/testUtils/providerRegistryMock.ts";
import { ServerSettingsService } from "../serverSettings.ts";
import * as PlanUsageSnapshot from "../usage/PlanUsageSnapshot.ts";
import * as McpHttpServer from "./McpHttpServer.ts";
import * as McpInvocationContext from "./McpInvocationContext.ts";
import * as McpSessionRegistry from "./McpSessionRegistry.ts";
Expand Down Expand Up @@ -153,6 +154,12 @@ const conformanceLayer = McpHttpServer.layer.pipe(
Layer.provide(PreviewAutomationBroker.layer.pipe(Layer.provide(NodeServices.layer))),
Layer.provide(deviceNotificationsLayer),
Layer.provide(ServerSettingsService.layerTest()),
Layer.provide(
PlanUsageSnapshot.layerTest({
updatedAt: "2026-07-10T08:00:00.000Z",
providers: [],
}),
),
Layer.provide(Layer.succeed(ProviderRegistry, makeProviderRegistryMock([makeProvider()]))),
Layer.provide(projectionSnapshotQueryLayer),
);
Expand Down
7 changes: 7 additions & 0 deletions apps/server/src/mcp/toolkits/usage/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as Layer from "effect/Layer";
import { McpSchema, McpServer } from "effect/unstable/ai";

import { ServerSettingsService } from "../../../serverSettings.ts";
import * as PlanUsageSnapshot from "../../../usage/PlanUsageSnapshot.ts";
import * as McpInvocationContext from "../../McpInvocationContext.ts";
import { UsageToolkitRegistrationLive } from "../../McpHttpServer.ts";

Expand Down Expand Up @@ -63,6 +64,12 @@ const withPlanUsagePollingDisabled = <A, E, R>(effect: Effect.Effect<A, E, R>) =
const makeLayer = () =>
UsageToolkitRegistrationLive.pipe(
Layer.provideMerge(McpServer.McpServer.layer),
Layer.provide(
PlanUsageSnapshot.layerTest({
updatedAt: "2026-07-10T08:00:00.000Z",
providers: [],
}),
),
Layer.provide(ServerSettingsService.layerTest(DEFAULT_SERVER_SETTINGS)),
);

Expand Down
21 changes: 3 additions & 18 deletions apps/server/src/mcp/toolkits/usage/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";

import { ServerSettingsService } from "../../../serverSettings.ts";
import { loadPlanUsageSnapshot } from "../../../usage/PlanUsage.ts";
import { PlanUsageSnapshotStore } from "../../../usage/PlanUsageSnapshot.ts";
import * as McpInvocationContext from "../../McpInvocationContext.ts";
import { GetUsageToolError, UsageToolkit, type GetUsageInput } from "./tools.ts";

Expand All @@ -12,26 +11,12 @@ const toToolError = (message: string, error: unknown): GetUsageToolError =>
});

const makeHandlers = Effect.fn("UsageToolkit.makeHandlers")(function* () {
const serverSettings = yield* ServerSettingsService;
const planUsage = yield* PlanUsageSnapshotStore;

const getUsage = (input: GetUsageInput) =>
McpInvocationContext.requireProviderMcpCapability("thread-management").pipe(
Effect.mapError((error) => toToolError("MCP credential is not authorized for usage", error)),
Effect.andThen(
serverSettings.getSettings.pipe(
Effect.mapError((error) => toToolError("Failed to read server settings", error)),
Effect.flatMap((settings) =>
Effect.tryPromise({
try: () =>
loadPlanUsageSnapshot({
settings,
providerInstanceId: input.providerInstanceId ?? null,
}),
catch: (error) => toToolError("Failed to load provider usage", error),
}),
),
),
),
Effect.andThen(planUsage.read(input.providerInstanceId ?? null)),
);

return {
Expand Down
11 changes: 11 additions & 0 deletions apps/server/src/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import { makeManualOnlyProviderMaintenanceCapabilities } from "./provider/provid
import * as ServerLifecycleEvents from "./serverLifecycleEvents.ts";
import * as ServerRuntimeStartup from "./serverRuntimeStartup.ts";
import * as ServerSettings from "./serverSettings.ts";
import * as PlanUsageSnapshot from "./usage/PlanUsageSnapshot.ts";
import * as TerminalManager from "./terminal/Manager.ts";
import * as PreviewManager from "./preview/Manager.ts";
import * as PortScanner from "./preview/PortScanner.ts";
Expand Down Expand Up @@ -567,6 +568,12 @@ const buildAppUnderTest = (options?: {
disableListenLog: true,
disableLogger: true,
}).pipe(
Layer.provide(
PlanUsageSnapshot.layerTest({
updatedAt: "1970-01-01T00:00:00.000Z",
providers: [],
}),
),
Layer.provide(
Layer.mock(Keybindings.Keybindings)({
loadConfigState: Effect.succeed({
Expand Down Expand Up @@ -4597,6 +4604,10 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
assert.deepEqual(first.config.keybindings, []);
assert.deepEqual(first.config.issues, []);
assert.deepEqual(first.config.providers, providers);
assert.deepEqual(first.config.planUsage, {
updatedAt: "1970-01-01T00:00:00.000Z",
providers: [],
});
assert.equal(first.config.observability.logsDirectoryPath.endsWith("/logs"), true);
assert.equal(first.config.observability.localTracingEnabled, true);
assert.equal(first.config.observability.otlpTracesUrl, "http://localhost:4318/v1/traces");
Expand Down
4 changes: 3 additions & 1 deletion apps/server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import * as AgentAwarenessRelay from "./relay/AgentAwarenessRelay.ts";
import { hasCloudPublicConfig } from "./cloud/publicConfig.ts";
import { ProviderRegistryLive } from "./provider/Layers/ProviderRegistry.ts";
import * as ServerSettings from "./serverSettings.ts";
import * as PlanUsageSnapshot from "./usage/PlanUsageSnapshot.ts";
import * as ProjectFaviconResolver from "./project/ProjectFaviconResolver.ts";
import * as RepositoryIdentityResolver from "./project/RepositoryIdentityResolver.ts";
import * as WorkspaceEntries from "./workspace/WorkspaceEntries.ts";
Expand Down Expand Up @@ -375,7 +376,8 @@ const RuntimeCoreDependenciesLive = RuntimeCoreDependenciesBaseLive.pipe(
),
);

const RuntimeDependenciesLive = RuntimeCoreDependenciesLive.pipe(
const RuntimeDependenciesLive = PlanUsageSnapshot.layer.pipe(
Layer.provideMerge(RuntimeCoreDependenciesLive),
// Misc.
Layer.provideMerge(ProcessDiagnostics.layer),
Layer.provideMerge(ProcessResourceMonitor.layer),
Expand Down
Loading
Loading