lucas/omp-deck

Watch

Fork 0

Improve forgejo_get_pr_diff for large PRs #9

New issue

Closed

opened 2026-10-15 10:04:14 +02:00 by omp-deck · 0 comments

omp-deck commented

2026-10-15 10:04:14 +02:00

Collaborator

Problem

When reviewing PR #8 (33 files, +2448 −43), a single forgejo_get_pr_diff call returned an incomplete diff:

The output stopped at the ~100 KB limit partway through apps/server/src/routes-forgejo-webhook.test.ts, so the route files and the whole apps/web part were never fetched.
The file list showed 30 of the 33 changed files. The other 3 couldn't be targeted with paths because their names were unknown.
The harness hid another ~62 KB of the fetched output, so the agent saw less than the tool fetched.

To get a complete diff, the agent would need several more calls with paths batches it had to guess.

Current state

The diff is returned inline and stops at about 100 KB, even partway through a file.
The file list stops at 30 entries, so some changed files never appear.
The harness hides part of large inline results, so the agent sees less than the tool fetched.
To get the rest, the agent has to guess paths batches.

Implementation plan

Context`through`## Assumptions & contingencies`. Writes it with` forgejo_update_issue_description`.

Captures an inbox item: deck_inbox_capture kind idea, source chat, title Snapshot pattern for Forgejo comment/review lists, body: "Follow-up to #9 item 6. Apply the snapshot pattern to forgejo_list_pr_comments_and_reviews and forgejo_list_issue_comments: fetch all pages, write the full timeline to local://, return counts plus the path inline. Comments have no natural SHA pin, so each call is a fresh snapshot."

Context

forgejo_get_pr_diff returns the diff inline. Output stops at about 100 KB (even partway through a file), and the file list stops at 30 entries, so large PRs can't be reviewed. Target end state:

