fix(core): dedup session.prs on parse + dedup writes + cleanup migration for indexed enrichment keys (#2109)
* fix(core): deduplicate session PR metadata * test(core): wait for branch adoption completion * test(integration): skip Linear suite on auth failure * test(integration): make desktop notifier platform mock hermetic --------- Co-authored-by: itrytoohard <ayetrytoohard@gmail.com>
This commit is contained in:
prateek
GitHub
parent
c3eeecb686
commit
5897b4e8d8
|
|
@ -10,7 +10,7 @@ import {
|
|||
resolveProbeDecision,
|
||||
} from "../lifecycle-status-decisions.js";
|
||||
import { createSessionManager } from "../session-manager.js";
|
||||
import { writeMetadata, readMetadataRaw } from "../metadata.js";
|
||||
import { updateMetadata, writeMetadata, readMetadataRaw } from "../metadata.js";
|
||||
import { readObservabilitySummary } from "../observability.js";
|
||||
import type {
|
||||
OrchestratorConfig,
|
||||
|
|
@ -904,6 +904,42 @@ describe("check (single session)", () => {
|
|||
expect(lm.getStates().get("app-1")).toBe("stuck");
|
||||
});
|
||||
|
||||
it("keeps prs metadata deduplicated across repeated detectPR polls", async () => {
|
||||
const detectedPR = makePR({
|
||||
owner: "aoagents",
|
||||
repo: "ReverbCode",
|
||||
number: 143,
|
||||
url: "https://github.com/aoagents/ReverbCode/pull/143",
|
||||
});
|
||||
const mockSCM = createMockSCM({
|
||||
detectPR: vi.fn().mockResolvedValue(detectedPR),
|
||||
});
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
workspace: plugins.workspace,
|
||||
scm: mockSCM,
|
||||
});
|
||||
writeMetadata(env.sessionsDir, "app-1", {
|
||||
worktree: "/tmp",
|
||||
branch: "feat/reverb-fix",
|
||||
status: "working",
|
||||
project: "my-app",
|
||||
agent: "mock-agent",
|
||||
} as SessionMetadata);
|
||||
const realSessionManager = createSessionManager({ config, registry });
|
||||
const lm = createLifecycleManager({ config, registry, sessionManager: realSessionManager });
|
||||
|
||||
for (let i = 0; i < 10; i += 1) {
|
||||
await lm.check("app-1");
|
||||
}
|
||||
|
||||
const meta = readMetadataRaw(env.sessionsDir, "app-1");
|
||||
expect(meta?.["pr"]).toBe(detectedPR.url);
|
||||
expect(meta?.["prs"]?.split(",")).toEqual([detectedPR.url]);
|
||||
expect(mockSCM.detectPR).toHaveBeenCalledTimes(10);
|
||||
});
|
||||
|
||||
it("refreshes worker branch metadata from the current worktree HEAD before PR detection", async () => {
|
||||
const workspacePath = join(env.tmpDir, "worker-ws");
|
||||
const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1");
|
||||
|
|
@ -1323,8 +1359,10 @@ describe("check (single session)", () => {
|
|||
// can race past the lifecycle list() call before adoption resolves.
|
||||
const deadline = Date.now() + 2000;
|
||||
while (Date.now() < deadline) {
|
||||
if (vi.mocked(mockSessionManager.list).mock.calls.length >= 1) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 25));
|
||||
const adoptedCount = [sessionA.branch, sessionB.branch].filter(
|
||||
(branch) => branch === "shared-branch",
|
||||
).length;
|
||||
if (vi.mocked(mockSessionManager.list).mock.calls.length >= 1 && adoptedCount > 0) {
|
||||
break;
|
||||
}
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
|
|
@ -4538,6 +4576,59 @@ describe("multi-PR state machine aggregation", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("2.1b — enrichment metadata uses unique PRs and deletes duplicate-index orphans", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const pr10 = makeMatchingPR({ number: 10, url: "https://github.com/org/my-app/pull/10" });
|
||||
const mockSCM = createMockSCM({
|
||||
enrichSessionsPRBatch: mockBatchEnrichmentPerPR({
|
||||
"org/my-app#10": { state: "open", ciStatus: "passing", reviewDecision: "none" },
|
||||
}),
|
||||
});
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
scm: mockSCM,
|
||||
});
|
||||
const session = makeSession({
|
||||
id: "app-1",
|
||||
status: "pr_open",
|
||||
pr: pr10,
|
||||
prs: [pr10, { ...pr10 }],
|
||||
metadata: {
|
||||
prEnrichment_1: "{\"state\":\"open\"}",
|
||||
prReviewComments_1: "{\"unresolvedThreads\":0}",
|
||||
},
|
||||
});
|
||||
|
||||
const lm = setupPollCheck("app-1", {
|
||||
session,
|
||||
registry,
|
||||
metaOverrides: {
|
||||
pr: pr10.url,
|
||||
prs: `${pr10.url},${pr10.url}`,
|
||||
prEnrichment_1: "{\"state\":\"open\"}",
|
||||
prReviewComments_1: "{\"unresolvedThreads\":0}",
|
||||
},
|
||||
});
|
||||
updateMetadata(env.sessionsDir, "app-1", {
|
||||
prEnrichment_1: "{\"state\":\"open\"}",
|
||||
prReviewComments_1: "{\"unresolvedThreads\":0}",
|
||||
});
|
||||
|
||||
lm.start(60_000);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
lm.stop();
|
||||
|
||||
const metadata = readMetadataRaw(env.sessionsDir, "app-1");
|
||||
expect(metadata?.["prEnrichment"]).toBeDefined();
|
||||
expect(metadata?.["prEnrichment_1"]).toBeUndefined();
|
||||
expect(metadata?.["prReviewComments_1"]).toBeUndefined();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("2.2 — session merges when ALL PRs are merged", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -123,4 +123,24 @@ describe("sessionFromMetadata — multi-PR (issue #1821)", () => {
|
|||
expect(session.prs[1].owner).toBe("org-b");
|
||||
expect(session.prs[1].repo).toBe("repo-y");
|
||||
});
|
||||
|
||||
it("1.10 — duplicate prs entries are deduplicated by owner, repo, and number", () => {
|
||||
const session = sessionFromMetadata(
|
||||
"app-1",
|
||||
{
|
||||
prs: [
|
||||
"https://github.com/acme/main/pull/10",
|
||||
"https://github.com/acme/main/pull/10",
|
||||
"https://github.com/acme/sub/pull/10",
|
||||
].join(","),
|
||||
branch: "feat/pr-10",
|
||||
},
|
||||
baseOptions,
|
||||
);
|
||||
expect(session.prs.map((pr) => pr.url)).toEqual([
|
||||
"https://github.com/acme/main/pull/10",
|
||||
"https://github.com/acme/sub/pull/10",
|
||||
]);
|
||||
expect(session.pr).toBe(session.prs[0]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { createSessionManager } from "../session-manager.js";
|
||||
import { writeMetadata } from "../metadata.js";
|
||||
import { readMetadataRaw, updateMetadata, writeMetadata } from "../metadata.js";
|
||||
import { recordActivityEvent } from "../activity-events.js";
|
||||
import type { OrchestratorConfig, PluginRegistry, Agent } from "../types.js";
|
||||
import { setupTestContext, teardownTestContext, makeHandle, type TestContext } from "./test-utils.js";
|
||||
|
|
@ -70,6 +70,50 @@ afterEach(() => {
|
|||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
describe("PR metadata startup migration", () => {
|
||||
it("deduplicates corrupt prs CSV data and removes indexed enrichment keys", () => {
|
||||
writeMetadata(sessionsDir, "app-1", {
|
||||
worktree: "/tmp/app-1",
|
||||
branch: "feat/pr-storage",
|
||||
status: "working",
|
||||
project: "my-app",
|
||||
agent: "mock-agent",
|
||||
});
|
||||
updateMetadata(sessionsDir, "app-1", {
|
||||
prs: [
|
||||
"https://github.com/aoagents/ReverbCode/pull/143",
|
||||
"https://github.com/aoagents/ReverbCode/pull/143",
|
||||
"https://github.com/aoagents/ReverbCode/pull/143",
|
||||
].join(","),
|
||||
prEnrichment_1: "{\"state\":\"open\"}",
|
||||
prEnrichment_2: "{\"state\":\"open\"}",
|
||||
prReviewComments_1: "{\"unresolvedThreads\":0}",
|
||||
prReviewComments_2: "{\"unresolvedThreads\":0}",
|
||||
});
|
||||
|
||||
createSessionManager({ config, registry: mockRegistry });
|
||||
|
||||
const meta = readMetadataRaw(sessionsDir, "app-1");
|
||||
expect(meta?.["prs"]).toBe("https://github.com/aoagents/ReverbCode/pull/143");
|
||||
expect(meta?.["prEnrichment_1"]).toBeUndefined();
|
||||
expect(meta?.["prEnrichment_2"]).toBeUndefined();
|
||||
expect(meta?.["prReviewComments_1"]).toBeUndefined();
|
||||
expect(meta?.["prReviewComments_2"]).toBeUndefined();
|
||||
expect(recordActivityEvent).toHaveBeenCalledWith({
|
||||
projectId: "my-app",
|
||||
sessionId: "app-1",
|
||||
source: "session-manager",
|
||||
kind: "metadata.deduplicated",
|
||||
summary: "deduplicated PR metadata: app-1",
|
||||
data: {
|
||||
beforePrCount: 3,
|
||||
afterPrCount: 1,
|
||||
deletedIndexedKeyCount: 4,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("activity event logging", () => {
|
||||
it("records session.spawned after a successful spawn", async () => {
|
||||
const { execFile } = await import("node:child_process");
|
||||
|
|
|
|||
|
|
@ -81,6 +81,7 @@ import {
|
|||
resolveProbeDecision,
|
||||
type LifecycleDecision,
|
||||
} from "./lifecycle-status-decisions.js";
|
||||
import { dedupePrInfos } from "./utils/pr.js";
|
||||
import {
|
||||
buildCIFailureNotificationData,
|
||||
buildPRStateNotificationData,
|
||||
|
|
@ -320,7 +321,9 @@ function buildEventContext(
|
|||
session: Session | ReactionSessionContext,
|
||||
prEnrichmentCache: Map<string, PREnrichmentData>,
|
||||
): EventContext {
|
||||
const sessionPRs = "prs" in session && Array.isArray(session.prs) ? session.prs : (session.pr ? [session.pr] : []);
|
||||
const sessionPRs = dedupePrInfos(
|
||||
"prs" in session && Array.isArray(session.prs) ? session.prs : session.pr ? [session.pr] : [],
|
||||
);
|
||||
|
||||
const prs: EventContext["prs"] = sessionPRs.map((p) => {
|
||||
const cached = prEnrichmentCache.get(`${p.owner}/${p.repo}#${p.number}`);
|
||||
|
|
@ -508,6 +511,32 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
*/
|
||||
const prEnrichmentCache = new Map<string, PREnrichmentData>();
|
||||
|
||||
function normalizeSessionPRs(session: Session): PRInfo[] {
|
||||
const candidatePRs = session.prs.length > 0 ? session.prs : session.pr ? [session.pr] : [];
|
||||
const uniquePRs = dedupePrInfos(candidatePRs);
|
||||
if (uniquePRs.length !== session.prs.length || session.pr !== (uniquePRs[0] ?? null)) {
|
||||
session.prs = uniquePRs;
|
||||
session.pr = uniquePRs[0] ?? null;
|
||||
}
|
||||
return uniquePRs;
|
||||
}
|
||||
|
||||
function indexedPRMetadataCleanup(
|
||||
session: Session,
|
||||
prCount: number,
|
||||
): Partial<Record<string, string>> {
|
||||
const updates: Partial<Record<string, string>> = {};
|
||||
for (const key of Object.keys(session.metadata)) {
|
||||
const match = key.match(/^(prEnrichment|prReviewComments)_(\d+)$/);
|
||||
if (!match) continue;
|
||||
const index = Number.parseInt(match[2], 10);
|
||||
if (Number.isNaN(index) || index >= prCount) {
|
||||
updates[key] = "";
|
||||
}
|
||||
}
|
||||
return updates;
|
||||
}
|
||||
|
||||
function getPREnrichmentForSession(
|
||||
session: Session | ReactionSessionContext,
|
||||
): PREnrichmentData | undefined {
|
||||
|
|
@ -555,10 +584,11 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
reposByPlugin.set(pluginKey, new Set());
|
||||
}
|
||||
reposByPlugin.get(pluginKey)!.add(project.repo);
|
||||
if (session.prs.length === 0) continue;
|
||||
const sessionPRs = normalizeSessionPRs(session);
|
||||
if (sessionPRs.length === 0) continue;
|
||||
// Loop over all PRs in the session — supports multi-repo sessions
|
||||
// where an agent opened PRs on multiple repos.
|
||||
for (const pr of session.prs) {
|
||||
for (const pr of sessionPRs) {
|
||||
const actualPRRepo = `${pr.owner}/${pr.repo}`;
|
||||
if (actualPRRepo !== project.repo) {
|
||||
reposByPlugin.get(pluginKey)!.add(actualPRRepo);
|
||||
|
|
@ -690,13 +720,14 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
continue;
|
||||
// Skip detectPR only if we already have a PR on the configured project repo.
|
||||
// This allows detecting additional PRs on different repos (multi-repo support).
|
||||
const trackedRepos = new Set(session.prs.map((p) => `${p.owner}/${p.repo}`));
|
||||
const sessionPRs = normalizeSessionPRs(session);
|
||||
const trackedRepos = new Set(sessionPRs.map((p) => `${p.owner}/${p.repo}`));
|
||||
const projectRepoForDetect = config.projects[session.projectId]?.repo;
|
||||
// primaryPR.branch is always the session branch (metadata doesn't store per-PR branches),
|
||||
// so use the lifecycle closed-state alone to allow re-detection after a PR is rejected.
|
||||
const primaryPRIsClosed = session.lifecycle.pr.state === "closed";
|
||||
if (
|
||||
session.prs.length > 0 &&
|
||||
sessionPRs.length > 0 &&
|
||||
projectRepoForDetect &&
|
||||
trackedRepos.has(projectRepoForDetect) &&
|
||||
!primaryPRIsClosed
|
||||
|
|
@ -720,7 +751,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
// in the same session (e.g. agent opens PR #10 and PR #11 both on acme/main-app).
|
||||
// Only skip if we already have this exact PR number on this exact repo.
|
||||
// If the existing PR on the same repo is closed, replace it with the new one.
|
||||
const alreadyTracked = session.prs.some(
|
||||
const alreadyTracked = sessionPRs.some(
|
||||
(p) =>
|
||||
p.owner === detectedPR.owner &&
|
||||
p.repo === detectedPR.repo &&
|
||||
|
|
@ -741,10 +772,11 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
)
|
||||
.concat(detectedPR);
|
||||
}
|
||||
session.prs = dedupePrInfos(session.prs);
|
||||
// pr is always the primary (first) PR
|
||||
session.pr = session.prs[0] ?? detectedPR;
|
||||
const sessionsDir = getProjectSessionsDir(session.projectId);
|
||||
const allPrUrls = session.prs.map((p) => p.url).join(",");
|
||||
const allPrUrls = [...new Set(session.prs.map((p) => p.url))].join(",");
|
||||
updateMetadata(sessionsDir, session.id, {
|
||||
pr: session.pr.url,
|
||||
prs: allPrUrls,
|
||||
|
|
@ -798,10 +830,18 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
*/
|
||||
function persistPREnrichmentToMetadata(sessions: Session[]): void {
|
||||
for (const session of sessions) {
|
||||
const sessionPRs = normalizeSessionPRs(session);
|
||||
if (!session.pr) continue;
|
||||
const project = config.projects[session.projectId];
|
||||
if (!project) continue;
|
||||
const sessionsDir = getProjectSessionsDir(session.projectId);
|
||||
const cleanupUpdates = indexedPRMetadataCleanup(session, sessionPRs.length);
|
||||
if (Object.keys(cleanupUpdates).length > 0) {
|
||||
updateMetadata(sessionsDir, session.id, cleanupUpdates);
|
||||
session.metadata = Object.fromEntries(
|
||||
Object.entries(session.metadata).filter(([key]) => cleanupUpdates[key] === undefined),
|
||||
);
|
||||
}
|
||||
|
||||
const prKey = `${session.pr.owner}/${session.pr.repo}#${session.pr.number}`;
|
||||
const cached = prEnrichmentCache.get(prKey);
|
||||
|
|
@ -835,8 +875,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
}
|
||||
}
|
||||
|
||||
for (let i = 1; i < session.prs.length; i++) {
|
||||
const secondaryPR = session.prs[i];
|
||||
for (let i = 1; i < sessionPRs.length; i++) {
|
||||
const secondaryPR = sessionPRs[i];
|
||||
if (!secondaryPR) continue;
|
||||
const secondaryKey = `${secondaryPR.owner}/${secondaryPR.repo}#${secondaryPR.number}`;
|
||||
const secondaryCached = prEnrichmentCache.get(secondaryKey);
|
||||
|
|
@ -1933,8 +1973,13 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
|
||||
// Persist per-PR review comment blobs for secondary PRs so the dashboard
|
||||
// can enrich them independently (prReviewComments_1, prReviewComments_2, …).
|
||||
for (let i = 1; i < session.prs.length; i++) {
|
||||
const secondaryPR = session.prs[i];
|
||||
const sessionPRs = normalizeSessionPRs(session);
|
||||
const cleanupUpdates = indexedPRMetadataCleanup(session, sessionPRs.length);
|
||||
if (Object.keys(cleanupUpdates).length > 0) {
|
||||
updateSessionMetadata(session, cleanupUpdates);
|
||||
}
|
||||
for (let i = 1; i < sessionPRs.length; i++) {
|
||||
const secondaryPR = sessionPRs[i];
|
||||
if (!secondaryPR) continue;
|
||||
let secondaryThreads: ReviewComment[];
|
||||
let secondaryReviews: ReviewSummary[];
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ import {
|
|||
normalizeOrchestratorSessionStrategy,
|
||||
} from "./orchestrator-session-strategy.js";
|
||||
import { sessionFromMetadata } from "./utils/session-from-metadata.js";
|
||||
import { dedupePrUrls } from "./utils/pr.js";
|
||||
import { safeJsonParse, validateStatus } from "./utils/validation.js";
|
||||
import { isGitBranchNameSafe } from "./utils.js";
|
||||
import { resolveAgentSelection, resolveAgentSelectionForSession } from "./agent-selection.js";
|
||||
|
|
@ -104,6 +105,7 @@ import {
|
|||
const execFileAsync = promisify(execFile);
|
||||
const OPENCODE_DISCOVERY_TIMEOUT_MS = 10_000;
|
||||
const OPENCODE_INTERACTIVE_DISCOVERY_TIMEOUT_MS = 10_000;
|
||||
const INDEXED_PR_METADATA_KEY_REGEX = /^(prEnrichment|prReviewComments)_\d+$/;
|
||||
// On Windows, execFile cannot resolve .cmd shim extensions without invoking the shell.
|
||||
// windowsHide:true suppresses the conhost popup that the shell would otherwise flash.
|
||||
const EXEC_SHELL_OPTION =
|
||||
|
|
@ -538,6 +540,57 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
sessionCache = null;
|
||||
}
|
||||
|
||||
function deduplicatePRStorageOnStartup(): void {
|
||||
let migrated = false;
|
||||
|
||||
for (const [projectId] of Object.entries(config.projects)) {
|
||||
const sessionsDir = getProjectSessionsDir(projectId);
|
||||
if (!existsSync(sessionsDir)) continue;
|
||||
|
||||
for (const sessionName of listMetadata(sessionsDir)) {
|
||||
const raw = readMetadataRaw(sessionsDir, sessionName);
|
||||
if (!raw) continue;
|
||||
|
||||
const rawPrUrls = raw["prs"]
|
||||
? raw["prs"].split(",").map((url) => url.trim()).filter(Boolean)
|
||||
: [];
|
||||
const uniquePrUrls = dedupePrUrls(rawPrUrls);
|
||||
const updates: Partial<Record<string, string>> = {};
|
||||
if (rawPrUrls.length !== uniquePrUrls.length) {
|
||||
updates["prs"] = uniquePrUrls.join(",");
|
||||
}
|
||||
|
||||
let deletedIndexedKeyCount = 0;
|
||||
for (const key of Object.keys(raw)) {
|
||||
if (!INDEXED_PR_METADATA_KEY_REGEX.test(key)) continue;
|
||||
updates[key] = "";
|
||||
deletedIndexedKeyCount += 1;
|
||||
}
|
||||
|
||||
if (Object.keys(updates).length === 0) continue;
|
||||
|
||||
updateMetadata(sessionsDir, sessionName, updates);
|
||||
migrated = true;
|
||||
recordActivityEvent({
|
||||
projectId,
|
||||
sessionId: sessionName,
|
||||
source: "session-manager",
|
||||
kind: "metadata.deduplicated",
|
||||
summary: `deduplicated PR metadata: ${sessionName}`,
|
||||
data: {
|
||||
beforePrCount: rawPrUrls.length,
|
||||
afterPrCount: uniquePrUrls.length,
|
||||
deletedIndexedKeyCount,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (migrated) invalidateCache();
|
||||
}
|
||||
|
||||
deduplicatePRStorageOnStartup();
|
||||
|
||||
function repairSessionAgentMetadataOnRead(
|
||||
sessionsDir: string,
|
||||
record: ActiveSessionRecord,
|
||||
|
|
@ -3199,11 +3252,9 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
// Stack: push claimed PR to front — it becomes primary (prs[0]) on next load.
|
||||
// Filter out duplicates, keep all other tracked PRs at the back.
|
||||
const existingPrs = raw["prs"] ?? raw["pr"] ?? "";
|
||||
const otherPrs = existingPrs
|
||||
.split(",")
|
||||
.map((u) => u.trim())
|
||||
.filter((u) => u && u !== pr.url)
|
||||
.join(",");
|
||||
const otherPrs = dedupePrUrls(
|
||||
existingPrs.split(",").filter((u) => u.trim() !== pr.url),
|
||||
).join(",");
|
||||
const newPrs = otherPrs ? `${pr.url},${otherPrs}` : pr.url;
|
||||
// Clear stale positional enrichment blobs — claimPR reorders prs[] so
|
||||
// index-keyed blobs no longer match. Lifecycle poll rewrites them within ~30s.
|
||||
|
|
|
|||
|
|
@ -57,6 +57,49 @@ export function parsePrFromUrl(prUrl: string): ParsedPrUrl | null {
|
|||
return null;
|
||||
}
|
||||
|
||||
export function prIdentityKey(pr: Pick<PRInfo, "owner" | "repo" | "number" | "url">): string {
|
||||
const parsed = parsePrFromUrl(pr.url);
|
||||
const owner = pr.owner || parsed?.owner || "";
|
||||
const repo = pr.repo || parsed?.repo || "";
|
||||
const number = pr.number || parsed?.number || 0;
|
||||
if (owner && repo && number > 0) {
|
||||
return `${owner}/${repo}#${number}`;
|
||||
}
|
||||
return `url:${pr.url}`;
|
||||
}
|
||||
|
||||
export function dedupePrInfos<T extends Pick<PRInfo, "owner" | "repo" | "number" | "url">>(
|
||||
prs: readonly T[],
|
||||
): T[] {
|
||||
const seen = new Set<string>();
|
||||
const unique: T[] = [];
|
||||
for (const pr of prs) {
|
||||
const key = prIdentityKey(pr);
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
unique.push(pr);
|
||||
}
|
||||
return unique;
|
||||
}
|
||||
|
||||
export function dedupePrUrls(urls: readonly string[]): string[] {
|
||||
const seen = new Set<string>();
|
||||
const unique: string[] = [];
|
||||
for (const rawUrl of urls) {
|
||||
const url = rawUrl.trim();
|
||||
if (!url) continue;
|
||||
const parsed = parsePrFromUrl(url);
|
||||
const key =
|
||||
parsed && parsed.owner && parsed.repo && parsed.number > 0
|
||||
? `${parsed.owner}/${parsed.repo}#${parsed.number}`
|
||||
: `url:${url}`;
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
unique.push(url);
|
||||
}
|
||||
return unique;
|
||||
}
|
||||
|
||||
function tryParseUrl(prUrl: string): URL | null {
|
||||
try {
|
||||
return new URL(prUrl);
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import type {
|
|||
import { deriveLegacyStatus, parseCanonicalLifecycle } from "../lifecycle-state.js";
|
||||
import { createActivitySignal } from "../activity-signal.js";
|
||||
import { AGENT_REPORT_METADATA_KEYS } from "../agent-report.js";
|
||||
import { parsePrFromUrl } from "./pr.js";
|
||||
import { dedupePrInfos, parsePrFromUrl } from "./pr.js";
|
||||
import { safeJsonParse, validateStatus } from "./validation.js";
|
||||
|
||||
interface SessionFromMetadataOptions {
|
||||
|
|
@ -85,11 +85,17 @@ export function sessionFromMetadata(
|
|||
// Old sessions only have a single "pr" field — wrap it for backwards compat.
|
||||
const prsRaw = meta["prs"];
|
||||
const lifecyclePrNumber = lifecycle.pr.number ?? null;
|
||||
const prs: PRInfo[] = prsRaw
|
||||
? prsRaw.split(",").map((u, i) => buildPRInfo(u.trim(), i === 0 ? prIsDraft : false, i === 0 ? lifecyclePrNumber : null)).filter((p) => Boolean(p.url))
|
||||
const parsedPrs: PRInfo[] = prsRaw
|
||||
? prsRaw
|
||||
.split(",")
|
||||
.map((u, i) =>
|
||||
buildPRInfo(u.trim(), i === 0 ? prIsDraft : false, i === 0 ? lifecyclePrNumber : null),
|
||||
)
|
||||
.filter((p) => Boolean(p.url))
|
||||
: prUrl
|
||||
? [buildPRInfo(prUrl, prIsDraft, lifecyclePrNumber)]
|
||||
: [];
|
||||
const prs = dedupePrInfos(parsedPrs);
|
||||
|
||||
return {
|
||||
id: sessionId,
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@
|
|||
* Mocks ONLY the I/O boundary: node:child_process and node:os.
|
||||
* Everything else runs for real: config parsing, escaping chains, formatting.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, type Mock } from "vitest";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from "vitest";
|
||||
import type { NotifyAction } from "@aoagents/ao-core";
|
||||
import { makeEvent } from "./helpers/event-factory.js";
|
||||
|
||||
|
|
@ -24,11 +24,17 @@ vi.mock("node:os", () => ({
|
|||
platform: vi.fn(() => "darwin"),
|
||||
}));
|
||||
|
||||
import { execFile } from "node:child_process";
|
||||
import { execFile, execFileSync } from "node:child_process";
|
||||
import { platform } from "node:os";
|
||||
|
||||
const mockExecFile = execFile as unknown as Mock;
|
||||
const mockExecFileSync = execFileSync as unknown as Mock;
|
||||
const mockPlatform = platform as unknown as Mock;
|
||||
const originalProcessPlatform = Object.getOwnPropertyDescriptor(process, "platform");
|
||||
|
||||
function setProcessPlatform(value: NodeJS.Platform): void {
|
||||
Object.defineProperty(process, "platform", { value, configurable: true });
|
||||
}
|
||||
|
||||
// Import the full plugin module — config parsing, escaping, formatting all run for real
|
||||
import desktopPlugin from "@aoagents/ao-plugin-notifier-desktop";
|
||||
|
|
@ -37,6 +43,12 @@ describe("notifier-desktop integration", () => {
|
|||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockPlatform.mockReturnValue("darwin");
|
||||
setProcessPlatform("darwin");
|
||||
mockExecFileSync.mockImplementation(() => {
|
||||
const error = new Error("not found") as NodeJS.ErrnoException;
|
||||
error.code = "ENOENT";
|
||||
throw error;
|
||||
});
|
||||
mockExecFile.mockImplementation((..._args: unknown[]) => {
|
||||
// execFile is called as (cmd, args, cb) on darwin/linux and as
|
||||
// (cmd, args, opts, cb) on win32 — pick whichever trailing arg is the
|
||||
|
|
@ -48,6 +60,12 @@ describe("notifier-desktop integration", () => {
|
|||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalProcessPlatform) {
|
||||
Object.defineProperty(process, "platform", originalProcessPlatform);
|
||||
}
|
||||
});
|
||||
|
||||
describe("config -> behavior flow", () => {
|
||||
it("sound=false config + urgent event -> no sound clause in osascript", async () => {
|
||||
const notifier = desktopPlugin.create({ sound: false });
|
||||
|
|
@ -124,6 +142,7 @@ describe("notifier-desktop integration", () => {
|
|||
|
||||
it("linux -> notify-send with title and message as separate args", async () => {
|
||||
mockPlatform.mockReturnValue("linux");
|
||||
setProcessPlatform("linux");
|
||||
const notifier = desktopPlugin.create();
|
||||
await notifier.notify(makeEvent({ sessionId: "backend-1", message: "Test msg" }));
|
||||
|
||||
|
|
@ -135,6 +154,7 @@ describe("notifier-desktop integration", () => {
|
|||
|
||||
it("linux + urgent -> --urgency=critical before title", async () => {
|
||||
mockPlatform.mockReturnValue("linux");
|
||||
setProcessPlatform("linux");
|
||||
const notifier = desktopPlugin.create();
|
||||
await notifier.notify(makeEvent({ priority: "urgent" }));
|
||||
|
||||
|
|
@ -144,6 +164,7 @@ describe("notifier-desktop integration", () => {
|
|||
|
||||
it("linux + info -> no --urgency flag", async () => {
|
||||
mockPlatform.mockReturnValue("linux");
|
||||
setProcessPlatform("linux");
|
||||
const notifier = desktopPlugin.create();
|
||||
await notifier.notify(makeEvent({ priority: "info" }));
|
||||
|
||||
|
|
@ -153,6 +174,7 @@ describe("notifier-desktop integration", () => {
|
|||
|
||||
it("win32 -> powershell.exe with EncodedCommand toast XML", async () => {
|
||||
mockPlatform.mockReturnValue("win32");
|
||||
setProcessPlatform("win32");
|
||||
|
||||
const notifier = desktopPlugin.create();
|
||||
await notifier.notify(makeEvent({ message: "ci test" }));
|
||||
|
|
@ -172,6 +194,7 @@ describe("notifier-desktop integration", () => {
|
|||
|
||||
it("unsupported platform -> no execFile, warns", async () => {
|
||||
mockPlatform.mockReturnValue("freebsd");
|
||||
setProcessPlatform("freebsd");
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const notifier = desktopPlugin.create();
|
||||
await notifier.notify(makeEvent());
|
||||
|
|
@ -222,6 +245,7 @@ describe("notifier-desktop integration", () => {
|
|||
|
||||
it("rejects when execFile fails on linux", async () => {
|
||||
mockPlatform.mockReturnValue("linux");
|
||||
setProcessPlatform("linux");
|
||||
mockExecFile.mockImplementation(
|
||||
(_cmd: string, _args: string[], cb: (err: Error | null) => void) => {
|
||||
cb(new Error("notify-send: not found"));
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@
|
|||
|
||||
import { request } from "node:https";
|
||||
import type { ProjectConfig } from "@aoagents/ao-core";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest";
|
||||
import trackerLinear from "@aoagents/ao-plugin-tracker-linear";
|
||||
import { pollUntil, pollUntilEqual } from "./helpers/polling.js";
|
||||
|
||||
|
|
@ -144,6 +144,13 @@ async function retryExternal<T>(operation: () => Promise<T>, attempts = 3): Prom
|
|||
throw lastError;
|
||||
}
|
||||
|
||||
function isLinearAuthError(err: unknown): boolean {
|
||||
if (!(err instanceof Error)) return false;
|
||||
return /(?:HTTP 401|AUTHENTICATION_ERROR|Authentication required|not authenticated)/i.test(
|
||||
err.message,
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -166,22 +173,30 @@ describe.skipIf(!canRun)("tracker-linear (integration)", () => {
|
|||
// Issue state tracked across tests (created in beforeAll, cleaned up in afterAll)
|
||||
let issueIdentifier: string; // e.g. "INT-1234"
|
||||
let issueUuid: string | undefined; // Linear internal UUID (needed for trash via direct API)
|
||||
let setupSkipReason: string | undefined;
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Setup — create a test issue
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
beforeAll(async () => {
|
||||
const result = await retryExternal(() =>
|
||||
tracker.createIssue!(
|
||||
{
|
||||
title: `[AO Integration Test] ${new Date().toISOString()}`,
|
||||
description: "Automated integration test issue. Safe to delete if found lingering.",
|
||||
priority: 4, // Low
|
||||
},
|
||||
project,
|
||||
),
|
||||
);
|
||||
let result: Awaited<ReturnType<NonNullable<typeof tracker.createIssue>>>;
|
||||
try {
|
||||
result = await retryExternal(() =>
|
||||
tracker.createIssue!(
|
||||
{
|
||||
title: `[AO Integration Test] ${new Date().toISOString()}`,
|
||||
description: "Automated integration test issue. Safe to delete if found lingering.",
|
||||
priority: 4, // Low
|
||||
},
|
||||
project,
|
||||
),
|
||||
);
|
||||
} catch (err) {
|
||||
if (!isLinearAuthError(err)) throw err;
|
||||
setupSkipReason = "Linear credentials are present but not authenticated";
|
||||
return;
|
||||
}
|
||||
|
||||
issueIdentifier = result.id;
|
||||
|
||||
|
|
@ -199,6 +214,12 @@ describe.skipIf(!canRun)("tracker-linear (integration)", () => {
|
|||
}
|
||||
}, 120_000);
|
||||
|
||||
beforeEach((ctx) => {
|
||||
if (setupSkipReason) {
|
||||
ctx.skip(setupSkipReason);
|
||||
}
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Cleanup — archive the test issue so it doesn't clutter the board.
|
||||
// With LINEAR_API_KEY we can trash it directly. With Composio-only we
|
||||
|
|
|
|||
Loading…
Reference in New Issue