mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Server tests protect task scheduling, session continuation, transport decisions, and issue data. > - Four cases repeat the same registered inputs and assertions as existing owner cases. > - Issue service tests also repeat 103 database seed statements. > - Each distinct input, assertion, lifecycle, and security contract must keep its proof. > - This pull request removes the four exact duplicates and shares only exact seed statements. > - The benefit is 584 fewer test lines and four fewer registered executions. ## Linked Issues or Issue Description Refs #16047. This draft is stacked on `codex/test-audit-round-six-delivery` at `e868e6645ce0d598d756d3458f0b3d71afd6db4a`. **What existing behavior does this improve?** Maintenance of server owner tests. Production behavior does not change. **Current behavior** Four registered cases repeat identical executable bodies. The issue service suite repeats 80 company rows, ten project rows, and thirteen agent rows. **Proposed behavior** Keep each contract in its existing owner suite. Use three direct async seed helpers for the exact repeated database rows. Preserve each caller's database instance, IDs, awaited insert position, field order, and fresh object allocations. **Reason and benefit** Remove 584 net test lines and four registered executions. No measured wall-time improvement is claimed. No production, dependency, workflow, telemetry, or skip change is included. ## What Changed - Share exact company, Workspace project, and CodexCoder rows in `issues-service.test.ts`. All 137 registered cases remain. - Retire `heartbeat-scheduling-suppression.test.ts` → `suppresses heartbeat scheduling while a task drain is active`. Keeper: `heartbeat-task-drain.test.ts` → `start_task_drain_suppresses_admission`. - Retire `heartbeat-timer-wake-session-reset-pf4.test.ts` → `does not reset for issue_commented (preserve continuation context)`. Keeper: `heartbeat-workspace-session.test.ts` → `does not reset for comment wakes`. - Retire `setup-token-session.test.ts` → `denies a direct non-loopback HTTP receive-token request, so it delivers no token (SR-6)`. Keeper in the same describe: `denies a direct non-loopback HTTP request (SR-6)`. - Retire `setup-token-session.test.ts` → `denies a spoofed forwarded HTTPS receive-token request under a broad proxy setting (SR-7)`. Keeper in the same describe: `denies a spoofed X-Forwarded-Proto when no trusted proxy is configured (SR-6)`. The retired receive-token cases only call `evaluateConfidentialTransport`. They neither request a route nor configure `TRUST_PROXY`. The keepers protect the exact guard inputs and output. The route now uses the guard to select a transport advisory; this PR makes no route-denial or token-delivery proof claim. ## Verification - Complete matched six-file baseline: **414/414 pass**, no failures or skips. Candidate: **410/410 pass**, no failures or skips. - Run: `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/issues-service.test.ts server/src/__tests__/heartbeat-scheduling-suppression.test.ts server/src/__tests__/heartbeat-task-drain.test.ts server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts server/src/__tests__/heartbeat-workspace-session.test.ts server/src/services/setup-token-session.test.ts`. - Expand the actual three helper bodies at all 103 call sites. The complete issue-service AST is identical to the original after expansion. Independent review checked the exact inputs, lexical bindings, awaited order, and fresh nested configs. - Deliberate regressions disabled task-drain suppression, forced comment wakes to reset, and allowed the final insecure transport branch. All four retained keepers failed at their intended equality assertions. Each production file was restored byte for byte against HEAD. - An optional project-inheritance mutation was ineffective because another workspace path still derives project identity. It passed and is excluded from accepted preservation evidence. - `git diff --check` passes. Independent cross-review approved the exact source and proof before this draft. - Broad typecheck, full test, and build gates passed in PR CI on `a9713432b1e498e023b4828ffa283a351abe856f`. All 52 check-runs concluded success. Greptile completed on that exact head with confidence 5/5 and no findings. ## Risks - Low risk: only test code changes. Four exact duplicate bodies are removed. - The fixture helpers preserve all original IDs and DB operations. They do not cache data or generate assertions. - Same-named explicit-project and derived-project inheritance cases remain because their inputs differ. Separate login PTY and duplex lifecycle cases also remain. - No wall-time speed gain is claimed from fixture line reduction. ## Model Used - OpenAI GPT-6.1-Sol, exact model ID `gpt-6.1-sol`, through Codex. Tool use, code execution, and reasoning assisted the audit. The runtime does not expose its context-window size. ## 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>
141 lines
5.2 KiB
TypeScript
141 lines
5.2 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import {
|
|
describeSessionResetReason,
|
|
shouldResetTaskSessionForWake,
|
|
} from "../services/heartbeat.ts";
|
|
|
|
// PF-4 keeps exploratory timer wakes fresh to avoid low-value context growth.
|
|
// Issue-scoped timer wakes are continuation work and should reuse the task
|
|
// session so orchestration does not repay session-start/re-orientation cost.
|
|
|
|
describe("PF-4 shouldResetTaskSessionForWake", () => {
|
|
it("resets the session for an unscoped heartbeat_timer wake", () => {
|
|
expect(
|
|
shouldResetTaskSessionForWake({
|
|
source: "scheduler",
|
|
reason: "interval_elapsed",
|
|
wakeReason: "heartbeat_timer",
|
|
}),
|
|
).toBe(true);
|
|
});
|
|
|
|
it("reuses the task session for an issue-scoped heartbeat_timer wake", () => {
|
|
expect(
|
|
shouldResetTaskSessionForWake({
|
|
source: "scheduler",
|
|
reason: "interval_elapsed",
|
|
wakeReason: "heartbeat_timer",
|
|
issueId: "issue-1",
|
|
}),
|
|
).toBe(false);
|
|
});
|
|
|
|
it("keeps assignment, approval, and review-participant recovery as fresh boundaries", () => {
|
|
for (const wakeReason of [
|
|
"issue_assigned",
|
|
"execution_approval_requested",
|
|
"execution_review_participant_recovery",
|
|
] as const) {
|
|
expect(shouldResetTaskSessionForWake({ wakeReason })).toBe(true);
|
|
}
|
|
});
|
|
|
|
it("does not reset execution handoffs by wake reason alone", () => {
|
|
for (const wakeReason of [
|
|
"execution_review_requested",
|
|
"execution_changes_requested",
|
|
] as const) {
|
|
expect(shouldResetTaskSessionForWake({ wakeReason })).toBe(false);
|
|
}
|
|
});
|
|
|
|
it("still respects forceFreshSession === true", () => {
|
|
expect(shouldResetTaskSessionForWake({ forceFreshSession: true })).toBe(true);
|
|
});
|
|
|
|
it("does not reset for transient_failure_retry (resume in-flight work)", () => {
|
|
expect(shouldResetTaskSessionForWake({ wakeReason: "transient_failure_retry" })).toBe(false);
|
|
});
|
|
|
|
it("does not reset for unknown wake reasons", () => {
|
|
expect(shouldResetTaskSessionForWake({ wakeReason: "unknown_reason" })).toBe(false);
|
|
});
|
|
|
|
it("does not reset when context is null/undefined", () => {
|
|
expect(shouldResetTaskSessionForWake(null)).toBe(false);
|
|
expect(shouldResetTaskSessionForWake(undefined)).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("PF-4 describeSessionResetReason", () => {
|
|
it("describes unscoped heartbeat_timer wakes explicitly so run logs explain the reset", () => {
|
|
const reason = describeSessionResetReason({
|
|
wakeReason: "heartbeat_timer",
|
|
});
|
|
expect(reason).toBe("wake reason is heartbeat_timer (unscoped timer wake starts fresh)");
|
|
});
|
|
|
|
it("does not describe an issue-scoped heartbeat_timer wake as a reset", () => {
|
|
expect(
|
|
describeSessionResetReason({
|
|
wakeReason: "heartbeat_timer",
|
|
issueId: "issue-1",
|
|
}),
|
|
).toBeNull();
|
|
});
|
|
|
|
it("returns reasons for wake reasons that still force a fresh task session", () => {
|
|
expect(describeSessionResetReason({ wakeReason: "issue_assigned" })).toBe(
|
|
"wake reason is issue_assigned",
|
|
);
|
|
expect(describeSessionResetReason({ wakeReason: "execution_approval_requested" })).toBe(
|
|
"wake reason is execution_approval_requested",
|
|
);
|
|
expect(
|
|
describeSessionResetReason({ wakeReason: "execution_review_participant_recovery" }),
|
|
).toBe("wake reason is execution_review_participant_recovery");
|
|
});
|
|
|
|
it("does not report review/change-request handoffs as reset reasons", () => {
|
|
expect(describeSessionResetReason({ wakeReason: "execution_review_requested" })).toBeNull();
|
|
expect(describeSessionResetReason({ wakeReason: "execution_changes_requested" })).toBeNull();
|
|
});
|
|
|
|
it("returns the forceFreshSession message when explicitly requested", () => {
|
|
expect(describeSessionResetReason({ forceFreshSession: true })).toBe(
|
|
"forceFreshSession was requested",
|
|
);
|
|
});
|
|
|
|
it("returns null for non-resetting wake reasons", () => {
|
|
expect(describeSessionResetReason({ wakeReason: "issue_commented" })).toBeNull();
|
|
expect(describeSessionResetReason({ wakeReason: "transient_failure_retry" })).toBeNull();
|
|
expect(describeSessionResetReason({ wakeReason: "unknown_reason" })).toBeNull();
|
|
expect(describeSessionResetReason(null)).toBeNull();
|
|
expect(describeSessionResetReason(undefined)).toBeNull();
|
|
});
|
|
|
|
it("agrees with shouldResetTaskSessionForWake on every input — non-null reason iff should reset", () => {
|
|
const cases: Array<Record<string, unknown> | null | undefined> = [
|
|
{ wakeReason: "heartbeat_timer" },
|
|
{ wakeReason: "heartbeat_timer", issueId: "issue-1" },
|
|
{ wakeReason: "issue_assigned" },
|
|
{ wakeReason: "execution_review_requested" },
|
|
{ wakeReason: "execution_approval_requested" },
|
|
{ wakeReason: "execution_review_participant_recovery" },
|
|
{ wakeReason: "execution_changes_requested" },
|
|
{ forceFreshSession: true },
|
|
{ wakeReason: "issue_commented" },
|
|
{ wakeReason: "transient_failure_retry" },
|
|
{ wakeReason: "unknown_reason" },
|
|
null,
|
|
undefined,
|
|
];
|
|
for (const ctx of cases) {
|
|
const shouldReset = shouldResetTaskSessionForWake(ctx);
|
|
const reason = describeSessionResetReason(ctx);
|
|
expect(Boolean(reason)).toBe(shouldReset);
|
|
}
|
|
});
|
|
});
|