The tool writes the complete diff to session storage (local://) as one numbered file per changed file, pinned to the head SHA and reused on repeat calls.
It returns a short summary that lists every changed file, and attaches a typed details payload for a web card.
An optional since_sha limits the snapshot to changes after that commit.
forgejo_read_file_at_ref gets the same snapshot treatment.
forgejo_read_pr stops listing a truncated file list and points to the diff tool instead.
Comment/review lists are out of scope here; they go to a follow-up.

Approach

1. Give Forgejo tools the session's `local://` root

apps/server/src/forgejo/tools/shared.ts:
- Add export interface ForgejoCallContext { localProtocolOptions?: LocalProtocolOptions }, with import type { LocalProtocolOptions } from "@oh-my-pi/pi-coding-agent/internal-urls". This path is the package export already used by bridge/plan-mode-bridge.ts L48.
- Change ForgejoToolSpec.run to run(ctx, params, signal?: AbortSignal, call?: ForgejoCallContext).
- In defineForgejoTool's execute(_toolCallId, params, _onUpdate, sdkCtx, signal), pass { localProtocolOptions: sdkCtx?.localProtocolOptions }. The SDK fills this per call (createCustomToolContext, node_modules/@oh-my-pi/pi-coding-agent/src/sdk.ts L1337-1346). Tools are built before the session exists (buildForgejoTools), so the context must come per call, not through ForgejoToolContext.
Move mapLimit from prs.ts (L66-78) into shared.ts as an export, unchanged. Its two callers in prs.ts (fetchInlineComments, searchPrs) import it from there.
New file apps/server/src/forgejo/tools/snapshot.ts. Nothing equivalent exists.
- export const SNAPSHOT_READ_BYTES = 64 * 1024 * 1024;
- export function localRoot(call?: ForgejoCallContext): { ok: true; root: string } | { ok: false; error: string }. Uses resolveLocalRoot(options) from @oh-my-pi/pi-coding-agent/internal-urls, which gives <artifactsDir>/local or a tmp fallback. Without localProtocolOptions it returns the error session storage (local://) is not available in this session.
- export function safeRelPath(p: string): string | null. Returns null for an absolute path or any /-separated segment equal to "", "." or "..". PR content can come from forks, so these checks must happen before any write.
- export async function writeSnapshotDir(root: string, name: string, files: { rel: string; content: string }[]): Promise<"written" | "exists">:
  1. mkdir(root, { recursive: true }), then tmp = mkdtemp(join(root, ".forgejo-tmp-")).
  2. Write each file under tmp, creating parent dirs. Assert each resolved path stays inside tmp.
  3. rename(tmp, join(root, name)). On EEXIST/ENOTEMPTY, remove tmp and return "exists".
  4. On any other error, remove tmp and rethrow.
- export async function writeFileAtomic(root: string, rel: string, content: string): Promise<void>. Writes to join(root, ".forgejo-tmp-<random>"), creates the target's parent dir, then renames the temp file over the target.
- export function formatSize(bytes: number): string. 1024-based: 512 B, 4.1 KB, 12.3 MB (one decimal for KB/MB).

2. Per-file diff analysis and numbering (pure)

In apps/server/src/forgejo/tools/diff.ts:

Delete filterDiff, normalizeFilter and FilteredDiff; getPrDiff was their only caller. Delete describe("filterDiff") in diff.test.ts.
parseDiffHeader: the quoted branch returns paths decoded by a new unquoteGitPath(s: string): string.
- Escapes: \\ \" \a \b \t \n \v \f \r, and \NNN octal turned into bytes.
- The bytes are decoded as UTF-8 with TextDecoder.
- Update diff.test.ts L17 to expect café.md.
Add export type DiffStatus = "A" | "M" | "D" | "R" | "C" and the analysis function:
```
export interface DiffChunkAnalysis {
  status: DiffStatus;
  additions: number;
  deletions: number;
  binary: boolean;
  numbered: string;
}
export function analyzeDiffChunk(file: DiffFile): DiffChunkAnalysis
```
- Status, from header lines before the first @@: new file mode → A; deleted file mode → D; rename from → R; copy from → C; otherwise M.
- Binary: some line starts with Binary files or equals GIT binary patch.
- Hunk parsing: ^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@ (missing counts default to 1). Track oldLeft/newLeft from the header, and leave the hunk when both reach 0. Counting, not prefix-sniffing, ends hunks, so content lines starting with ---/+++ are counted correctly.
- Lines inside a hunk:
  - ' ' or an empty line: context; both numbers advance.
  - '-': deletion; counts toward deletions and advances the old number.
  - '+': addition; counts toward additions and advances the new number.
  - A line starting with \ (e.g. \ No newline at end of file) gets a blank gutter and consumes nothing. This applies even right after a hunk ended.
- Numbered output:
  - Every hunk line becomes ${old.padStart(w)} ${new.padStart(w)} |${line}. A missing number is "", line keeps its marker character, and w is the digit count of the largest line number in the chunk (at least 1).
  - All other lines (diff --git, index, mode, rename, ---/+++, @@, Binary files …) stay verbatim and unnumbered.
  - Line structure and the trailing newline are preserved.

3. Snapshot manifest, index text and card details

In packages/protocol/src/forgejo-tools.ts, add:

export type ForgejoDiffFileStatus = "A" | "M" | "D" | "R" | "C";
export type ForgejoDiffFileFlag = "binary" | "truncated" | "missing" | "too-large";
export interface ForgejoDiffFile {
  path: string;
  oldPath: string | null;          // renames/copies only
  status: ForgejoDiffFileStatus;
  additions: number;
  deletions: number;
  bytes: number;                   // UTF-8 size of the stored .diff; 0 when none stored
  flag: ForgejoDiffFileFlag | null;
}
/** `forgejo_get_pr_diff`. `files` is null when totals.files > 30 (the card shows a note instead). */
export interface ForgejoPrDiffResult {
  number: number;
  headSha: string;                 // full SHA
  mergeBaseSha: string | null;
  sinceSha: string | null;         // full SHA, since mode only
  snapshot: string;                // e.g. "local://pr-8@fafd5d9" (no trailing slash)
  index: string;                   // e.g. "local://pr-8@fafd5d9.md"
  totals: { files: number; additions: number; deletions: number };
  truncated: boolean;
  reused: boolean;
  files: ForgejoDiffFile[] | null;
}
/** `forgejo_read_file_at_ref`. */
export interface ForgejoReadFileResult {
  path: string;
  ref: string | null;              // as passed; null = default branch
  sha: string;                     // full SHA the ref resolved to
  lines: number;
  bytes: number;
  truncated: boolean;
  snapshot: string;                // e.g. "local://tree@2c3a04a/CHANGELOG.md"
}

New file apps/server/src/forgejo/tools/pr-diff.ts, which holds the diff tool's logic.

Manifest type:

interface PrDiffManifest {
  number: number;
  headSha: string;
  mergeBaseSha: string | null;
  sinceSha: string | null;
  snapshot: string;
  index: string;
  truncated: boolean;
  files: ForgejoDiffFile[];
}

Constants: INLINE_SUMMARY_BYTES = 32 * 1024, CARD_FILE_LIMIT = 30.
Snapshot names:
- Full mode: pr-<n>@<head7>.
- Since mode: pr-<n>@<head7>-since-<since7>. Don't use ..: the SDK's validateRelativePath rejects .. segments and /...
Files written into the local root:
- Snapshot dir <name>/, one <repo path>.diff per stored file.
- Index <name>.md, as a sibling so it can never collide with a repo path.
- Manifest <name>.json, written last. Its presence marks the snapshot complete.
renderIndex(m: PrDiffManifest): string. The header block comes first, ended by a blank line:
- Line 1, full mode: PR #<n> @ <headSha> · merge base <mergeBase7 or ?> · <F> files +<A> −<D>.
- Line 1, since mode: PR #<n> changes since <sinceSha> → <headSha> · <F> files +<A> −<D>.
- Line 2: snapshot: <snapshot>/<path>.diff · index: <index>.
- Line 3: gutter: "<old> <new> |<diff line>"; comment with forgejo_post_review line=<new> (side new), or line=<old> with side "old" for removed lines.
- Line 4, only if truncated: TRUNCATED: the diff exceeded the 64 MB read limit. [missing] files have no snapshot; [truncated] files are cut off.
- Then one row per file, in diff order, with missing rows last:
  <status> +<add> −<del> <formatSize(bytes)> <oldPath → path | path> plus [<flag>] when the flag is set.
- With zero files the rows are just (no changes).
renderInline(index: string, m): string. If the index is at most INLINE_SUMMARY_BYTES in UTF-8, return it whole. Otherwise return the header block plus \n\n<F> files; full list in <index URL>.
toDetails(m, reused: boolean): ForgejoPrDiffResult. Computes totals, and sets files to null when there are more than CARD_FILE_LIMIT files.
API status → letter map, local to pr-diff.ts: added → A, deleted → D, renamed → R, copied → C, anything else → M. It is used only for missing rows.
async function storeSnapshot(root, name, m, stored: { rel: string; content: string }[]): Promise<{ manifest: PrDiffManifest; reused: boolean } | { error: string }>:
1. writeSnapshotDir.
2. If it returns "written": writeFileAtomic(root, name + ".md", renderIndex(m)), then writeFileAtomic(root, name + ".json", JSON.stringify(m, null, 2)).
3. If it returns "exists": read the manifest. If present, reuse it. If absent, return the error another call is writing snapshot <name>; call again.
async function loadManifest(root, name): Promise<PrDiffManifest | null>. Returns null when the file is missing or its JSON doesn't parse.
Reuse check, run before any diff fetch in both modes:
1. If a manifest loads, and its headSha and sinceSha equal the requested full SHAs, return textResult(renderInline(<read index file>, m), toDetails(m, true)).
2. If it loads but the SHAs differ, return the error snapshot <name> belongs to a different commit (<m.headSha>); short-SHA collision.
3. If the dir <name> exists without a manifest (an incomplete earlier write), rm -rf the dir and continue.

4. Full-mode `forgejo_get_pr_diff`

api-types.ts: add merge_base?: string | null to ApiPull, and add export interface ApiCommit { sha: string }.
prs.ts getPrDiff (L230-282):
- Rewrite. Parameters: { number: numberField("Pull request"), since_sha: z.string().regex(/^[0-9a-f]{7,40}$/i).optional().describe("Only changes after this commit (7-40 hex chars), e.g. the head you last reviewed") }. paths is removed.
- Description: Forgejo: snapshot the diff of a pull request in ${REPO_SCOPE} into session storage and return a summary listing every changed file. Each file's diff is stored at local://pr-<n>@<head7>/<path>.diff (read with `read`, search with `grep`) with old and new line numbers in a gutter: use the new number as `line` in forgejo_post_review, or the old number with side "old" for removed lines. Snapshots are pinned to the head commit and reused. Pass since_sha to get only what changed after that commit.
- run calls buildPrDiff(ctx, p.number, p.since_sha, signal, call), exported from pr-diff.ts.
Full-mode flow in pr-diff.ts:
1. localRoot(call); on error, errorResult.
2. GET pulls/{n}, which gives headSha = head.sha and mergeBaseSha = merge_base ?? null. Missing head SHA → error PR #<n> has no head commit. Then run the reuse check.
3. ctx.client.text(repoApiPath(owner, repo, "pulls", ${n}.diff), { maxBytes: SNAPSHOT_READ_BYTES, signal }).
4. GET pulls/{n} again. If the head changed, return the error PR #<n> head moved from <a7> to <b7> while the diff was being fetched; call again, and write nothing.
5. splitDiff. For each file:
  - safeRelPath(path) returns null → error refusing to store a diff for unsafe path "<path>".
  - Run analyzeDiffChunk. flag = "binary" if binary, else null. oldPath = oldPath when it differs from path, else null.
  - The stored file is { rel: path + ".diff", content: numbered }, and bytes = its UTF-8 length.
6. If res.truncated:
  - Set the manifest's truncated, and give the last diff file the flag "truncated".
  - fetchAllPages<ApiChangedFile>(ctx.client, pullPath(ctx, n, "files"), { maxPages: 100, signal }). On failure, return apiError and write nothing.
  - Every API file whose filename is not among the diff paths becomes a row: mapped status, additions/deletions from the API, bytes: 0, flag: "missing", oldPath: previous_filename if it differs. No file is stored for these rows.
7. Empty diff text: a manifest with zero files.
8. storeSnapshot, then return textResult(renderInline(index, m), toDetails(m, reused)).

5. Since mode (`since_sha`), API-only

The Forgejo API has no range-diff endpoint (verified on Forgejo 16.0.6). The changed files are found through compare, raw content is fetched at both SHAs, and git diff --no-index diffs them locally.

Get the local root, then GET pulls/{n} for the head SHA.
Resolve since_sha with GET repoApiPath(owner, repo, "commits"), query { sha: since, limit: 1, stat: false, verification: false, files: false }, which returns ApiCommit[].
- 404 or an empty array → error since_sha <since> not found in <owner>/<repo>.
- Use [0].sha as the full SHA.
Run the reuse check with name pr-<n>@<head7>-since-<since7>.
If the since SHA equals the head SHA, store a manifest with zero files.
Promise.all of GET compare/<since>...<head> and GET compare/<head>...<since>. A new type ApiCompare { total_commits?: number | null; commits?: unknown[] | null; files?: { filename: string; status?: string | null }[] | null } goes in api-types.ts.
- Either request fails → apiError.
- If total_commits is greater than commits.length → error compare <a7>...<b7> lists only <x> of <t> commits; the range is too large for since_sha.
- Path set = unique files[].filename from both responses, sorted alphabetically. Every path must pass safeRelPath. Querying both directions covers every path that differs, since each one is touched on one side of the merge base.
Create a work dir mkdtemp(join(os.tmpdir(), "omp-deck-since-")) containing a/ and b/. Delete it in finally (rm, recursive and force).
With mapLimit(paths, 4, …), fetch each path twice: ctx.client.text(repoApiPath(owner, repo, "raw", encodePath(path)), { query: { ref: <full sha> }, maxBytes: SNAPSHOT_READ_BYTES, signal }), once at the since SHA and once at the head SHA.
- 404 means the path is absent on that side.
- Any other failure → apiError (abort).
- If either side is truncated, write neither side and add a row: status A if absent at since, D if absent at head, else M; +0 −0; bytes: 0; flag: "too-large".
- Otherwise write a/<path> and/or b/<path>. Content is decoded as UTF-8 by client.text, which is lossy for non-UTF-8 files. Accepted.
export async function diffTrees(workDir: string, signal?: AbortSignal): Promise<{ ok: true; text: string; truncated: boolean } | { ok: false; error: string }>:
- Runs runGit(workDir, ["diff", "--no-index", "--no-prefix", "--no-color", "--no-ext-diff", "-M", "a", "b"], { readOnly: true, maxStdoutBytes: SNAPSHOT_READ_BYTES, timeoutMs: NETWORK_TIMEOUT_MS, signal }), all from apps/server/src/bridge/git-tools/run.ts. forgejo/repo.ts L13 already imports runGit from there.
- Exit 0: no differences. Exit 1: differences.
- Any other exit → error git diff failed: <stderr trimmed>.
- --no-prefix with dirs named a/b yields diff --git a/<path> b/<path> headers, the same shape as the PR diff.
Run the output through splitDiff and analyzeDiffChunk as in full mode. Identical files produce no chunk and so drop out. If stdout was truncated, set the manifest's truncated and give the last chunk the flag "truncated".
storeSnapshot and return, as in full mode.

6. `forgejo_read_pr` drops its partial file list

prs.ts readPr: remove the /files request and call formatPr(pr.data).
format.ts:
- formatPr(pr: ApiPull): delete the PrFormatExtras parameter and the files block (L120-124). Right after the changes: line, always push files: use forgejo_get_pr_diff for the full list and per-file diffs.
- Delete PrFormatExtras, formatChangedFile and FILE_STATUS.
prs.ts: delete FILE_LIST_LIMIT and FILTERABLE_READ_BYTES, plus the unused imports (formatChangedFile, filterDiff, truncationNotice if unused).
format.test.ts:
- In the "refs, short SHAs…" test, drop the files argument and the two file-row expectations (L133-134).
- Drop L143-144 and the whole formatChangedFile status letters test.

7. `forgejo_read_file_at_ref` as a snapshot

Parameters: { path, ref? }. Remove startLine/endLine.
Description: Forgejo: snapshot a file from ${REPO_SCOPE} at a branch, tag, or commit (not your working tree) into session storage at local://tree@<sha7>/<path> and return its size and line count; read it with `read`. `ref` defaults to the repo's default branch.
run:
1. Keep the existing path validation, then call localRoot(call).
2. Resolve the ref: GET commits with { sha: ref, limit: 1, stat: false, verification: false, files: false }; omit sha when there's no ref, which gives the default branch. 404 with a ref → error ref "<ref>" not found; other failures → apiError; empty array → error ref "<ref>" has no commits.
3. Fetch the raw file at the full SHA with maxBytes: SNAPSHOT_READ_BYTES. A NUL byte → keep the existing binary error and write nothing.
4. Always write with writeFileAtomic(root, "tree@<sha7>/<path>", text). Content at a SHA is immutable, so overwriting is safe, and the truncation state needs no storage.
5. lines = 0 for an empty file; otherwise the count of \n plus 1 if the text doesn't end with \n.
Inline text:
```
<path> @ <ref or "default branch"> (<sha7>) · <lines> lines · <formatSize(bytes)>
snapshot: local://tree@<sha7>/<path>
```
If truncated, append [truncated: the file exceeds the 64 MB read limit; the snapshot holds the first <formatSize(bytes)>].
Attach ForgejoReadFileResult as details.
Delete sliceNumberedLines from format.ts and its tests (format.test.ts L268 onward).

8. Web cards

apps/web/src/components/tools/git/ui.tsx: export the existing LETTER_TONE.
New apps/web/src/components/tools/forgejo/PrDiffView.tsx (ForgejoViewProps), following PostReviewView.tsx.
- It uses ForgejoShell, which renders the error text, the details, or linkified text for old results.
- Rows: ArgRow k="pull request" with PrRef, plus ArgRow k="since" with a ShaChip (n=7) when args.since_sha is a string.
- When isPrDiff(details) passes (a type guard in the file):
  - Header line: ShaChip for the head (n=7), since ShaChip if present, <N> files, Counts, Tag "reused" (muted) if reused, Tag "truncated" (tone warn) if truncated.
  - Snapshot line: PathChip with snapshot + "/", plus a CopyButton (@/lib/CopyButton) with text={snapshot}.
  - File list:
    - files === null: a Muted note <N> files: full list in <index> plus a CopyButton for the index.
    - 0 files: Muted "no changes".
    - 1-5 files: <details open> with summary files (N) (SUMMARY_CLASS).
    - 6-30 files: LazyDetails with summary files (N), collapsed.
  - Row:
    - Letter span: inline-block w-3 shrink-0 text-center font-mono text-2xs font-semibold + LETTER_TONE[status].
    - Path: truncated, with the full text as title; old → new for renames.
    - Counts, then formatBytes(bytes) from @/lib/utils in muted text.
    - Flag Tag: warn for truncated/missing/too-large, muted for binary.
New forgejo/ReadFileView.tsx:
- Rows: ArgRow k="file" with PathChip path plus @ ref (the current read-file rendering, L150-158).
- Details: ShaChip (n=7), <lines> lines, formatBytes(bytes), warn Tag "truncated" when truncated, and the snapshot PathChip with a CopyButton.
ForgejoTool.tsx:
- Route forgejo_get_pr_diff → <PrDiffView {...view} /> and forgejo_read_file_at_ref → <ReadFileView {...view} /> in the dispatch block next to the write tools, and delete their old cases (L129-140, L143-167).
- Summary function: forgejo_get_pr_diff returns [prRef(num), since ? "since " + since.slice(0, 7) : ""] joined with ·.
- Update the header comment (L1-11): these two read tools now also render details.
Preview gallery:
- New apps/web/src/preview-forgejo.ts exporting FORGEJO_FIXTURES: Record<string, { name: string; args: Record<string, unknown>; text?: string; details?: unknown; isError?: boolean }>, following preview-git.ts.
- Fixtures: diff with 3 files (list open); 12 files (collapsed); 35 files (files: null, note); truncated with one missing and one truncated row; since mode with a too-large row; reused; error; read-file; read-file truncated.
- Preview.tsx: build forgejoToolCalls like gitToolCalls (L205-217), with result: { content: [{ type: "text", text }], details }. Add <Section title="Forgejo tools"> after "Git tools".

9. Permanent tests

diff.test.ts, for analyzeDiffChunk:
- The exact numbered output of a two-hunk chunk with a context line, a deletion and an addition, and a \ No newline at end of file line with a blank gutter.
- A deleted line whose content starts with -- and an added line starting with ++ are counted, not treated as headers.
- Status A/D/R/C/M from the header lines.
- The binary flag.
- parseDiffHeader unquotes an octal UTF-8 path.
New snapshot.test.ts, using a temp root:
- safeRelPath rejects ../x, a//b, a/./b and /abs, and accepts a/b c.ts.
- writeSnapshotDir writes nested files and returns "exists" when the target exists, leaving no .forgejo-tmp-* entries behind.
- writeFileAtomic overwrites an existing file.
New pr-diff.test.ts:
- renderInline returns the whole index under 32 KB, and above it the header block plus <N> files; full list in local://…md.
- toDetails gives files: null for 31 files and an array for 30.
- diffTrees on temp a//b/ trees: modified, added and deleted files appear with A/D/M headers; an identical file is absent; an empty a/ dir works.

Critical files & anchors

apps/server/src/forgejo/tools/shared.ts defineForgejoTool (L69-95): where the SDK ctx.localProtocolOptions is dropped today.
apps/server/src/forgejo/tools/prs.ts getPrDiff (L230-282), readFileAtRef (L284-323), readPr (L204-228): the tools being rewritten.
apps/server/src/forgejo/tools/diff.ts splitDiff/parseDiffHeader: reused for per-file split.
node_modules/@oh-my-pi/pi-coding-agent/src/internal-urls/local-protocol.ts resolveLocalRoot (L240) and resolveLocalUrlToPath (L296): how local:// maps to disk. Treat it as read-only reference.
apps/web/src/components/tools/forgejo/PostReviewView.tsx: the pattern for a details card with a type guard and ForgejoShell.

Verification

Use only the repo, the test suite, and the deck's own Forgejo tools in a deck session. Do not open the deck DB, read tokens or deck config, or call Forgejo outside the forgejo_* tools.

Run bun test src/forgejo in apps/server. The new tests from step 9 pass, and the removed tests are gone.
Run bun run typecheck at the repo root.
Run bun run dev:web at the repo root, open http://127.0.0.1:5173/preview?preview=1, and check the "Forgejo tools" section:
- 3 files: list open. 12 files: collapsed. 35 files: note with a copy button, no list.
- Flags appear as tags; the reused and truncated tags show.
- The read-file cards show SHA, lines, size and the snapshot path.
  Use browser for a screenshot, then close the tab.
Live check. Ask the user to restart the deck server with the change and confirm. Do not restart it yourself. Then, in a new deck session in this repo:
1. forgejo_get_pr_diff {number: 8}. Expect a header starting PR #8 @ fafd5d9fca59d86c241997ddb0604a36efe9bd8f, 35 files, and every row present. The PR API reports +2749 −47; if the summed rows differ, find out why before finishing. The card shows the "files: full list in …" note.
2. read local://pr-8@fafd5d9.md, then read one .diff file from it.
3. Pick a + line with new number N. Call forgejo_read_file_at_ref {path: <that file>, ref: "fafd5d9fca59d86c241997ddb0604a36efe9bd8f"}, then read local://tree@fafd5d9/<path> at line N. The text matches.
4. Repeat step 1. The card shows "reused".
5. forgejo_get_pr_diff {number: 8, since_sha: "b6a37ee"}. Expect 15 files (the commits after b6a37ee touch 15 paths) and a collapsed list in the card.
6. forgejo_get_pr_diff {number: 8, since_sha: "zzz"}. The schema rejects it.
7. forgejo_read_file_at_ref {path: "CHANGELOG.md", ref: "no-such-branch"} returns error: ref "no-such-branch" not found.

Assumptions & contingencies

Trigger sessions: unattended Forgejo conversation sessions have no read tool (bridge/session-policy.ts). There they get only the summary. This is accepted; their tool set is handled separately.
Size limits: a 64 MB read cap per response. On overflow the tool writes what it read and flags it; there are no hard failures.
- Inline summary: up to 32 KB.
- Card file list: up to 30 files.
Since mode details:
- It is API-only, with renames detected by git -M.
- Files dropped by a force-push are still included, because compare is queried both ways.
- Raw content is decoded as UTF-8.
Contingencies:
- If git diff --no-index fails when a/ or b/ is empty, run diffTrees once per path instead: git diff --no-index --no-prefix --no-color --no-ext-diff a/<path> b/<path>, passing /dev/null for the absent side. Rewrite each chunk's diff --git, --- and +++ lines to a/<path>/b/<path> (/dev/null stays as is), concatenate the chunks, and re-run the diffTrees test.
- If compare responses have no total_commits, skip that guard.
- If renaming over a non-empty existing dir succeeds on some platform instead of failing, check for the target dir before the rename and treat an existing dir as "exists".

## Problem When reviewing PR #8 (33 files, +2448 −43), a single `forgejo_get_pr_diff` call returned an incomplete diff: - The output stopped at the ~100 KB limit partway through `apps/server/src/routes-forgejo-webhook.test.ts`, so the route files and the whole `apps/web` part were never fetched. - The file list showed 30 of the 33 changed files. The other 3 couldn't be targeted with `paths` because their names were unknown. - The harness hid another ~62 KB of the fetched output, so the agent saw less than the tool fetched. To get a complete diff, the agent would need several more calls with `paths` batches it had to guess. ## Current state - The diff is returned inline and stops at about 100 KB, even partway through a file. - The file list stops at 30 entries, so some changed files never appear. - The harness hides part of large inline results, so the agent sees less than the tool fetched. - To get the rest, the agent has to guess `paths` batches. ## Implementation plan ### Context` through `## Assumptions & contingencies`. Writes it with `forgejo_update_issue_description`. 2. Captures an inbox item: `deck_inbox_capture` kind `idea`, source `chat`, title `Snapshot pattern for Forgejo comment/review lists`, body: "Follow-up to #9 item 6. Apply the snapshot pattern to `forgejo_list_pr_comments_and_reviews` and `forgejo_list_issue_comments`: fetch all pages, write the full timeline to `local://`, return counts plus the path inline. Comments have no natural SHA pin, so each call is a fresh snapshot." ### Context `forgejo_get_pr_diff` returns the diff inline. Output stops at about 100 KB (even partway through a file), and the file list stops at 30 entries, so large PRs can't be reviewed. Target end state: - The tool writes the complete diff to session storage (`local://`) as one numbered file per changed file, pinned to the head SHA and reused on repeat calls. - It returns a short summary that lists every changed file, and attaches a typed `details` payload for a web card. - An optional `since_sha` limits the snapshot to changes after that commit. - `forgejo_read_file_at_ref` gets the same snapshot treatment. - `forgejo_read_pr` stops listing a truncated file list and points to the diff tool instead. - Comment/review lists are out of scope here; they go to a follow-up. ### Approach #### 1. Give Forgejo tools the session's `local://` root - `apps/server/src/forgejo/tools/shared.ts`: - Add `export interface ForgejoCallContext { localProtocolOptions?: LocalProtocolOptions }`, with `import type { LocalProtocolOptions } from "@oh-my-pi/pi-coding-agent/internal-urls"`. This path is the package export already used by `bridge/plan-mode-bridge.ts` L48. - Change `ForgejoToolSpec.run` to `run(ctx, params, signal?: AbortSignal, call?: ForgejoCallContext)`. - In `defineForgejoTool`'s `execute(_toolCallId, params, _onUpdate, sdkCtx, signal)`, pass `{ localProtocolOptions: sdkCtx?.localProtocolOptions }`. The SDK fills this per call (`createCustomToolContext`, `node_modules/@oh-my-pi/pi-coding-agent/src/sdk.ts` L1337-1346). Tools are built before the session exists (`buildForgejoTools`), so the context must come per call, not through `ForgejoToolContext`. - Move `mapLimit` from `prs.ts` (L66-78) into `shared.ts` as an export, unchanged. Its two callers in `prs.ts` (`fetchInlineComments`, `searchPrs`) import it from there. - New file `apps/server/src/forgejo/tools/snapshot.ts`. Nothing equivalent exists. - `export const SNAPSHOT_READ_BYTES = 64 * 1024 * 1024;` - `export function localRoot(call?: ForgejoCallContext): { ok: true; root: string } | { ok: false; error: string }`. Uses `resolveLocalRoot(options)` from `@oh-my-pi/pi-coding-agent/internal-urls`, which gives `<artifactsDir>/local` or a tmp fallback. Without `localProtocolOptions` it returns the error `session storage (local://) is not available in this session`. - `export function safeRelPath(p: string): string | null`. Returns `null` for an absolute path or any `/`-separated segment equal to `""`, `"."` or `".."`. PR content can come from forks, so these checks must happen before any write. - `export async function writeSnapshotDir(root: string, name: string, files: { rel: string; content: string }[]): Promise<"written" | "exists">`: 1. `mkdir(root, { recursive: true })`, then `tmp = mkdtemp(join(root, ".forgejo-tmp-"))`. 2. Write each file under `tmp`, creating parent dirs. Assert each resolved path stays inside `tmp`. 3. `rename(tmp, join(root, name))`. On `EEXIST`/`ENOTEMPTY`, remove `tmp` and return `"exists"`. 4. On any other error, remove `tmp` and rethrow. - `export async function writeFileAtomic(root: string, rel: string, content: string): Promise<void>`. Writes to `join(root, ".forgejo-tmp-<random>")`, creates the target's parent dir, then renames the temp file over the target. - `export function formatSize(bytes: number): string`. 1024-based: `512 B`, `4.1 KB`, `12.3 MB` (one decimal for KB/MB). #### 2. Per-file diff analysis and numbering (pure) In `apps/server/src/forgejo/tools/diff.ts`: - Delete `filterDiff`, `normalizeFilter` and `FilteredDiff`; `getPrDiff` was their only caller. Delete `describe("filterDiff")` in `diff.test.ts`. - `parseDiffHeader`: the quoted branch returns paths decoded by a new `unquoteGitPath(s: string): string`. - Escapes: `\\ \" \a \b \t \n \v \f \r`, and `\NNN` octal turned into bytes. - The bytes are decoded as UTF-8 with `TextDecoder`. - Update `diff.test.ts` L17 to expect `café.md`. - Add `export type DiffStatus = "A" | "M" | "D" | "R" | "C"` and the analysis function: ```ts export interface DiffChunkAnalysis { status: DiffStatus; additions: number; deletions: number; binary: boolean; numbered: string; } export function analyzeDiffChunk(file: DiffFile): DiffChunkAnalysis ``` - **Status**, from header lines before the first `@@`: `new file mode` → A; `deleted file mode` → D; `rename from ` → R; `copy from ` → C; otherwise M. - **Binary**: some line starts with `Binary files ` or equals `GIT binary patch`. - **Hunk parsing**: `^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@` (missing counts default to 1). Track `oldLeft`/`newLeft` from the header, and leave the hunk when both reach 0. Counting, not prefix-sniffing, ends hunks, so content lines starting with `---`/`+++` are counted correctly. - **Lines inside a hunk**: - `' '` or an empty line: context; both numbers advance. - `'-'`: deletion; counts toward `deletions` and advances the old number. - `'+'`: addition; counts toward `additions` and advances the new number. - A line starting with `\` (e.g. `\ No newline at end of file`) gets a blank gutter and consumes nothing. This applies even right after a hunk ended. - **Numbered output**: - Every hunk line becomes `${old.padStart(w)} ${new.padStart(w)} |${line}`. A missing number is `""`, `line` keeps its marker character, and `w` is the digit count of the largest line number in the chunk (at least 1). - All other lines (`diff --git`, `index`, mode, rename, `---`/`+++`, `@@`, `Binary files …`) stay verbatim and unnumbered. - Line structure and the trailing newline are preserved. #### 3. Snapshot manifest, index text and card details - In `packages/protocol/src/forgejo-tools.ts`, add: ```ts export type ForgejoDiffFileStatus = "A" | "M" | "D" | "R" | "C"; export type ForgejoDiffFileFlag = "binary" | "truncated" | "missing" | "too-large"; export interface ForgejoDiffFile { path: string; oldPath: string | null; // renames/copies only status: ForgejoDiffFileStatus; additions: number; deletions: number; bytes: number; // UTF-8 size of the stored .diff; 0 when none stored flag: ForgejoDiffFileFlag | null; } /** `forgejo_get_pr_diff`. `files` is null when totals.files > 30 (the card shows a note instead). */ export interface ForgejoPrDiffResult { number: number; headSha: string; // full SHA mergeBaseSha: string | null; sinceSha: string | null; // full SHA, since mode only snapshot: string; // e.g. "local://pr-8@fafd5d9" (no trailing slash) index: string; // e.g. "local://pr-8@fafd5d9.md" totals: { files: number; additions: number; deletions: number }; truncated: boolean; reused: boolean; files: ForgejoDiffFile[] | null; } /** `forgejo_read_file_at_ref`. */ export interface ForgejoReadFileResult { path: string; ref: string | null; // as passed; null = default branch sha: string; // full SHA the ref resolved to lines: number; bytes: number; truncated: boolean; snapshot: string; // e.g. "local://tree@2c3a04a/CHANGELOG.md" } ``` - New file `apps/server/src/forgejo/tools/pr-diff.ts`, which holds the diff tool's logic. - Manifest type: ```ts interface PrDiffManifest { number: number; headSha: string; mergeBaseSha: string | null; sinceSha: string | null; snapshot: string; index: string; truncated: boolean; files: ForgejoDiffFile[]; } ``` - Constants: `INLINE_SUMMARY_BYTES = 32 * 1024`, `CARD_FILE_LIMIT = 30`. - Snapshot names: - Full mode: `pr-<n>@<head7>`. - Since mode: `pr-<n>@<head7>-since-<since7>`. Don't use `..`: the SDK's `validateRelativePath` rejects `..` segments and `/..`. - Files written into the local root: - Snapshot dir `<name>/`, one `<repo path>.diff` per stored file. - Index `<name>.md`, as a sibling so it can never collide with a repo path. - Manifest `<name>.json`, written last. Its presence marks the snapshot complete. - `renderIndex(m: PrDiffManifest): string`. The header block comes first, ended by a blank line: - Line 1, full mode: `PR #<n> @ <headSha> · merge base <mergeBase7 or ?> · <F> files +<A> −<D>`. - Line 1, since mode: `PR #<n> changes since <sinceSha> → <headSha> · <F> files +<A> −<D>`. - Line 2: `snapshot: <snapshot>/<path>.diff · index: <index>`. - Line 3: `gutter: "<old> <new> |<diff line>"; comment with forgejo_post_review line=<new> (side new), or line=<old> with side "old" for removed lines`. - Line 4, only if truncated: `TRUNCATED: the diff exceeded the 64 MB read limit. [missing] files have no snapshot; [truncated] files are cut off.` - Then one row per file, in diff order, with `missing` rows last: `<status> +<add> −<del> <formatSize(bytes)> <oldPath → path | path>` plus ` [<flag>]` when the flag is set. - With zero files the rows are just `(no changes)`. - `renderInline(index: string, m): string`. If the index is at most `INLINE_SUMMARY_BYTES` in UTF-8, return it whole. Otherwise return the header block plus `\n\n<F> files; full list in <index URL>`. - `toDetails(m, reused: boolean): ForgejoPrDiffResult`. Computes `totals`, and sets `files` to `null` when there are more than `CARD_FILE_LIMIT` files. - API status → letter map, local to `pr-diff.ts`: `added → A`, `deleted → D`, `renamed → R`, `copied → C`, anything else → M. It is used only for `missing` rows. - `async function storeSnapshot(root, name, m, stored: { rel: string; content: string }[]): Promise<{ manifest: PrDiffManifest; reused: boolean } | { error: string }>`: 1. `writeSnapshotDir`. 2. If it returns `"written"`: `writeFileAtomic(root, name + ".md", renderIndex(m))`, then `writeFileAtomic(root, name + ".json", JSON.stringify(m, null, 2))`. 3. If it returns `"exists"`: read the manifest. If present, reuse it. If absent, return the error `another call is writing snapshot <name>; call again`. - `async function loadManifest(root, name): Promise<PrDiffManifest | null>`. Returns `null` when the file is missing or its JSON doesn't parse. - Reuse check, run before any diff fetch in both modes: 1. If a manifest loads, and its `headSha` and `sinceSha` equal the requested full SHAs, return `textResult(renderInline(<read index file>, m), toDetails(m, true))`. 2. If it loads but the SHAs differ, return the error `snapshot <name> belongs to a different commit (<m.headSha>); short-SHA collision`. 3. If the dir `<name>` exists without a manifest (an incomplete earlier write), `rm -rf` the dir and continue. #### 4. Full-mode `forgejo_get_pr_diff` - `api-types.ts`: add `merge_base?: string | null` to `ApiPull`, and add `export interface ApiCommit { sha: string }`. - `prs.ts` `getPrDiff` (L230-282): - Rewrite. Parameters: `{ number: numberField("Pull request"), since_sha: z.string().regex(/^[0-9a-f]{7,40}$/i).optional().describe("Only changes after this commit (7-40 hex chars), e.g. the head you last reviewed") }`. `paths` is removed. - Description: ``Forgejo: snapshot the diff of a pull request in ${REPO_SCOPE} into session storage and return a summary listing every changed file. Each file's diff is stored at local://pr-<n>@<head7>/<path>.diff (read with `read`, search with `grep`) with old and new line numbers in a gutter: use the new number as `line` in forgejo_post_review, or the old number with side "old" for removed lines. Snapshots are pinned to the head commit and reused. Pass since_sha to get only what changed after that commit.`` - `run` calls `buildPrDiff(ctx, p.number, p.since_sha, signal, call)`, exported from `pr-diff.ts`. - Full-mode flow in `pr-diff.ts`: 1. `localRoot(call)`; on error, `errorResult`. 2. `GET pulls/{n}`, which gives `headSha = head.sha` and `mergeBaseSha = merge_base ?? null`. Missing head SHA → error `PR #<n> has no head commit`. Then run the reuse check. 3. `ctx.client.text(repoApiPath(owner, repo, "pulls", `${n}.diff`), { maxBytes: SNAPSHOT_READ_BYTES, signal })`. 4. `GET pulls/{n}` again. If the head changed, return the error `PR #<n> head moved from <a7> to <b7> while the diff was being fetched; call again`, and write nothing. 5. `splitDiff`. For each file: - `safeRelPath(path)` returns null → error `refusing to store a diff for unsafe path "<path>"`. - Run `analyzeDiffChunk`. `flag` = `"binary"` if binary, else null. `oldPath` = `oldPath` when it differs from `path`, else null. - The stored file is `{ rel: path + ".diff", content: numbered }`, and `bytes` = its UTF-8 length. 6. If `res.truncated`: - Set the manifest's `truncated`, and give the last diff file the flag `"truncated"`. - `fetchAllPages<ApiChangedFile>(ctx.client, pullPath(ctx, n, "files"), { maxPages: 100, signal })`. On failure, return `apiError` and write nothing. - Every API file whose `filename` is not among the diff paths becomes a row: mapped status, `additions`/`deletions` from the API, `bytes: 0`, `flag: "missing"`, `oldPath: previous_filename` if it differs. No file is stored for these rows. 7. Empty diff text: a manifest with zero files. 8. `storeSnapshot`, then return `textResult(renderInline(index, m), toDetails(m, reused))`. #### 5. Since mode (`since_sha`), API-only The Forgejo API has no range-diff endpoint (verified on Forgejo 16.0.6). The changed files are found through compare, raw content is fetched at both SHAs, and `git diff --no-index` diffs them locally. 1. Get the local root, then `GET pulls/{n}` for the head SHA. 2. Resolve `since_sha` with `GET repoApiPath(owner, repo, "commits")`, query `{ sha: since, limit: 1, stat: false, verification: false, files: false }`, which returns `ApiCommit[]`. - 404 or an empty array → error `since_sha <since> not found in <owner>/<repo>`. - Use `[0].sha` as the full SHA. 3. Run the reuse check with name `pr-<n>@<head7>-since-<since7>`. 4. If the since SHA equals the head SHA, store a manifest with zero files. 5. `Promise.all` of `GET compare/<since>...<head>` and `GET compare/<head>...<since>`. A new type `ApiCompare { total_commits?: number | null; commits?: unknown[] | null; files?: { filename: string; status?: string | null }[] | null }` goes in `api-types.ts`. - Either request fails → `apiError`. - If `total_commits` is greater than `commits.length` → error `compare <a7>...<b7> lists only <x> of <t> commits; the range is too large for since_sha`. - Path set = unique `files[].filename` from both responses, sorted alphabetically. Every path must pass `safeRelPath`. Querying both directions covers every path that differs, since each one is touched on one side of the merge base. 6. Create a work dir `mkdtemp(join(os.tmpdir(), "omp-deck-since-"))` containing `a/` and `b/`. Delete it in `finally` (`rm`, recursive and force). 7. With `mapLimit(paths, 4, …)`, fetch each path twice: `ctx.client.text(repoApiPath(owner, repo, "raw", encodePath(path)), { query: { ref: <full sha> }, maxBytes: SNAPSHOT_READ_BYTES, signal })`, once at the since SHA and once at the head SHA. - 404 means the path is absent on that side. - Any other failure → `apiError` (abort). - If either side is truncated, write neither side and add a row: status A if absent at since, D if absent at head, else M; `+0 −0`; `bytes: 0`; `flag: "too-large"`. - Otherwise write `a/<path>` and/or `b/<path>`. Content is decoded as UTF-8 by `client.text`, which is lossy for non-UTF-8 files. Accepted. 8. `export async function diffTrees(workDir: string, signal?: AbortSignal): Promise<{ ok: true; text: string; truncated: boolean } | { ok: false; error: string }>`: - Runs `runGit(workDir, ["diff", "--no-index", "--no-prefix", "--no-color", "--no-ext-diff", "-M", "a", "b"], { readOnly: true, maxStdoutBytes: SNAPSHOT_READ_BYTES, timeoutMs: NETWORK_TIMEOUT_MS, signal })`, all from `apps/server/src/bridge/git-tools/run.ts`. `forgejo/repo.ts` L13 already imports `runGit` from there. - Exit 0: no differences. Exit 1: differences. - Any other exit → error `git diff failed: <stderr trimmed>`. - `--no-prefix` with dirs named `a`/`b` yields `diff --git a/<path> b/<path>` headers, the same shape as the PR diff. 9. Run the output through `splitDiff` and `analyzeDiffChunk` as in full mode. Identical files produce no chunk and so drop out. If stdout was truncated, set the manifest's `truncated` and give the last chunk the flag `"truncated"`. 10. `storeSnapshot` and return, as in full mode. #### 6. `forgejo_read_pr` drops its partial file list - `prs.ts` `readPr`: remove the `/files` request and call `formatPr(pr.data)`. - `format.ts`: - `formatPr(pr: ApiPull)`: delete the `PrFormatExtras` parameter and the files block (L120-124). Right after the `changes:` line, always push `files: use forgejo_get_pr_diff for the full list and per-file diffs`. - Delete `PrFormatExtras`, `formatChangedFile` and `FILE_STATUS`. - `prs.ts`: delete `FILE_LIST_LIMIT` and `FILTERABLE_READ_BYTES`, plus the unused imports (`formatChangedFile`, `filterDiff`, `truncationNotice` if unused). - `format.test.ts`: - In the "refs, short SHAs…" test, drop the `files` argument and the two file-row expectations (L133-134). - Drop L143-144 and the whole `formatChangedFile status letters` test. #### 7. `forgejo_read_file_at_ref` as a snapshot - Parameters: `{ path, ref? }`. Remove `startLine`/`endLine`. - Description: ``Forgejo: snapshot a file from ${REPO_SCOPE} at a branch, tag, or commit (not your working tree) into session storage at local://tree@<sha7>/<path> and return its size and line count; read it with `read`. `ref` defaults to the repo's default branch.`` - `run`: 1. Keep the existing path validation, then call `localRoot(call)`. 2. Resolve the ref: `GET commits` with `{ sha: ref, limit: 1, stat: false, verification: false, files: false }`; omit `sha` when there's no ref, which gives the default branch. 404 with a ref → error `ref "<ref>" not found`; other failures → `apiError`; empty array → error `ref "<ref>" has no commits`. 3. Fetch the raw file at the full SHA with `maxBytes: SNAPSHOT_READ_BYTES`. A NUL byte → keep the existing binary error and write nothing. 4. Always write with `writeFileAtomic(root, "tree@<sha7>/<path>", text)`. Content at a SHA is immutable, so overwriting is safe, and the truncation state needs no storage. 5. `lines` = 0 for an empty file; otherwise the count of `\n` plus 1 if the text doesn't end with `\n`. - Inline text: ``` <path> @ <ref or "default branch"> (<sha7>) · <lines> lines · <formatSize(bytes)> snapshot: local://tree@<sha7>/<path> ``` If truncated, append `[truncated: the file exceeds the 64 MB read limit; the snapshot holds the first <formatSize(bytes)>]`. - Attach `ForgejoReadFileResult` as `details`. - Delete `sliceNumberedLines` from `format.ts` and its tests (`format.test.ts` L268 onward). #### 8. Web cards - `apps/web/src/components/tools/git/ui.tsx`: export the existing `LETTER_TONE`. - New `apps/web/src/components/tools/forgejo/PrDiffView.tsx` (`ForgejoViewProps`), following `PostReviewView.tsx`. - It uses `ForgejoShell`, which renders the error text, the details, or linkified text for old results. - Rows: `ArgRow k="pull request"` with `PrRef`, plus `ArgRow k="since"` with a `ShaChip` (n=7) when `args.since_sha` is a string. - When `isPrDiff(details)` passes (a type guard in the file): - **Header line:** `ShaChip` for the head (n=7), `since` `ShaChip` if present, `<N> files`, `Counts`, `Tag` "reused" (muted) if reused, `Tag` "truncated" (tone warn) if truncated. - **Snapshot line:** `PathChip` with `snapshot + "/"`, plus a `CopyButton` (`@/lib/CopyButton`) with `text={snapshot}`. - **File list:** - `files === null`: a `Muted` note `<N> files: full list in <index>` plus a `CopyButton` for the index. - 0 files: `Muted` "no changes". - 1-5 files: `<details open>` with summary `files (N)` (`SUMMARY_CLASS`). - 6-30 files: `LazyDetails` with summary `files (N)`, collapsed. - **Row:** - Letter span: `inline-block w-3 shrink-0 text-center font-mono text-2xs font-semibold` + `LETTER_TONE[status]`. - Path: truncated, with the full text as `title`; `old → new` for renames. - `Counts`, then `formatBytes(bytes)` from `@/lib/utils` in muted text. - Flag `Tag`: warn for `truncated`/`missing`/`too-large`, muted for `binary`. - New `forgejo/ReadFileView.tsx`: - Rows: `ArgRow k="file"` with `PathChip` path plus `@ ref` (the current read-file rendering, L150-158). - Details: `ShaChip` (n=7), `<lines> lines`, `formatBytes(bytes)`, warn `Tag` "truncated" when truncated, and the snapshot `PathChip` with a `CopyButton`. - `ForgejoTool.tsx`: - Route `forgejo_get_pr_diff` → `<PrDiffView {...view} />` and `forgejo_read_file_at_ref` → `<ReadFileView {...view} />` in the dispatch block next to the write tools, and delete their old cases (L129-140, L143-167). - Summary function: `forgejo_get_pr_diff` returns `[prRef(num), since ? "since " + since.slice(0, 7) : ""]` joined with ` · `. - Update the header comment (L1-11): these two read tools now also render `details`. - Preview gallery: - New `apps/web/src/preview-forgejo.ts` exporting `FORGEJO_FIXTURES: Record<string, { name: string; args: Record<string, unknown>; text?: string; details?: unknown; isError?: boolean }>`, following `preview-git.ts`. - Fixtures: diff with 3 files (list open); 12 files (collapsed); 35 files (`files: null`, note); truncated with one `missing` and one `truncated` row; since mode with a `too-large` row; reused; error; read-file; read-file truncated. - `Preview.tsx`: build `forgejoToolCalls` like `gitToolCalls` (L205-217), with `result: { content: [{ type: "text", text }], details }`. Add `<Section title="Forgejo tools">` after "Git tools". #### 9. Permanent tests - `diff.test.ts`, for `analyzeDiffChunk`: - The exact numbered output of a two-hunk chunk with a context line, a deletion and an addition, and a `\ No newline at end of file` line with a blank gutter. - A deleted line whose content starts with `--` and an added line starting with `++` are counted, not treated as headers. - Status A/D/R/C/M from the header lines. - The binary flag. - `parseDiffHeader` unquotes an octal UTF-8 path. - New `snapshot.test.ts`, using a temp root: - `safeRelPath` rejects `../x`, `a//b`, `a/./b` and `/abs`, and accepts `a/b c.ts`. - `writeSnapshotDir` writes nested files and returns `"exists"` when the target exists, leaving no `.forgejo-tmp-*` entries behind. - `writeFileAtomic` overwrites an existing file. - New `pr-diff.test.ts`: - `renderInline` returns the whole index under 32 KB, and above it the header block plus `<N> files; full list in local://…md`. - `toDetails` gives `files: null` for 31 files and an array for 30. - `diffTrees` on temp `a/`/`b/` trees: modified, added and deleted files appear with A/D/M headers; an identical file is absent; an empty `a/` dir works. ### Critical files & anchors - `apps/server/src/forgejo/tools/shared.ts` `defineForgejoTool` (L69-95): where the SDK `ctx.localProtocolOptions` is dropped today. - `apps/server/src/forgejo/tools/prs.ts` `getPrDiff` (L230-282), `readFileAtRef` (L284-323), `readPr` (L204-228): the tools being rewritten. - `apps/server/src/forgejo/tools/diff.ts` `splitDiff`/`parseDiffHeader`: reused for per-file split. - `node_modules/@oh-my-pi/pi-coding-agent/src/internal-urls/local-protocol.ts` `resolveLocalRoot` (L240) and `resolveLocalUrlToPath` (L296): how `local://` maps to disk. Treat it as read-only reference. - `apps/web/src/components/tools/forgejo/PostReviewView.tsx`: the pattern for a details card with a type guard and `ForgejoShell`. ### Verification Use only the repo, the test suite, and the deck's own Forgejo tools in a deck session. Do not open the deck DB, read tokens or deck config, or call Forgejo outside the `forgejo_*` tools. 1. Run `bun test src/forgejo` in `apps/server`. The new tests from step 9 pass, and the removed tests are gone. 2. Run `bun run typecheck` at the repo root. 3. Run `bun run dev:web` at the repo root, open `http://127.0.0.1:5173/preview?preview=1`, and check the "Forgejo tools" section: - 3 files: list open. 12 files: collapsed. 35 files: note with a copy button, no list. - Flags appear as tags; the reused and truncated tags show. - The read-file cards show SHA, lines, size and the snapshot path. Use `browser` for a screenshot, then close the tab. 4. **Live check.** Ask the user to restart the deck server with the change and confirm. Do not restart it yourself. Then, in a new deck session in this repo: 1. `forgejo_get_pr_diff {number: 8}`. Expect a header starting `PR #8 @ fafd5d9fca59d86c241997ddb0604a36efe9bd8f`, 35 files, and every row present. The PR API reports +2749 −47; if the summed rows differ, find out why before finishing. The card shows the "files: full list in …" note. 2. `read local://pr-8@fafd5d9.md`, then `read` one `.diff` file from it. 3. Pick a `+` line with new number N. Call `forgejo_read_file_at_ref {path: <that file>, ref: "fafd5d9fca59d86c241997ddb0604a36efe9bd8f"}`, then `read local://tree@fafd5d9/<path>` at line N. The text matches. 4. Repeat step 1. The card shows "reused". 5. `forgejo_get_pr_diff {number: 8, since_sha: "b6a37ee"}`. Expect 15 files (the commits after b6a37ee touch 15 paths) and a collapsed list in the card. 6. `forgejo_get_pr_diff {number: 8, since_sha: "zzz"}`. The schema rejects it. 7. `forgejo_read_file_at_ref {path: "CHANGELOG.md", ref: "no-such-branch"}` returns `error: ref "no-such-branch" not found`. ### Assumptions & contingencies - **Trigger sessions:** unattended Forgejo conversation sessions have no `read` tool (`bridge/session-policy.ts`). There they get only the summary. This is accepted; their tool set is handled separately. - **Size limits:** a 64 MB read cap per response. On overflow the tool writes what it read and flags it; there are no hard failures. - Inline summary: up to 32 KB. - Card file list: up to 30 files. - **Since mode details:** - It is API-only, with renames detected by `git -M`. - Files dropped by a force-push are still included, because compare is queried both ways. - Raw content is decoded as UTF-8. - **Contingencies:** - If `git diff --no-index` fails when `a/` or `b/` is empty, run `diffTrees` once per path instead: `git diff --no-index --no-prefix --no-color --no-ext-diff a/<path> b/<path>`, passing `/dev/null` for the absent side. Rewrite each chunk's `diff --git`, `---` and `+++` lines to `a/<path>`/`b/<path>` (`/dev/null` stays as is), concatenate the chunks, and re-run the `diffTrees` test. - If compare responses have no `total_commits`, skip that guard. - If renaming over a non-empty existing dir succeeds on some platform instead of failing, check for the target dir before the rename and treat an existing dir as `"exists"`.

omp-deck referenced this issue from a commit

2026-10-15 17:28:06 +02:00

feat(forgejo): snapshot PR diffs and files into session storage

omp-deck referenced this issue from a pull request that will close it,

2026-10-15 17:28:19 +02:00

feat(forgejo): snapshot PR diffs and files into session storage #14