Merge branch 'main' into feat/editable-agent-name
This commit is contained in:
5
.changeset/FN-5583-notification-deeplinks.md
Normal file
5
.changeset/FN-5583-notification-deeplinks.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix ntfy notification deep links: project-only links now switch projects, and task links to non-current projects resolve against the correct project before opening the modal.
|
||||
10
.changeset/ci-engine-test-fixes.md
Normal file
10
.changeset/ci-engine-test-fixes.md
Normal file
@@ -0,0 +1,10 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Fix two engine reliability bugs surfaced by CI sharding repair:
|
||||
|
||||
- Self-healing in-review branch rebind now dedups case-variant candidate refs by resolved SHA rather than lowercase name, so two distinct branches sharing a case-insensitive name on case-sensitive filesystems (Linux) are correctly flagged as ambiguous instead of one being silently picked.
|
||||
- CI test sharding: removed the `--` separator between `pnpm test` and `--shard`, which vitest's CLI parser was treating as end-of-flags and turning the shard selector into a positional file filter — silently disabling sharding so every shard ran the full suite. Test shards now run their actual slice.
|
||||
- CI test-shards jobs now check out with `fetch-depth: 0` so engine tests that depend on real git history (merge-base, ref resolution) behave the same on CI as locally.
|
||||
- PR Checks workflow now also runs on push to `main`, so post-merge regressions surface immediately instead of waiting for the next PR.
|
||||
10
.github/workflows/pr-checks.yml
vendored
10
.github/workflows/pr-checks.yml
vendored
@@ -3,6 +3,10 @@ name: PR Checks
|
||||
on:
|
||||
pull_request:
|
||||
branches: [main]
|
||||
# Also run on every push to main so post-merge regressions surface
|
||||
# immediately instead of being discovered on the next PR.
|
||||
push:
|
||||
branches: [main]
|
||||
|
||||
concurrency:
|
||||
group: pr-checks-${{ github.ref }}
|
||||
@@ -69,6 +73,12 @@ jobs:
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
# Engine tests run real git operations (merge-base against main,
|
||||
# case-variant ref checks) that require full history. Shallow
|
||||
# clones silently break tests like worktree-acquisition's resume
|
||||
# misbinding path.
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Setup Node.js and pnpm
|
||||
uses: ./.github/actions/setup-node-pnpm
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { StrictMode, createElement, type PropsWithChildren } from "react";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
import { useDeepLink } from "../useDeepLink";
|
||||
import * as api from "../../api";
|
||||
@@ -150,17 +151,65 @@ describe("useDeepLink", () => {
|
||||
expect(window.history.replaceState).not.toHaveBeenCalledWith(expect.anything(), "", "/?task=FN-123");
|
||||
});
|
||||
|
||||
it("switches project for project-only deep links without opening task detail", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL("http://localhost:3000/?project=proj_456"),
|
||||
});
|
||||
|
||||
const { setCurrentProject, openTaskDetail, addToast } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setCurrentProject).toHaveBeenCalledTimes(1);
|
||||
expect(setCurrentProject).toHaveBeenCalledWith(otherProject);
|
||||
});
|
||||
|
||||
expect(openTaskDetail).not.toHaveBeenCalled();
|
||||
expect(mockFetchTaskDetail).not.toHaveBeenCalled();
|
||||
expect(addToast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows unknown project toast only once under StrictMode", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL("http://localhost:3000/?project=missing"),
|
||||
});
|
||||
|
||||
const addToast = vi.fn();
|
||||
const strictWrapper = ({ children }: PropsWithChildren) => createElement(StrictMode, null, children);
|
||||
|
||||
renderHook(() => useDeepLink({
|
||||
projectId: defaultProject.id,
|
||||
projects: [defaultProject, otherProject],
|
||||
projectsLoading: false,
|
||||
currentProject: defaultProject,
|
||||
setCurrentProject: vi.fn(),
|
||||
addToast,
|
||||
openTaskDetail: vi.fn(),
|
||||
closeTaskDetail: vi.fn(),
|
||||
}), { wrapper: strictWrapper });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(addToast).toHaveBeenCalledTimes(1);
|
||||
expect(addToast).toHaveBeenCalledWith("Project 'missing' not found", "error");
|
||||
});
|
||||
|
||||
expect(mockFetchTaskDetail).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("switches project and uses project param for task fetch", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL("http://localhost:3000/?project=proj_456&task=FN-999"),
|
||||
});
|
||||
|
||||
const { setCurrentProject } = renderUseDeepLink();
|
||||
const { setCurrentProject, openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setCurrentProject).toHaveBeenCalledTimes(1);
|
||||
expect(setCurrentProject).toHaveBeenCalledWith(otherProject);
|
||||
expect(mockFetchTaskDetail).toHaveBeenCalledWith("FN-999", "proj_456");
|
||||
expect(openTaskDetail).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -170,15 +219,74 @@ describe("useDeepLink", () => {
|
||||
value: new URL("http://localhost:3000/?project=missing&task=FN-123"),
|
||||
});
|
||||
|
||||
const { addToast } = renderUseDeepLink();
|
||||
const { addToast, setCurrentProject } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(addToast).toHaveBeenCalledWith("Project 'missing' not found", "error");
|
||||
});
|
||||
|
||||
expect(addToast).toHaveBeenCalledTimes(1);
|
||||
expect(setCurrentProject).not.toHaveBeenCalled();
|
||||
expect(mockFetchTaskDetail).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps task-only deep-link behavior and strips task on detail close", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL("http://localhost:3000/?task=FN-9999"),
|
||||
});
|
||||
|
||||
const { result, setCurrentProject, closeTaskDetail } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchTaskDetail).toHaveBeenCalledWith("FN-9999", "proj_123");
|
||||
});
|
||||
|
||||
expect(setCurrentProject).not.toHaveBeenCalled();
|
||||
|
||||
result.current.handleDetailClose();
|
||||
expect(window.history.replaceState).toHaveBeenCalledWith(expect.anything(), "", "/");
|
||||
expect(closeTaskDetail).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("leaves mailbox view deep-link params intact after project switch", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL("http://localhost:3000/?project=proj_456&view=mailbox&mailbox-message=msg-1#message-msg-1"),
|
||||
});
|
||||
|
||||
const { setCurrentProject, openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setCurrentProject).toHaveBeenCalledTimes(1);
|
||||
expect(setCurrentProject).toHaveBeenCalledWith(otherProject);
|
||||
});
|
||||
|
||||
expect(mockFetchTaskDetail).not.toHaveBeenCalled();
|
||||
expect(openTaskDetail).not.toHaveBeenCalled();
|
||||
expect(window.location.search).toContain("view=mailbox");
|
||||
expect(window.location.search).toContain("mailbox-message=msg-1");
|
||||
});
|
||||
|
||||
it("switches project for rooms view deep links without consuming room params", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL("http://localhost:3000/?project=proj_456&view=rooms&room=room-1"),
|
||||
});
|
||||
|
||||
const { setCurrentProject, openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setCurrentProject).toHaveBeenCalledTimes(1);
|
||||
expect(setCurrentProject).toHaveBeenCalledWith(otherProject);
|
||||
});
|
||||
|
||||
expect(mockFetchTaskDetail).not.toHaveBeenCalled();
|
||||
expect(openTaskDetail).not.toHaveBeenCalled();
|
||||
expect(window.location.search).toContain("view=rooms");
|
||||
expect(window.location.search).toContain("room=room-1");
|
||||
});
|
||||
|
||||
it("waits for projects to load before resolving deep links", async () => {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
|
||||
@@ -46,6 +46,12 @@ export function useDeepLink(options: UseDeepLinkOptions): UseDeepLinkResult {
|
||||
// Track whether the currently open detail modal came from a deep-link.
|
||||
const deepLinkTaskIdRef = useRef<string | null>(null);
|
||||
|
||||
// Avoid duplicate not-found toasts in StrictMode double-effect runs.
|
||||
const projectNotFoundToastRef = useRef<string | null>(null);
|
||||
|
||||
// Ensure project switching from ?project= only happens once per project value.
|
||||
const projectSwitchAppliedRef = useRef<string | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (!pathRewroteRef.current) {
|
||||
const pathMatch = window.location.pathname.match(/^\/tasks\/([A-Z]+-\d+)\/?$/);
|
||||
@@ -66,25 +72,40 @@ export function useDeepLink(options: UseDeepLinkOptions): UseDeepLinkResult {
|
||||
const projectParam = params.get("project");
|
||||
const taskId = params.get("task");
|
||||
|
||||
if (!taskId) return;
|
||||
if (projectsLoading) return;
|
||||
|
||||
let taskProjectId = projectId;
|
||||
|
||||
if (projectParam) {
|
||||
const matchingProject = projects.find((project) => project.id === projectParam);
|
||||
if (!matchingProject) {
|
||||
addToast(`Project '${projectParam}' not found`, "error");
|
||||
if (projectNotFoundToastRef.current !== projectParam) {
|
||||
addToast(`Project '${projectParam}' not found`, "error");
|
||||
projectNotFoundToastRef.current = projectParam;
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (currentProject?.id !== matchingProject.id) {
|
||||
projectNotFoundToastRef.current = null;
|
||||
taskProjectId = matchingProject.id;
|
||||
|
||||
if (
|
||||
currentProject?.id !== matchingProject.id
|
||||
&& projectSwitchAppliedRef.current !== matchingProject.id
|
||||
) {
|
||||
setCurrentProject(matchingProject);
|
||||
projectSwitchAppliedRef.current = matchingProject.id;
|
||||
}
|
||||
} else {
|
||||
projectNotFoundToastRef.current = null;
|
||||
projectSwitchAppliedRef.current = null;
|
||||
}
|
||||
|
||||
if (!taskId) return;
|
||||
|
||||
if (deepLinkFetchedRef.current) return;
|
||||
deepLinkFetchedRef.current = true;
|
||||
|
||||
const taskProjectId = projectParam ?? projectId;
|
||||
fetchTaskDetail(taskId, taskProjectId)
|
||||
.then((detail) => {
|
||||
openTaskDetail(detail);
|
||||
|
||||
@@ -3,8 +3,11 @@ import { mkdtempSync } from "node:fs";
|
||||
import { rm } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import express from "express";
|
||||
import { TaskStore } from "@fusion/core";
|
||||
import { GitHubTrackingStateService } from "../github-tracking-state.js";
|
||||
import { createApiRoutes } from "../routes.js";
|
||||
import { request as performRequest } from "../test-request.js";
|
||||
|
||||
type GitHubIssueActionPayload = Record<string, unknown>;
|
||||
type StoreEventApi = {
|
||||
@@ -73,6 +76,11 @@ async function expectNoGithubIssueAction(
|
||||
).rejects.toThrow(timeoutMessage);
|
||||
}
|
||||
|
||||
async function requestDelete(app: express.Express, path: string): Promise<{ status: number; body: any }> {
|
||||
const res = await performRequest(app, "DELETE", path);
|
||||
return { status: res.status, body: res.body };
|
||||
}
|
||||
|
||||
describe("github tracking delete flow", () => {
|
||||
let rootDir: string;
|
||||
let globalDir: string;
|
||||
@@ -157,6 +165,98 @@ describe("github tracking delete flow", () => {
|
||||
expect(mockSetIssueState).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes linked issue when delete receives explicit githubIssueAction=close", async () => {
|
||||
const task = await store.createTask({
|
||||
description: "delete tracked task with explicit close",
|
||||
githubTracking: { enabled: true },
|
||||
});
|
||||
|
||||
await store.linkGithubIssue(task.id, {
|
||||
owner: "octocat",
|
||||
repo: "hello-world",
|
||||
number: 10,
|
||||
url: "https://github.com/octocat/hello-world/issues/10",
|
||||
createdAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const closeAction = waitForGithubIssueAction(
|
||||
store,
|
||||
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "success",
|
||||
{ timeoutMessage: `Timed out waiting for explicit close action for deleted task ${task.id}` },
|
||||
);
|
||||
|
||||
await store.deleteTask(task.id, { githubIssueAction: "close" });
|
||||
await closeAction;
|
||||
|
||||
expect(mockSetIssueState).toHaveBeenCalledWith("octocat", "hello-world", 10, "closed", "not_planned");
|
||||
});
|
||||
|
||||
it("does not report failed close when task is done then deleted", async () => {
|
||||
const task = await store.createTask({
|
||||
description: "done then deleted task",
|
||||
githubTracking: { enabled: true },
|
||||
});
|
||||
|
||||
await store.linkGithubIssue(task.id, {
|
||||
owner: "octocat",
|
||||
repo: "hello-world",
|
||||
number: 11,
|
||||
url: "https://github.com/octocat/hello-world/issues/11",
|
||||
createdAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
await store.moveTask(task.id, "done");
|
||||
mockSetIssueState.mockClear();
|
||||
mockGetIssue.mockResolvedValue({ state: "closed" });
|
||||
|
||||
const skippedCloseAction = waitForGithubIssueAction(
|
||||
store,
|
||||
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "skipped",
|
||||
{ timeoutMessage: `Timed out waiting for skipped close action for deleted task ${task.id}` },
|
||||
);
|
||||
|
||||
await store.deleteTask(task.id);
|
||||
await skippedCloseAction;
|
||||
|
||||
expect(mockSetIssueState).not.toHaveBeenCalled();
|
||||
await expectNoGithubIssueAction(
|
||||
store,
|
||||
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "failed",
|
||||
`Unexpected failed close action for done-then-deleted task ${task.id}`,
|
||||
);
|
||||
});
|
||||
|
||||
it("route delete uses same store instance observed by tracking state service", async () => {
|
||||
const task = await store.createTask({
|
||||
description: "route delete tracked task",
|
||||
githubTracking: { enabled: true },
|
||||
});
|
||||
|
||||
await store.linkGithubIssue(task.id, {
|
||||
owner: "octocat",
|
||||
repo: "hello-world",
|
||||
number: 12,
|
||||
url: "https://github.com/octocat/hello-world/issues/12",
|
||||
createdAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
|
||||
const closeAction = waitForGithubIssueAction(
|
||||
store,
|
||||
(payload) => payload.taskId === task.id && payload.action === "close" && payload.outcome === "success",
|
||||
{ timeoutMessage: `Timed out waiting for route close action for deleted task ${task.id}` },
|
||||
);
|
||||
|
||||
const response = await requestDelete(app, `/api/tasks/${task.id}?githubIssueAction=close`);
|
||||
expect(response.status).toBe(200);
|
||||
|
||||
await closeAction;
|
||||
expect(mockSetIssueState).toHaveBeenCalledWith("octocat", "hello-world", 12, "closed", "not_planned");
|
||||
});
|
||||
|
||||
it("does not trigger an unhandled rejection when closing linked issue fails on delete", async () => {
|
||||
const task = await store.createTask({
|
||||
description: "delete tracked task with close failure",
|
||||
|
||||
@@ -2946,16 +2946,26 @@ export class SelfHealingManager {
|
||||
}
|
||||
|
||||
const integrationBase = task.baseBranch || await resolveIntegrationBranch(this.options.rootDir, undefined);
|
||||
const existingCandidatesByRef = new Map<string, { branch: string; aheadCount: number }>();
|
||||
// Dedup by resolved SHA, not by lowercase name. On case-insensitive
|
||||
// filesystems (macOS APFS default) two case-variant refs resolve to the
|
||||
// same underlying ref → same SHA → collapse to canonical. On
|
||||
// case-sensitive filesystems (Linux) two case-variants are physically
|
||||
// distinct refs with distinct SHAs → keep both, so downstream detects
|
||||
// the ambiguity rather than silently picking one.
|
||||
const candidateByRefSha = new Map<string, { branch: string; aheadCount: number }>();
|
||||
const normalizedCandidate = canonicalFusionBranchName(task.id);
|
||||
for (const branch of candidates) {
|
||||
let branchSha: string;
|
||||
try {
|
||||
await execAsync(`git show-ref --verify --quiet ${shellQuote(`refs/heads/${branch}`)}`, {
|
||||
const { stdout } = await execAsync(`git rev-parse --verify ${shellQuote(`refs/heads/${branch}`)}`, {
|
||||
cwd: this.options.rootDir,
|
||||
timeout: 30_000,
|
||||
});
|
||||
branchSha = stdout.trim();
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
if (!branchSha) continue;
|
||||
|
||||
let comparisonBase = integrationBase;
|
||||
try {
|
||||
@@ -2980,18 +2990,16 @@ export class SelfHealingManager {
|
||||
timeout: 30_000,
|
||||
});
|
||||
const aheadCount = Number.parseInt(aheadCountRaw.stdout.trim(), 10);
|
||||
const normalizedBranchRef = branch.toLowerCase();
|
||||
const existingCandidate = existingCandidatesByRef.get(normalizedBranchRef);
|
||||
const normalizedCandidate = canonicalFusionBranchName(task.id);
|
||||
if (!existingCandidate || branch === normalizedCandidate) {
|
||||
existingCandidatesByRef.set(normalizedBranchRef, {
|
||||
const existing = candidateByRefSha.get(branchSha);
|
||||
if (!existing || branch === normalizedCandidate) {
|
||||
candidateByRefSha.set(branchSha, {
|
||||
branch,
|
||||
aheadCount: Number.isFinite(aheadCount) ? aheadCount : 0,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const existingCandidates = [...existingCandidatesByRef.values()];
|
||||
const existingCandidates = [...candidateByRefSha.values()];
|
||||
|
||||
if (existingCandidates.length === 0) {
|
||||
await this.emitBranchRebindAuditEvent({
|
||||
|
||||
@@ -3,6 +3,21 @@ import { describe, expect, it, vi } from "vitest";
|
||||
import * as reportsHook from "../useReports.js";
|
||||
import { ReportsView } from "../ReportsView.js";
|
||||
|
||||
// ReportDetailPanel transitively calls useReportPreview → fetch(), and jsdom
|
||||
// has no fetch. The rejection lands after teardown and React's state update
|
||||
// then references `window`, surfacing as an unhandled error that fails the
|
||||
// suite. Stub the preview API to resolve synchronously.
|
||||
vi.mock("../api.js", () => ({
|
||||
listReports: vi.fn().mockResolvedValue([]),
|
||||
getReport: vi.fn().mockResolvedValue(null),
|
||||
getReportPreviewHtml: vi.fn().mockResolvedValue(""),
|
||||
getReportExportUrl: vi.fn().mockReturnValue(""),
|
||||
approveReport: vi.fn().mockResolvedValue(null),
|
||||
rejectReport: vi.fn().mockResolvedValue(null),
|
||||
publishReport: vi.fn().mockResolvedValue(null),
|
||||
getShareBlocks: vi.fn().mockResolvedValue({}),
|
||||
}));
|
||||
|
||||
describe("ReportsView", () => {
|
||||
it("renders list and compare toggle", () => {
|
||||
vi.spyOn(reportsHook, "useReports").mockReturnValue({
|
||||
|
||||
@@ -1,5 +1,13 @@
|
||||
import "@testing-library/jest-dom/vitest";
|
||||
import { vi } from "vitest";
|
||||
import { cleanup } from "@testing-library/react";
|
||||
import { afterEach, vi } from "vitest";
|
||||
|
||||
// @testing-library/react only auto-registers cleanup when vitest globals are
|
||||
// enabled. We don't enable globals here, so we wire it manually — otherwise
|
||||
// React leaves the test tree mounted, its scheduler fires a deferred update
|
||||
// via setImmediate after the jsdom environment is torn down, and the suite
|
||||
// fails with "ReferenceError: window is not defined".
|
||||
afterEach(() => cleanup());
|
||||
|
||||
if (typeof window !== "undefined") {
|
||||
Object.defineProperty(window, "matchMedia", {
|
||||
|
||||
@@ -396,9 +396,15 @@ export function main(argv = process.argv.slice(2), env = process.env) {
|
||||
console.log(
|
||||
`[ci-test-shard] shard ${shard}/${total}: running ${entry.name} --shard ${entry.shardIndex}/${entry.shardCount}`,
|
||||
);
|
||||
run("pnpm", ["--filter", entry.name, "test", "--", "--shard", `${entry.shardIndex}/${entry.shardCount}`], {
|
||||
env: shardEnv,
|
||||
});
|
||||
// NB: no `--` between `test` and `--shard`. pnpm 10 forwards extra args to
|
||||
// the script regardless, and inserting `--` causes vitest's CLI parser
|
||||
// (cac) to treat `--shard X/Y` as positional file filters → sharding is
|
||||
// silently disabled and every shard runs the full suite.
|
||||
run(
|
||||
"pnpm",
|
||||
["--filter", entry.name, "test", `--shard=${entry.shardIndex}/${entry.shardCount}`],
|
||||
{ env: shardEnv },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user