mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Skills can come from a tracked GitHub source, and an update moves
the installed copy to a newer upstream revision
> - Before an update, a person asks the agent "what changed in the new
version?"
> - The product has no diff for a pending update. The agent previews two
full SKILL.md snapshots and compares them by eye
> - In two eval runs the agent reversed the same changed line. In a
third run it fetched the files with curl and ran `diff` itself, and the
turn took too long
> - This pull request adds a read-only endpoint that returns the line
diff between the installed and the latest tracked revision
> - The benefit is that review answers come from an exact diff, and they
come faster
## Linked Issues or Issue Description
No public issue exists. Feature description:
**What happened**
An agent asked to show what changed in a pending skill update had only
`POST /skill-sources/preview`, which returns one file at one commit. It
previewed both commits and compared about 9 KB of text by eye. It
reported one changed line in the wrong direction ("every new build" ->
"every build or release" instead of the reverse), and later retracted a
real change.
**Expected behavior**
The agent can request the exact changes of a pending tracked-source
update and report them with the correct direction.
**Steps to reproduce**
1. Install a skill from a GitHub source at an old commit and set the
tracking reference to a newer commit (automatic updates off).
2. Ask the chat agent: "Show me what changed in the new accessibility
skill version before we update it."
3. See that the agent can only preview full files at each commit and
must compare them itself.
## What Changed
- Add `GET /api/companies/{companyId}/skills/{skillId}/update-diff`. For
a skill from a tracked GitHub source, it compares the installed source
revision (`currentRef`) with the latest tracked revision (`latestRef`).
For each changed package file it returns `change` (added, removed,
modified, or `not_compared`), `binary`, `executableChanged`, and a
unified line diff (`-` installed, `+` latest), or `diffUnavailable`
(`binary`, `too_large`, `omitted`, `budget`) that says why there is no
diff. The endpoint is read-only. The `skillId` parameter handler checks
company access and skill visibility, the same checks other skill reads
use.
- `hasUpdate` is false and `files` is empty when the installed revision
is already the latest one. That case reads only the latest commit and
scans nothing.
- Return `supported: false` with a reason for: a skill without a tracked
source, a disconnected source, an unknown installed revision, and a
repository package that a scan could include only in part (a subset, or
skill folders only). In the last case, files that a scan limit left out
would otherwise look removed.
- A path the scan left out of a revision (too large, a symlink, or
rejected by the audit) is listed as `not_compared` instead of added or
removed.
- One response shares a work budget (24M LCS cells), a diff-text budget
(1M characters) and a file-count budget (1,000 files), and sets
`truncated` when it reaches one. The GitHub read and both scans stop
when the caller disconnects.
- Add `server/src/services/skill-update-diff.ts` with a bounded line
diff (shared head and tail trimmed, LCS size capped). An absent side has
no lines, a final newline is not a line, and a missing final newline
shows the unified-diff marker. A replaced line reads `-old` then `+new`.
- Add the shared types `SkillSourceUpdateDiff` and
`SkillSourceUpdateDiffFile`.
- Document the endpoint in the API catalog (`openapi.ts`) so agents find
it with `search_api`.
## Verification
- `cd server && npx vitest run src/__tests__/skill-update-diff.test.ts`
(10 tests):
- direction of `-`/`+` lines, and separate hunks with bounded context
- exact diffs for added and removed files, and final-newline handling
- a change to only the executable bit, and paths left out of a scan
- the response budgets
- the snapshot comparison for the selected skill path, with the abort
signal passed to both scans
- a commit mismatch
- refusal of partial repository scans
- `cd server && npx vitest run
src/__tests__/skill-update-diff-routes.test.ts` (6 tests):
- the diff response
- no pending update (no scan)
- a caller from another company (board user and agent) gets 403 before
any lookup
- a skill the caller cannot read gets 404 before any source read
- every unsupported case
- a client disconnect aborts the scans
- `company-skills-routes.test.ts` and `openapi-routes.test.ts` pass (119
tests across the four files).
- `npx tsc --noEmit -p server`: no errors from this change.
- End to end: the `skills.review-update` chat eval exercises this path.
Its grader checks the quoted hunks against the real pinned diff.
## Risks
- Low risk. The endpoint is new and read-only. Existing routes do not
change.
- When an update is pending, each call scans the source twice (installed
and latest commit). It uses the same GitHub reader, scan limits and scan
slots as preview and refresh. The diff work and response size are capped
per response.
- `/update-status` and this route both read the latest commit of the
tracking ref. They are kept separate because `/update-status` compares
with the source's last scan, while this route compares with the
installed revision.
- No UI uses the endpoint yet.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), 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>
123 lines
9.0 KiB
TypeScript
123 lines
9.0 KiB
TypeScript
import { describe, expect, it, vi } from "vitest";
|
|
|
|
type ScanFile = { path: string; content: string; encoding?: "utf8" | "base64"; executable?: boolean };
|
|
type Scan = { commitSha: string; packageMode?: "skills" | "repository"; repositorySubset?: boolean; repositoryFiles?: ScanFile[];
|
|
repositoryPackage?: { warnings: string[] }; warnings?: string[]; skills: Array<{ path: string; files: ScanFile[]; warnings?: string[] }> };
|
|
const scans = vi.hoisted(() => new Map<string, Scan>());
|
|
const scanSignals = vi.hoisted(() => [] as Array<AbortSignal | undefined>);
|
|
vi.mock("../services/github-skill-source.js", () => ({
|
|
scanGitHubSkillsWithPackageFallback: vi.fn(async (input: { commitSha: string }, _read: unknown, options: { signal?: AbortSignal } = {}) => {
|
|
scanSignals.push(options.signal);
|
|
return { warnings: [], ...scans.get(input.commitSha) };
|
|
}),
|
|
}));
|
|
import { diffGitHubSkillUpdate, diffSkillPackageFiles, MAX_UPDATE_DIFF_CHARS, MAX_UPDATE_DIFF_FILES, unifiedLineDiff } from "../services/skill-update-diff.js";
|
|
|
|
const installed = ["---", "version: \"0.4\"", "---", "", "Run checks so every build or release runs them.", "", "## Notes", "Keep it short."].join("\n");
|
|
const latest = ["---", "version: \"0.5\"", "---", "", "Run checks so every new build runs them.", "", "## Design", "Breathable layouts.", "", "## Notes", "Keep it short."].join("\n");
|
|
|
|
describe("skill update diff", () => {
|
|
it("marks installed-only lines with - and latest-only lines with +", () => {
|
|
const diff = unifiedLineDiff(installed, latest)!;
|
|
const lines = diff.split("\n");
|
|
expect(lines).toContain("-version: \"0.4\"");
|
|
expect(lines).toContain("+version: \"0.5\"");
|
|
expect(lines).toContain("-Run checks so every build or release runs them.");
|
|
expect(lines).toContain("+Run checks so every new build runs them.");
|
|
expect(lines).toContain("+## Design");
|
|
expect(lines).toContain("+Breathable layouts.");
|
|
expect(lines).not.toContain("-## Notes");
|
|
expect(lines.filter(line => line.startsWith("@@"))).toEqual(["@@ -1,8 +1,11 @@"]);
|
|
expect(unifiedLineDiff(installed, installed)).toBe("");
|
|
});
|
|
|
|
it("keeps distant changes in separate hunks with bounded context", () => {
|
|
const before = Array.from({ length: 30 }, (_, i) => `line ${i + 1}`);
|
|
const after = [...before]; after[1] = "changed 2"; after[27] = "changed 28";
|
|
const diff = unifiedLineDiff(before.join("\n"), after.join("\n"))!;
|
|
expect(diff.split("\n").filter(line => line.startsWith("@@"))).toEqual(["@@ -1,5 +1,5 @@", "@@ -25,6 +25,6 @@"]);
|
|
expect(diff).not.toContain(" line 15");
|
|
});
|
|
|
|
it("lists added, removed, modified and binary files and skips unchanged ones", () => {
|
|
const { files, truncated } = diffSkillPackageFiles(
|
|
[{ path: "SKILL.md", content: installed }, { path: "old.md", content: "gone\n" }, { path: "same.md", content: "same" }, { path: "logo.png", content: "AAA=", encoding: "base64" }],
|
|
[{ path: "SKILL.md", content: latest }, { path: "new.md", content: "fresh\n" }, { path: "same.md", content: "same" }, { path: "logo.png", content: "AAE=", encoding: "base64" }],
|
|
);
|
|
expect(truncated).toBe(false);
|
|
expect(files.map(file => [file.path, file.change, file.binary, file.diffUnavailable])).toEqual([
|
|
["SKILL.md", "modified", false, null], ["logo.png", "modified", true, "binary"], ["new.md", "added", false, null], ["old.md", "removed", false, null],
|
|
]);
|
|
// An absent side has no lines, so an added or removed file shows only its own lines.
|
|
expect(files.find(file => file.path === "new.md")!.diff).toBe("@@ -0,0 +1,1 @@\n+fresh");
|
|
expect(files.find(file => file.path === "old.md")!.diff).toBe("@@ -1,1 +0,0 @@\n-gone");
|
|
expect(files.find(file => file.path === "logo.png")!.diff).toBeNull();
|
|
});
|
|
|
|
it("does not count a final newline as a line and shows when only the final newline changes", () => {
|
|
expect(unifiedLineDiff("a\nb\n", "a\nc\n")).toBe("@@ -1,2 +1,2 @@\n a\n-b\n+c");
|
|
expect(unifiedLineDiff("a\n", "a")).toBe("@@ -1,1 +1,1 @@\n-a\n+a\n\\ No newline at end of file");
|
|
});
|
|
|
|
it("reports a change to only the executable bit", () => {
|
|
const { files } = diffSkillPackageFiles([{ path: "run.sh", content: "echo hi\n" }], [{ path: "run.sh", content: "echo hi\n", executable: true }]);
|
|
expect(files).toEqual([{ path: "run.sh", change: "modified", binary: false, executableChanged: true, diff: "", diffUnavailable: null }]);
|
|
});
|
|
|
|
it("does not call a file the scan left out of one revision added or removed", () => {
|
|
const { files } = diffSkillPackageFiles([{ path: "SKILL.md", content: "a" }], [{ path: "SKILL.md", content: "a" }, { path: "assets/logo.svg", content: "<svg/>" }],
|
|
{ installed: new Set(["assets/logo.svg"]), latest: new Set() });
|
|
expect(files).toEqual([{ path: "assets/logo.svg", change: "not_compared", binary: false, executableChanged: false, diff: null, diffUnavailable: "omitted" }]);
|
|
});
|
|
|
|
it("bounds the diff text and the file count of one response", () => {
|
|
const big = Array.from({ length: 400 }, (_, i) => ({ path: `f${String(i).padStart(3, "0")}.md`, content: `${"x".repeat(5_000)}\n` }));
|
|
const { files, truncated } = diffSkillPackageFiles(big.map(file => ({ ...file, content: "" })), big);
|
|
expect(truncated).toBe(true);
|
|
expect(files.reduce((sum, file) => sum + (file.diff?.length ?? 0), 0)).toBeLessThanOrEqual(MAX_UPDATE_DIFF_CHARS);
|
|
expect(files.at(-1)!.diffUnavailable).toBe("budget");
|
|
const many = Array.from({ length: MAX_UPDATE_DIFF_FILES + 5 }, (_, i) => ({ path: `m${i}.md`, content: "x" }));
|
|
const listed = diffSkillPackageFiles([], many);
|
|
expect([listed.files.length, listed.truncated]).toEqual([MAX_UPDATE_DIFF_FILES, true]);
|
|
});
|
|
|
|
it("compares the installed commit snapshot with the latest one for the selected skill", async () => {
|
|
scans.set("a".repeat(40), { commitSha: "a".repeat(40), skills: [{ path: "accessibility/SKILL.md", files: [{ path: "SKILL.md", content: installed }] }] });
|
|
scans.set("b".repeat(40), { commitSha: "b".repeat(40), skills: [{ path: "accessibility/SKILL.md", files: [{ path: "SKILL.md", content: latest }] }] });
|
|
const controller = new AbortController();
|
|
scanSignals.length = 0;
|
|
const result = await diffGitHubSkillUpdate({ repositoryUrl: "https://github.com/example/skills", skillPath: "accessibility/SKILL.md", installedCommitSha: "a".repeat(40), latestCommitSha: "b".repeat(40) }, {} as never, controller.signal);
|
|
if (!result.supported) throw new Error(result.reason);
|
|
expect(result.files).toHaveLength(1);
|
|
expect(result.files[0]!.diff).toContain("-Run checks so every build or release runs them.");
|
|
expect(scanSignals).toEqual([controller.signal, controller.signal]);
|
|
scans.set("c".repeat(40), { commitSha: "d".repeat(40), skills: [] });
|
|
await expect(diffGitHubSkillUpdate({ repositoryUrl: "https://github.com/example/skills", skillPath: "accessibility/SKILL.md", installedCommitSha: "a".repeat(40), latestCommitSha: "c".repeat(40) }, {} as never)).rejects.toThrow("did not match");
|
|
});
|
|
|
|
it("marks files the scan left out of a skill folder as not compared", async () => {
|
|
scans.set("e".repeat(40), { commitSha: "e".repeat(40), skills: [{ path: "design/SKILL.md", files: [{ path: "SKILL.md", content: "a" }],
|
|
warnings: ["design/assets/hero.png — Exceeds the 1 MB file limit; not included."] }] });
|
|
scans.set("f".repeat(40), { commitSha: "f".repeat(40), skills: [{ path: "design/SKILL.md", files: [{ path: "SKILL.md", content: "a" }, { path: "assets/hero.png", content: "AAA=", encoding: "base64" }] }] });
|
|
const result = await diffGitHubSkillUpdate({ repositoryUrl: "https://github.com/example/skills", skillPath: "design/SKILL.md", installedCommitSha: "e".repeat(40), latestCommitSha: "f".repeat(40) }, {} as never);
|
|
expect(result).toEqual({ supported: true, truncated: false, files: [{ path: "assets/hero.png", change: "not_compared", binary: false, executableChanged: false, diff: null, diffUnavailable: "omitted" }] });
|
|
});
|
|
|
|
it("refuses to compare a repository package that a scan could only include in part", async () => {
|
|
const full = { commitSha: "1".repeat(40), packageMode: "repository" as const, repositoryFiles: [{ path: "README.md", content: "a" }], skills: [] };
|
|
const input = { repositoryUrl: "https://github.com/example/skills", packageMode: "repository" as const, skillPath: "x/SKILL.md", installedCommitSha: "1".repeat(40), latestCommitSha: "2".repeat(40) };
|
|
scans.set("1".repeat(40), full);
|
|
for (const partial of [
|
|
{ commitSha: "2".repeat(40), packageMode: "repository" as const, repositorySubset: true, repositoryFiles: [], skills: [] },
|
|
{ commitSha: "2".repeat(40), packageMode: "skills" as const, skills: [] },
|
|
]) {
|
|
scans.set("2".repeat(40), partial);
|
|
const result = await diffGitHubSkillUpdate(input, {} as never);
|
|
expect(result.supported).toBe(false);
|
|
}
|
|
scans.set("2".repeat(40), { ...full, commitSha: "2".repeat(40), repositoryFiles: [{ path: "README.md", content: "b" }] });
|
|
const result = await diffGitHubSkillUpdate(input, {} as never);
|
|
expect(result.supported && result.files.map(file => file.path)).toEqual(["README.md"]);
|
|
});
|
|
});
|