Fix local polling bug

This commit is contained in:
James Brunton
2026-08-28 13:40:00 +01:00
parent 12b05a59c1
commit 1f85515154
4 changed files with 75 additions and 31 deletions
@@ -45,16 +45,12 @@ export interface PolicyRunRecord {
/** Set while an auto-retry is pending after a transient (queue-full) rejection, so the activity
* feed shows a soft "busy" row instead of a hard failure during the backoff window. */
retrying?: boolean;
/** A run computed entirely in the browser - it has no server run behind it,
* so it must never be polled for status (a status poll 404s and would flip a
* succeeded run to FAILED) or reconciled against the server. */
browserLocal?: boolean;
/** Epoch ms when the run was dispatched. */
startedAt: number;
/**
* Ran in the browser (the local classification heuristic), not on a backend. Such a run has no
* server-side status to poll, and - crucially - must NOT claim the (policy, file) dispatch key:
* it is the first pass, not the policy's run, so claiming it would suppress the server run the
* verdict may still need to escalate to. Distinct from {@link target}, which says which BACKEND
* holds a real run's outputs.
*/
browserLocal?: boolean;
}
/** Statuses of a run that is still executing (not yet settled). */
@@ -224,15 +220,11 @@ export function recordRunStart(record: PolicyRunRecord) {
const waveStartedAt = state.runs.some(isRunInFlight)
? state.waveStartedAt
: record.startedAt;
// A browser-local run is the first pass, not the policy's run: claiming the dispatch key here
// would permanently suppress the server run its verdict may still need to escalate to.
const claimsDispatch = !record.browserLocal;
state = {
runs: capRuns([record, ...state.runs]),
dispatched:
!claimsDispatch || state.dispatched.includes(key)
? state.dispatched
: [...state.dispatched, key],
dispatched: state.dispatched.includes(key)
? state.dispatched
: [...state.dispatched, key],
waveStartedAt,
};
emit();
@@ -26,12 +26,6 @@ import {
orderedRewritingCategories,
} from "@app/data/classificationPolicy";
/**
* Dispatch-store key namespace for "this file's local pass has been metered". Deliberately NOT the
* Classification category id: that key is the server escalation's own guard, so metering under it
* would tell the auto-run the policy had already run and kill the escalation entirely.
*/
export const LOCAL_METER_CATEGORY = `${CLASSIFICATION_CATEGORY_ID}:local-meter`;
/** Files classified per idle pass, so a large library drains over several ticks. */
const CLASSIFY_BATCH = 3;
/** How long to wait for an upload's bytes to land in IndexedDB (20 × 250ms ≈ 5s).
@@ -147,7 +141,7 @@ export function useClassificationPolicy(): void {
if (claimed.current.has(key)) continue;
claimed.current.add(key);
const verdict = await classifyStub(
stub.id,
stub.id as FileId,
stub.name,
stub.size ?? 0,
);
@@ -156,11 +150,11 @@ export function useClassificationPolicy(): void {
if (verdict == null) continue;
// Deliver unconditionally - a re-render must never discard a computed
// (and already metered) result. Writes are idempotent.
updateStirlingFileStub(stub.id, {
updateStirlingFileStub(stub.id as FileId, {
classificationLabels: verdict.labels,
classificationConfidence: verdict.confidence,
});
const ok = await fileStorage.updateFileMetadata(stub.id, {
const ok = await fileStorage.updateFileMetadata(stub.id as FileId, {
classificationLabels: verdict.labels,
classificationConfidence: verdict.confidence,
});
@@ -230,7 +224,9 @@ async function classifyStub(
// A local run is still a billable policy run, so it belongs in the activity feed; recorded only
// once the bytes are in hand, so a file whose bytes never land leaves no phantom row.
const alreadyMetered = isDispatched(LOCAL_METER_CATEGORY, fileId);
// Read before recordRunStart, which takes the dispatch key itself and would otherwise always
// answer "already dispatched", silently stopping metering.
const alreadyMetered = isDispatched(CLASSIFICATION_CATEGORY_ID, fileId);
const runId = `local-${CLASSIFICATION_CATEGORY_ID}-${fileId}-${Date.now()}`;
recordRunStart({
runId,
@@ -239,8 +235,7 @@ async function classifyStub(
fileName,
fileSize,
target: "local",
// Ran here, not on a backend: nothing to poll, and it must not claim the classification
// dispatch key - that key is what the server escalation checks before running.
// The heuristic ran in the browser - there is no server run to poll (see the poll effect).
browserLocal: true,
status: "RUNNING",
outputs: [],
@@ -271,7 +266,7 @@ async function classifyStub(
labels,
});
}
markDispatched(LOCAL_METER_CATEGORY, fileId);
markDispatched(CLASSIFICATION_CATEGORY_ID, fileId);
// Labels, no output file - the same settle shape the server-run classification uses.
updateRun(runId, {
status: "COMPLETED",
@@ -59,12 +59,14 @@ import { usePolicyAutoRun } from "@app/components/policies/usePolicyAutoRun";
import {
recordRunStart,
updateRun,
getRun,
resetPolicyRuns,
} from "@app/components/policies/policyRunStore";
import { runStoredPolicy } from "@app/services/policyApi";
import { runStoredPolicy, getPolicyRun } from "@app/services/policyApi";
import { fileStorage } from "@app/services/fileStorage";
const runStored = vi.mocked(runStoredPolicy);
const getRunStatus = vi.mocked(getPolicyRun);
const getFile = vi.mocked(fileStorage.getStirlingFile);
/** Reset the shared file list between tests without swapping the array identity. */
@@ -176,4 +178,53 @@ describe("auto-run ordered chaining", () => {
expect(runStored).not.toHaveBeenCalled();
});
it("never polls a browser-local run (no server behind it) so its success can't 404 to FAILED", async () => {
getRunStatus.mockResolvedValue({
runId: "srv-1",
policyId: null,
status: "COMPLETED",
currentStep: 1,
stepCount: 1,
error: null,
outputs: [],
} as never);
// A browser-local heuristic run and a real server run, both left in flight.
recordRunStart({
runId: "local-1",
categoryId: "classification",
fileId: "f1",
fileName: "d.pdf",
fileSize: 1,
target: "local",
browserLocal: true,
status: "RUNNING",
outputs: [],
error: null,
startedAt: 0,
});
recordRunStart({
runId: "srv-1",
categoryId: "security",
fileId: "f2",
fileName: "d.pdf",
fileSize: 1,
target: "saas",
status: "RUNNING",
outputs: [],
error: null,
startedAt: 0,
});
renderHook(() => usePolicyAutoRun());
await act(async () => {
await vi.advanceTimersByTimeAsync(700); // past the first poll (500ms)
});
const polled = getRunStatus.mock.calls.map((c) => c[0]);
expect(polled).toContain("srv-1"); // the server run is polled…
expect(polled).not.toContain("local-1"); // …the browser-local one never is
// And its success is left intact, not flipped to FAILED by a 404 streak.
expect(getRun("local-1")?.status).toBe("RUNNING");
});
});
@@ -278,10 +278,16 @@ export function usePolicyAutoRun(): void {
}
}, [runs, policies, orderedUploadCategories]);
// Poll each in-flight run to a terminal state.
// Poll each in-flight run to a terminal state. A browser-local run (the classification heuristic's
// first pass) has no server run behind it, so polling it 404s and would flip its success to FAILED.
useEffect(() => {
for (const run of runs) {
if (isTerminal(run.status) || polling.current.has(run.runId)) continue;
if (
run.browserLocal ||
isTerminal(run.status) ||
polling.current.has(run.runId)
)
continue;
polling.current.add(run.runId);
void poll(run.runId, onRunFinished).finally(() =>
polling.current.delete(run.runId),