mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(run-log): answer 404 for an object run log that was never written (#20539)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Each heartbeat run keeps a run log, and the board reads it through
`GET /api/heartbeat-runs/:id/log`
> - A run can store its log in object storage (`s3_segments`)
> - When a run fails before its log is written (for example in
`runOutput.begin`), the object store throws a plain `Error("Object run
log not found")`
> - The route turns that plain error into a 500, so the board shows a
server failure for a log that simply does not exist
> - The local file store already answers 404 `Run log not found` for the
same case, and the UI treats a 404 as an unavailable log
> - This pull request makes the object store throw the same 404
> - The benefit is that a missing object run log looks like a missing
log, and a real storage failure still answers 500
## Linked Issues or Issue Description
**What happened?**
`GET /api/heartbeat-runs/:id/log` answered 500 `Internal server error`
for an `s3_segments` run whose log was never written. The server error
was `Object run log not found` from the object run-log store.
**Expected behavior**
The route answers 404 `Run log not found`, the same as the local file
store. The transcript view already treats a 404 for a finished run as a
missing log, and the log viewer already treats it as an unavailable log.
A real storage failure still answers 500.
**Steps to reproduce**
1. Use S3 storage for run logs.
2. Make a run fail before its run log is written (for example in
`runOutput.begin`).
3. Read `GET /api/heartbeat-runs/<run id>/log`. It answers 500.
**Paperclip version or commit**
`master` at adb0e77a4c.
## What Changed
- `server/src/services/object-run-log-store.ts`: a read of a log with no
head (never written or deleted) throws `notFound("Run log not found")`
(HTTP 404) instead of a plain `Error`.
- `server/src/__tests__/agent-live-run-routes.test.ts`: route tests
drive the real object run-log store behind the route. A never-written
log answers 404. A store read failure still answers 500. A board user of
another company gets 404 and the store is not read.
- `server/src/services/object-run-log-store.test.ts`: the never-written
read test now expects the 404 error.
## Verification
- Red: with the old store, `answers an object run log that was never
written` fails with `expected 500 to be 404`.
- Green: `cd server && PAPERCLIP_IN_WORKTREE=false npx vitest run
src/__tests__/agent-live-run-routes.test.ts
src/services/object-run-log-store.test.ts` passes (120 tests).
## Risks
- Low risk. Other readers of the store (`feedback.ts`, issue comment
derivation, export) already catch any read error. They do not match the
old message text.
- The response for a missing object log changes from 500 to 404. The
company access checks run before the store read, so they do not change.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.
## 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
- [x] 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
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c874cfa4da
commit
bf514759f9
3 files changed
+45
-2
No files matched your search
@@ -653,6 +653,48 @@ describe("agent live run routes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["was never written", async () => { throw Object.assign(Error("missing"), { name: "NoSuchKey" }); }, 404],
|
||||
["cannot be read from storage", async () => { throw Error("storage offline"); }, 500],
|
||||
] as const)("answers an object run log that %s", async (_case, getObject, status) => {
|
||||
const { createObjectRunLogStore } = await vi.importActual<typeof import("../services/object-run-log-store.js")>("../services/object-run-log-store.js");
|
||||
const store = createObjectRunLogStore({
|
||||
provider: {
|
||||
id: "s3",
|
||||
getObject,
|
||||
putObject: async () => { throw Error("unused"); },
|
||||
putObjectConditional: async () => { throw Error("unused"); },
|
||||
headObject: async () => ({ exists: false }),
|
||||
deleteObject: async () => undefined,
|
||||
listObjects: async () => ({ keys: [] }),
|
||||
},
|
||||
keyPrefix: "logs",
|
||||
assertWriterAuthority: async () => undefined,
|
||||
});
|
||||
const handle = { store: "s3_segments" as const, logRef: "company-1/agent-1/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" };
|
||||
mockHeartbeatService.getRunLogAccess.mockResolvedValue({
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", companyId: "company-1", scopeKind: "company", issueId: null,
|
||||
logStore: handle.store, logRef: handle.logRef,
|
||||
});
|
||||
mockHeartbeatService.readLog.mockImplementation(async () => store.read(handle));
|
||||
|
||||
const res = await requestApp(await createApp(), (baseUrl) =>
|
||||
request(baseUrl).get("/api/heartbeat-runs/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/log"),
|
||||
);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(status);
|
||||
if (status === 404) expect(res.body).toEqual({ error: "Run log not found" });
|
||||
});
|
||||
|
||||
it("does not read an object run log for another company", async () => {
|
||||
const res = await requestApp(await createApp({}, {
|
||||
type: "board", userId: "board-user", companyIds: ["company-2"], source: "session", isInstanceAdmin: false,
|
||||
}), (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/log"));
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(mockHeartbeatService.readLog).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["skill_test", "task_bridge"])(
|
||||
"denies %s keys from company-wide run and workspace logs",
|
||||
async (kind) => {
|
||||
|
||||
@@ -321,7 +321,7 @@ describe("S3 run log store", () => {
|
||||
const m = memory(),
|
||||
store = createObjectRunLogStore({ provider: m.provider, keyPrefix: "logs", assertWriterAuthority: async () => undefined });
|
||||
await expect(store.read({ store: "s3_segments", logRef: "company/agent/never-written", attemptId: "attempt" }))
|
||||
.rejects.toThrow("Object run log not found");
|
||||
.rejects.toMatchObject({ status: 404, message: "Run log not found" });
|
||||
});
|
||||
it("batches concurrent appends, confirms durability, and resumes offsets/sequence on a replacement", async () => {
|
||||
const m = memory(),
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { StorageProvider } from "../storage/types.js";
|
||||
import type { RunLogStore, RunLogHandle, RunLogBeginInput } from "./run-log-store.js";
|
||||
import { createObjectByteJournal, ObjectJournalSourcePrefixProofUnavailableError, type ObjectJournalIOPriority, type ObjectJournalWriter } from "./object-byte-journal.js";
|
||||
import { MAX_RUN_LOG_RECORD_BYTES } from "./run-log-record-limits.js";
|
||||
import { notFound } from "../errors.js";
|
||||
|
||||
const MAX_SEGMENT_BYTES = 1024 * 1024;
|
||||
const MAX_PENDING_BYTES = MAX_RUN_LOG_RECORD_BYTES;
|
||||
@@ -357,7 +358,7 @@ export function createObjectRunLogStore(options: ObjectRunLogStoreOptions): RunL
|
||||
if (handle.store !== "s3_segments" && handle.store !== "s3_archive") throw new Error("Invalid object run log backend");
|
||||
const store = journal(handle.logRef, opts.priority ?? "interactive"),
|
||||
snapshot = await store.head(opts.signal) ?? writers.get(handle)?.writer;
|
||||
if (!snapshot || snapshot.head.deleted) throw new Error("Object run log not found");
|
||||
if (!snapshot || snapshot.head.deleted) throw notFound("Run log not found");
|
||||
const offset = Math.max(0, opts.offset ?? 0),
|
||||
limit = opts.limitBytes ?? 256_000;
|
||||
if (!Number.isSafeInteger(limit) || limit < 1 || limit > MAX_READ_BYTES)
|
||||
|
||||
Reference in new issue
Block a user