perf: narrow run-to-issue authorization reads (#17405)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Run pages show links to issues associated with a run.
> - The issue-link route checks the run's company and issue visibility
before it reads those links.
> - That check needs four binding fields, but the route reads the full
run.
> - Full runs can include large prompt, output, and result fields.
> - This pull request uses the existing narrow authorization read.
> - The route transfers less database data and keeps its permission
checks and response contract.

## Linked Issues or Issue Description

**Problem or motivation**

`GET /api/heartbeat-runs/:runId/issues` calls `heartbeat.getRun` for a
permission check. Its consumers only need the run ID, company ID, scope
kind, and issue ID. The full read also fetches context, output excerpts,
result data, and runtime metadata. Repeated link reads pay for that
unused data.

**Proposed solution**

Use `getRunReadAccess`, the existing four-field read from #17348. Keep
authentication, company access, company-scope authorization, run issue
visibility, and linked-issue filtering in their current owners. Keep the
existing empty response for missing and inaccessible runs.

**Alternatives considered**

A new query contract is unnecessary because the existing read provides
all required fields. A permission cache could become stale. Changing the
full run reader could affect consumers that need run details.

**Roadmap alignment**

This is a focused improvement to an existing route. It adds no feature,
route, provider, cache, or background work. Searches for run-issue
links, narrow run authorization, and `getRunReadAccess` found no open
duplicate. Related work: #17348.

## What Changed

- Change the issue-link route to use the existing narrow run read.
- Reject full run reads in the route tests. Cover readable company and
issue scopes, denied issue access, missing bindings, anonymous requests,
and the missing/cross-company empty response.
- Add a developer note about the read contract.

## Verification

- `pnpm --filter @paperclipai/server exec vitest run
src/__tests__/activity-routes.test.ts
src/services/heartbeat/run-state.test.ts
src/__tests__/agent-live-run-routes.test.ts`: 126 tests passed.
- `pnpm build`: passed.
- `pnpm -r typecheck`: passed.
- The route tests use the real run visibility helper with only the four
binding fields. The full reader throws if a route calls it. The existing
run-state suite checks the exact four-column selection and covers
database wiring with disposable Postgres.
- `git diff --check`: passed. Reviewed the committed diff for secrets
and private identifiers before push.
- Apex review: 5/5 on exact head
`888a4ce9ea88bb2b284eb5e28fafc01b6f48b386`, completed successfully, with
zero review threads.
- Full current-head CI passed: 54 successful checks and two expected
skips. [Workflow attempt
2](https://github.com/paperclipai/paperclip/actions/runs/40051399577/attempts/2).
[Initial serialized shard
failure](https://github.com/paperclipai/paperclip/actions/runs/40051399577/job/120744710081):
the first company-history case in unchanged
`agent-live-run-routes.test.ts` hit its 15-second timeout; the shard had
391 passing tests and one failure. That file, its agent route, the
heartbeat state reader, and the service export are unchanged from base.
The five company-history cases pass on unchanged base
`9fb28d06ffebbb615f413765078aeeb7c4291311` (9.12s suite) and this head
(8.29s suite). They also passed in the 126-test focused run. The timeout
was not reproduced locally; its cause is not established. Only the
failed shard was retried on the same head. It passed in attempt 2. This
establishes recovery; the initial timeout remains in this record and its
cause is unproven.
- The prior local full-suite failures and baseline triage remain
documented in #17384. No new local full-suite pass is claimed here. This
change uses focused local tests and the full current-head CI gate.
- No hosted user journey, staging deployment, production deployment, or
billed savings is claimed for this follow-up.

## Risks

- Run visibility depends on the canonical `scopeKind` and `issueId`. The
existing helper returns those same fields. It does not use prompt or
result data for access decisions.
- Linked issues still pass the existing SQL visibility filter. Missing
and cross-company runs still return `200 []`. Anonymous requests still
fail before the run read.
- The database query count stays the same. The change reduces data
selected for this authorization read.
- No schema or API response changes.

## Model Used

OpenAI Codex, GPT-6 family. The exact deployment model ID and context
window are not exposed in this session. Used reasoning, repository
tools, and code execution.

## 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
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin FoleyandPaperclip authored and GitHub committed 2026-10-23 19:59:21 -07:00
1 parent 70de7d49e9
commit 1a79522a4e
3 files changed
+52 -5

No files matched your search

+4
View File
@@ -2885,6 +2885,10 @@ Sandbox-provider driver lookup filters plugin manifests by driver key and kind
in Postgres before returning rows. Install order, status and worker checks still
select the provider. Each lookup reads current registry state without a cache.
Run-to-issue links authorize through `getRunReadAccess`, which reads only the run
ID, company ID, scope kind, and issue ID. The existing visibility policy then
filters linked issues without loading the run's prompt, output, or result.
Run the deterministic native event replay with the normal server test runner:
```sh
+47 -4
View File
@@ -12,6 +12,7 @@ const mockActivityService = vi.hoisted(() => ({
const mockHeartbeatService = vi.hoisted(() => ({
getRun: vi.fn(),
getRunReadAccess: vi.fn(),
}));
const mockIssueService = vi.hoisted(() => ({
@@ -67,6 +68,7 @@ async function createApp(
source: "session",
isInstanceAdmin: false,
},
database: unknown = {},
) {
vi.resetModules();
const [{ errorHandler }, { activityRoutes }] = await Promise.all([
@@ -82,7 +84,7 @@ async function createApp(
};
next();
});
app.use("/api", activityRoutes({} as any));
app.use("/api", activityRoutes(database as any));
app.use(errorHandler);
return app;
}
@@ -118,6 +120,9 @@ describe("activity routes", () => {
beforeEach(() => {
for (const mock of Object.values(mockActivityService)) mock.mockReset();
for (const mock of Object.values(mockHeartbeatService)) mock.mockReset();
mockHeartbeatService.getRun.mockImplementation(() => {
throw new Error("Run issue links must use the narrow authorization read");
});
for (const mock of Object.values(mockIssueService)) mock.mockReset();
mockAccessService.decide.mockReset();
mockAccessService.canUser.mockReset();
@@ -344,13 +349,15 @@ describe("activity routes", () => {
});
it("passes task read filtering when listing links for a readable company run", async () => {
mockHeartbeatService.getRun.mockResolvedValue({
mockHeartbeatService.getRunReadAccess.mockResolvedValue({
id: "run-1", companyId: "company-1", scopeKind: "company", issueId: null,
});
mockActivityService.issuesForRun.mockResolvedValue([{ issueId: "public-issue", title: "Visible task" }]);
const app = await createApp({ type: "agent", companyId: "company-1", agentId: "agent-1" });
const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/heartbeat-runs/run-1/issues"));
expect(res.status).toBe(200);
expect(mockHeartbeatService.getRunReadAccess).toHaveBeenCalledWith("run-1");
expect(mockHeartbeatService.getRun).not.toHaveBeenCalled();
const { issueReadSqlCondition } = await import("../services/authorization.js");
expect(issueReadSqlCondition).toHaveBeenCalledWith({}, expect.objectContaining({
type: "agent", companyId: "company-1", agentId: "agent-1",
@@ -360,7 +367,7 @@ describe("activity routes", () => {
});
it("returns 200 [] (not 404) when listing issues for another company's run, preserving API contract and the cross-tenant oracle", async () => {
mockHeartbeatService.getRun.mockResolvedValue({
mockHeartbeatService.getRunReadAccess.mockResolvedValue({
id: "run-2",
companyId: "company-2",
});
@@ -374,7 +381,7 @@ describe("activity routes", () => {
});
it("returns 200 [] (not 404) for a non-existent heartbeat run, matching the cross-tenant response", async () => {
mockHeartbeatService.getRun.mockResolvedValue(null);
mockHeartbeatService.getRunReadAccess.mockResolvedValue(null);
const app = await createApp();
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/missing-run/issues"));
@@ -390,6 +397,42 @@ describe("activity routes", () => {
expect(res.status).toBe(401);
expect(mockHeartbeatService.getRun).not.toHaveBeenCalled();
expect(mockHeartbeatService.getRunReadAccess).not.toHaveBeenCalled();
expect(mockActivityService.issuesForRun).not.toHaveBeenCalled();
});
it.each([true, false])("preserves issue-scoped run authorization when read permission is %s", async (allowed) => {
mockHeartbeatService.getRunReadAccess.mockResolvedValue({
id: "run-1", companyId: "company-1", scopeKind: "issue", issueId: "issue-private",
});
mockAccessService.decide.mockImplementation(async ({ action }) => ({
allowed: action === "issue:read" ? allowed : true,
}));
mockActivityService.issuesForRun.mockResolvedValue([{ issueId: "visible-issue" }]);
const db = {
select: () => ({ from: () => ({ where: async () => [{ id: "issue-private" }] }) }),
};
const app = await createApp({ type: "agent", companyId: "company-1", agentId: "agent-1" }, db);
const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/heartbeat-runs/run-1/issues"));
expect(res.status).toBe(200);
expect(res.body).toEqual(allowed ? [{ issueId: "visible-issue" }] : []);
expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({
action: "issue:read", resource: expect.objectContaining({ companyId: "company-1", issueId: "issue-private" }),
}));
expect(mockHeartbeatService.getRunReadAccess).toHaveBeenCalledWith("run-1");
expect(mockHeartbeatService.getRun).not.toHaveBeenCalled();
expect(mockActivityService.issuesForRun).toHaveBeenCalledTimes(allowed ? 1 : 0);
});
it("does not read issue links for a run without a trusted issue binding", async () => {
mockHeartbeatService.getRunReadAccess.mockResolvedValue({
id: "run-1", companyId: "company-1", scopeKind: "issue", issueId: null,
});
const app = await createApp();
const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/heartbeat-runs/run-1/issues"));
expect(res.status).toBe(200);
expect(res.body).toEqual([]);
expect(mockActivityService.issuesForRun).not.toHaveBeenCalled();
expect(mockHeartbeatService.getRun).not.toHaveBeenCalled();
});
});
+1 -1
View File
@@ -372,7 +372,7 @@ export function activityRoutes(db: Db) {
router.get("/heartbeat-runs/:runId/issues", async (req, res) => {
assertAuthenticated(req);
const runId = req.params.runId as string;
const run = await heartbeat.getRun(runId);
const run = await heartbeat.getRunReadAccess(runId);
if (!run || !hasCompanyAccess(req, run.companyId)) {
// Return `200 []` for both "doesn't exist" and "cross-tenant" — preserves the
// legacy API contract while keeping the cross-tenant existence oracle closed