mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 11:20:42 +01:00
fix: count a recovered shutdown interrupt as recovered (#15031)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The dashboard success rate counts recovered runs as successes
> - A run is recovered when a later retry in its chain succeeds
> - The summary puts `failed` and `timed_out` into recovered
> - An `interrupted` run, including `server_shutdown_interrupted`, falls
through to other
> - The success rate stays down after the retry succeeds
> - This pull request counts a recovered interrupt in recovered, not in
other
> - The benefit is that a shutdown that was retried successfully no
longer looks like a failed day
## Linked Issues or Issue Description
No open issue covers this. I searched open and closed issues and pull
requests.
**What happened?**
`dashboardService.summary` puts only `failed` and `timed_out` into the
recovered bucket. An `interrupted` run, including
`server_shutdown_interrupted`, goes to other even when `recovered` is
set. The success rate uses `succeeded + recovered`, so the day stays low
after a successful retry.
**Expected behavior**
When the recovered flag is set, count `interrupted` in recovered. An
interrupt with no successful retry stays in other.
**Steps to reproduce**
1. Record an interrupted run with error code
`server_shutdown_interrupted`.
2. Record a succeeded retry of that run.
3. Open the dashboard summary for that day.
4. Observe that the interrupt is in other and the success rate does not
include it.
**Paperclip version or commit**
master at 215586d127
**Deployment mode**
Local dev (pnpm dev)
## What Changed
- When a run is recovered, count status `interrupted` in the recovered
bucket.
- Leave an unrecovered interrupt in other.
- Add a service test for a recovered shutdown and a stranded shutdown.
## Verification
- `pnpm exec vitest run src/__tests__/dashboard-service.test.ts --config
vitest.config.ts` in `server`
- Result: 1 file passed, 5 tests passed
## Risks
- Low risk. Unrecovered interrupts stay in other. Failed and timed-out
runs keep the same buckets.
## Model Used
- No model runtime is part of this change.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge
This commit is contained in:
1 parent
b46d1ab77b
commit
61ecc93033
3 files changed
+63
-8
No files matched your search
@@ -7,10 +7,10 @@ export interface DashboardRunActivityDay {
|
||||
*/
|
||||
failed: number;
|
||||
/**
|
||||
* Runs that terminated in a failure state (failed/timed_out) but whose retry
|
||||
* chain eventually succeeded — e.g. restart-killed runs that recovered. Kept
|
||||
* out of `failed` so the headline failure count reflects true, unrecovered
|
||||
* failures.
|
||||
* Runs that terminated as failed, timed_out, or interrupted (shutdown) but
|
||||
* whose retry chain eventually succeeded — e.g. restart-killed runs and
|
||||
* shutdown interrupts that recovered. Kept out of `failed` so the headline
|
||||
* failure count reflects true, unrecovered failures.
|
||||
*/
|
||||
recovered: number;
|
||||
other: number;
|
||||
|
||||
@@ -274,4 +274,54 @@ describeEmbeddedPostgres("dashboard service", () => {
|
||||
// process_lost kills that recovered must not leak into the failed breakdown.
|
||||
expect(bucket?.failedByErrorCode.process_lost).toBeUndefined();
|
||||
});
|
||||
|
||||
it("counts a recovered shutdown interrupt toward recovered, not other", async () => {
|
||||
const companyId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const day = utcDay(-1);
|
||||
const shutdown = randomUUID();
|
||||
const shutdownRetry = randomUUID();
|
||||
const stranded = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "CodexCoder",
|
||||
role: "engineer",
|
||||
status: "running",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
|
||||
const base = {
|
||||
companyId,
|
||||
agentId,
|
||||
invocationSource: "assignment" as const,
|
||||
createdAt: day,
|
||||
};
|
||||
await db.insert(heartbeatRuns).values([
|
||||
{ ...base, id: shutdown, status: "interrupted", errorCode: "server_shutdown_interrupted" },
|
||||
{ ...base, id: shutdownRetry, status: "succeeded", retryOfRunId: shutdown },
|
||||
{ ...base, id: stranded, status: "interrupted", errorCode: "server_shutdown_interrupted" },
|
||||
]);
|
||||
|
||||
const summary = await dashboardService(db).summary(companyId);
|
||||
const bucket = summary.runActivity.find((entry) => entry.date === utcDateKey(day));
|
||||
expect(bucket).toMatchObject({
|
||||
succeeded: 1,
|
||||
recovered: 1,
|
||||
failed: 0,
|
||||
other: 1,
|
||||
total: 3,
|
||||
failedByErrorCode: {},
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -103,9 +103,10 @@ export function dashboardService(db: Db) {
|
||||
const monthSpendCents = Number(monthSpend);
|
||||
// Per-day run breakdown. A run is "recovered" when its retry chain later
|
||||
// succeeded (recovered_runs = all ancestors of a succeeded retry), so a
|
||||
// restart-killed run whose retry succeeded is pulled out of the headline
|
||||
// failed count. error_code is carried through so a failure spike can be
|
||||
// attributed to an error class (e.g. process_lost, provider_quota).
|
||||
// restart-killed or shutdown-interrupted run whose retry succeeded is
|
||||
// pulled out of the headline failure count. error_code is carried through
|
||||
// so a failure spike can be attributed to an error class (e.g. process_lost,
|
||||
// provider_quota).
|
||||
// Both recursive arms are bounded to the chart window: a retry is always
|
||||
// created after the run it retries, so ancestors of an out-of-window
|
||||
// child are themselves out of window and invisible to the membership
|
||||
@@ -166,7 +167,11 @@ export function dashboardService(db: Db) {
|
||||
const recovered = row.recovered === true || row.recovered === "t" || row.recovered === "true";
|
||||
if (status === "succeeded") {
|
||||
bucket.succeeded += count;
|
||||
} else if (status === "failed" || status === "timed_out") {
|
||||
} else if (
|
||||
status === "failed" ||
|
||||
status === "timed_out" ||
|
||||
(recovered && status === "interrupted")
|
||||
) {
|
||||
if (recovered) {
|
||||
bucket.recovered += count;
|
||||
} else {
|
||||
|
||||
Reference in new issue
Block a user