Merge PR #2688: fix(github): refresh stale issue and pr views
This commit is contained in:
@@ -28,7 +28,7 @@ import {
|
||||
parsePositiveDecimalInt,
|
||||
resolveDefaultRepoMemoized,
|
||||
} from "../tools/gh";
|
||||
import { formatFreshnessNote } from "../tools/github-cache";
|
||||
import { type CacheStatus, formatFreshnessNote } from "../tools/github-cache";
|
||||
import * as git from "../utils/git";
|
||||
import type { InternalResource, InternalUrl, ProtocolHandler, ResolveContext } from "./types";
|
||||
|
||||
@@ -355,7 +355,7 @@ interface BuildSingleArgs {
|
||||
scheme: Scheme;
|
||||
parsed: ParsedSingle;
|
||||
rendered: string;
|
||||
status: "miss" | "fresh" | "stale" | "disabled";
|
||||
status: CacheStatus;
|
||||
fetchedAt: number;
|
||||
/** Resolved repo (post short-form expansion) — used for the PR-only diff hint. */
|
||||
repo?: string;
|
||||
@@ -377,11 +377,15 @@ function buildSingleResource({
|
||||
const diffUrl = repoSegment ? `pr://${repoSegment}/${parsed.number}/diff` : `pr://${parsed.number}/diff`;
|
||||
notes.push(`Diff: ${diffUrl}`);
|
||||
}
|
||||
const content =
|
||||
status === "stale"
|
||||
? `> WARNING: Live GitHub refresh failed; this ${scheme} content is cached and may be stale.\n\n${rendered}`
|
||||
: rendered;
|
||||
return {
|
||||
url: url.href,
|
||||
content: rendered,
|
||||
content,
|
||||
contentType: "text/markdown",
|
||||
size: Buffer.byteLength(rendered, "utf-8"),
|
||||
size: Buffer.byteLength(content, "utf-8"),
|
||||
notes,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -8,10 +8,11 @@
|
||||
* helpers swallow open/IO failures and degrade to "no cache" so a corrupt or
|
||||
* unreadable DB never blocks a `gh` call.
|
||||
*
|
||||
* TTL:
|
||||
* Soft TTL → return cached row directly.
|
||||
* Past soft TTL but within hard TTL → return cached row AND schedule a
|
||||
* background refresh (errors logged, never thrown).
|
||||
* Stateful issue/PR rows past soft TTL but within hard TTL → refresh
|
||||
* synchronously, falling back to the cached row if the live fetch fails.
|
||||
* Expensive PR diff rows past soft TTL but within hard TTL → return cached
|
||||
* row AND schedule a background refresh (errors logged, never thrown).
|
||||
* Past hard TTL → treat as miss and fetch fresh.
|
||||
*/
|
||||
|
||||
@@ -21,6 +22,7 @@ import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { getGithubCacheDbPath, logger } from "@oh-my-pi/pi-utils";
|
||||
import type { Settings } from "../config/settings";
|
||||
import { ToolAbortError } from "./tool-errors";
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
// Storage layer
|
||||
@@ -449,7 +451,7 @@ export interface CacheLookupOptions<T> {
|
||||
now?: number;
|
||||
}
|
||||
|
||||
export type CacheStatus = "miss" | "fresh" | "stale" | "disabled";
|
||||
export type CacheStatus = "miss" | "fresh" | "refreshed" | "stale" | "disabled";
|
||||
|
||||
export interface CacheLookupResult<T> {
|
||||
rendered: string;
|
||||
@@ -595,7 +597,7 @@ export async function getOrFetchView<T>(options: CacheLookupOptions<T>): Promise
|
||||
status: "fresh",
|
||||
fetchedAt: cached.fetchedAt,
|
||||
};
|
||||
} else {
|
||||
} else if (options.kind === "pr-diff") {
|
||||
scheduleBackgroundRefresh(
|
||||
authKey,
|
||||
options.repo,
|
||||
@@ -611,6 +613,28 @@ export async function getOrFetchView<T>(options: CacheLookupOptions<T>): Promise
|
||||
status: "stale",
|
||||
fetchedAt: cached.fetchedAt,
|
||||
};
|
||||
} else {
|
||||
try {
|
||||
const fresh = await options.fetchFresh();
|
||||
const fetchedAt = Date.now();
|
||||
storeResult(authKey, options.repo, options.kind, options.number, options.includeComments, fresh, fetchedAt);
|
||||
return { ...fresh, status: "refreshed", fetchedAt };
|
||||
} catch (err) {
|
||||
if (err instanceof ToolAbortError) throw err;
|
||||
logger.debug("github cache: synchronous refresh failed; returning stale view", {
|
||||
err: String(err),
|
||||
repo: options.repo,
|
||||
kind: options.kind,
|
||||
number: options.number,
|
||||
});
|
||||
return {
|
||||
rendered: cached.rendered,
|
||||
sourceUrl: cached.sourceUrl,
|
||||
payload: cached.payload,
|
||||
status: "stale",
|
||||
fetchedAt: cached.fetchedAt,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -624,7 +648,7 @@ export async function getOrFetchView<T>(options: CacheLookupOptions<T>): Promise
|
||||
* Human-friendly freshness note for protocol-handler `notes[]` rendering.
|
||||
*/
|
||||
export function formatFreshnessNote(status: CacheStatus, fetchedAtMs: number, now: number = Date.now()): string {
|
||||
if (status === "miss") return "Fetched live";
|
||||
if (status === "miss" || status === "refreshed") return "Fetched live";
|
||||
if (status === "disabled") return "Cache disabled; fetched live";
|
||||
const ageSec = Math.max(0, Math.round((now - fetchedAtMs) / 1000));
|
||||
const human =
|
||||
@@ -633,6 +657,7 @@ export function formatFreshnessNote(status: CacheStatus, fetchedAtMs: number, no
|
||||
: ageSec < 3600
|
||||
? `${Math.round(ageSec / 60)}m ago`
|
||||
: `${Math.round(ageSec / 3600)}h ago`;
|
||||
if (status === "stale") return `Cached: ${human} (refreshing in background)`;
|
||||
if (status === "stale")
|
||||
return `WARNING: showing cached content from ${human}; live refresh failed or is still running`;
|
||||
return `Cached: ${human}`;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user