From b6af36916ece54750e16965a9f6226fbb5d108fb Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 9 Aug 2026 08:31:26 -0700 Subject: [PATCH] FN-8877: import GitHub issue images into planning mode Planning Mode now preserves eligible GitHub issue and comment images through task creation. - Capture image-bearing issue and cached comment bodies when starting Planning Mode. - Persist validated image URLs with planning provenance and attach them after task creation. - Bound browser transport and server downloads while covering the import flow with regression tests. Files changed: .../fn-8877-planning-issue-image-attachments.md | 7 ++ docs/dashboard-guide.md | 4 +- .../dashboard/app/components/GitHubImportModal.tsx | 43 +++++-- .../__tests__/GitHubImportModal.test.tsx | 111 ++++++++++++++++++ .../src/__tests__/issue-image-attachments.test.ts | 33 +++++- .../planning-body-parser-integration.test.ts | 58 ++++++++++ .../__tests__/planning-e2e-plan-creation.test.ts | 34 ++++++ .../__tests__/routes-planning-issue-images.test.ts | 127 +++++++++++++++++++++ .../src/__tests__/routes-planning-tracking.test.ts | 3 + packages/dashboard/src/issue-image-attachments.ts | 37 ++++-- packages/dashboard/src/issue-image-markup.ts | 6 + packages/dashboard/src/planning.ts | 31 ++++- .../src/routes/register-planning-subtask-routes.ts | 28 ++++- packages/dashboard/src/server.ts | 33 ++++-- 14 files changed, 518 insertions(+), 37 deletions(-) Fusion-Task-Id: FN-8877 Fusion-Task-Lineage: 057a8a2b-1c0f-4502-9fb3-7ee136d39563 Co-authored-by: Fusion (runfusion.ai) --- ...n-8877-planning-issue-image-attachments.md | 7 + docs/dashboard-guide.md | 4 +- .../app/components/GitHubImportModal.tsx | 43 ++++-- .../__tests__/GitHubImportModal.test.tsx | 111 +++++++++++++++ .../__tests__/issue-image-attachments.test.ts | 33 ++++- .../planning-body-parser-integration.test.ts | 58 ++++++++ .../planning-e2e-plan-creation.test.ts | 34 +++++ .../routes-planning-issue-images.test.ts | 127 ++++++++++++++++++ .../routes-planning-tracking.test.ts | 3 + .../dashboard/src/issue-image-attachments.ts | 37 +++-- packages/dashboard/src/issue-image-markup.ts | 6 + packages/dashboard/src/planning.ts | 31 ++++- .../register-planning-subtask-routes.ts | 28 +++- packages/dashboard/src/server.ts | 33 +++-- 14 files changed, 518 insertions(+), 37 deletions(-) create mode 100644 .changeset/fn-8877-planning-issue-image-attachments.md create mode 100644 packages/dashboard/src/__tests__/planning-body-parser-integration.test.ts create mode 100644 packages/dashboard/src/__tests__/routes-planning-issue-images.test.ts create mode 100644 packages/dashboard/src/issue-image-markup.ts diff --git a/.changeset/fn-8877-planning-issue-image-attachments.md b/.changeset/fn-8877-planning-issue-image-attachments.md new file mode 100644 index 0000000000..5c5fd82643 --- /dev/null +++ b/.changeset/fn-8877-planning-issue-image-attachments.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Import GitHub issue and loaded comment screenshots from Planning Mode. +category: feature +dev: Uses importIssueImagesFromUrls with persisted imageUrls plus commentsUnavailable and droppedBodyCount markers. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 46d34c06f1..6d85675c2c 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -428,7 +428,7 @@ GitHub issue detail offers direct import and Planning Mode; both preserve source FNXC:GitHubImportDocs 2026-07-30-12:00: GitHub issues and pull requests also offer Chat so operators can discuss the selected upstream link without creating a Fusion task. The link is prefilled, never auto-sent, and GitLab retains its import-only action bar. --> -6. In a GitHub **issue** detail, choose **Import as task** to create the board task directly, **Plan** to open Planning Mode with the issue title, body, and source URL, or **Chat** to prefill the selected issue link. A planned task records the same GitHub source provenance as direct import and preserves the original body in its task description and document. GitHub tracking is enabled when `githubLinkImportedIssuesToTracking` or the resolved GitHub tracking default is on. If a live task already represents the issue, Planning still records truthful provenance but suppresses the second tracking stream; exclusive adoption makes concurrent plans converge on one linked issue. Planning does not download issue/comment image attachments, and breakdown subtasks preserve the source text but do not each link or track the issue. Chat does not create a task or send a message. Pull requests continue to use **Resolve feedback** for task creation and also offer **Chat** with their selected PR link prefilled. The Chat action is GitHub-only; GitLab import actions are unchanged. Each GitHub issue and pull-request comment also has **Import as task**, which creates a separate resolve-feedback task quoting that comment and linking its source without closing the detail window. +6. In a GitHub **issue** detail, choose **Import as task** to create the board task directly, **Plan** to open Planning Mode with the issue title, body, and source URL, or **Chat** to prefill the selected issue link. A planned task records the same GitHub source provenance as direct import and preserves the original body in its task description and document. GitHub tracking is enabled when `githubLinkImportedIssuesToTracking` or the resolved GitHub tracking default is on. If a live task already represents the issue, Planning still records truthful provenance but suppresses the second tracking stream; exclusive adoption makes concurrent plans converge on one linked issue. Planning captures issue and already-loaded comment image references when **Plan** is pressed and downloads them after task creation without re-fetching GitHub; when comments are not loaded yet, only issue-body images are captured. Breakdown subtasks preserve the source text but do not each link, track, or duplicate issue attachments. Chat does not create a task or send a message. Pull requests continue to use **Resolve feedback** for task creation and also offer **Chat** with their selected PR link prefilled. The Chat action is GitHub-only; GitLab import actions are unchanged. Each GitHub issue and pull-request comment also has **Import as task**, which creates a separate resolve-feedback task quoting that comment and linking its source without closing the detail window. Expected outcome: direct import and Plan create source-aware tasks; Plan opens the docked Planning Mode interview with retained issue context, while duplicate source tracking remains singular; Chat opens a focused composer ready for an operator question; comment imports remain available for further feedback. Leaving and returning to **Import Tasks** (for example switching to Board and back) restores the prior context for the current project — provider (GitHub/GitLab), active Issues/PRs tab, label filter, selected repository/remote, GitLab project/group inputs, the **Hide imported** preference, and the previously selected issue/PR — instead of resetting to defaults. When GitLab integration is disabled in Settings, the GitLab provider tab is hidden and any restored GitLab provider preference opens on GitHub instead; saved GitLab URLs and tokens remain configured. The restored selection re-validates against the freshly reloaded list; a selection that no longer exists (e.g. the issue was closed upstream) clears gracefully rather than showing a stuck or empty preview. First-time opens with no prior state keep the existing default-remote auto-detect behavior. State is scoped per project and does not leak across projects. @@ -444,7 +444,7 @@ on one touch-safe row. Labels may wrap inside their own actions rather than bein an inaccessible second row. --> 2. Choose the repository, issue/PR tab, candidate row, and detail action. For GitHub issues, choose **Import as task** for direct tracked creation or **Plan** to start Planning Mode with the issue context. - Expected outcome: direct import creates the board task with GitHub provenance/tracking metadata; Plan creates the same source-aware planned task, subject to the same tracking settings and single-tracker duplicate rule. Planning does not download issue images, and breakdown children retain text only rather than creating multiple issue links. When all GitHub issue actions are available, their full labels remain on one touch-safe action row. + Expected outcome: direct import creates the board task with GitHub provenance/tracking metadata; Plan creates the same source-aware planned task, subject to the same tracking settings and single-tracker duplicate rule. Plan captures issue and already-loaded comment image references when it is pressed, then downloads them after task creation without re-fetching GitHub; if comments are still loading, only issue-body images are captured. Breakdown children retain text only rather than creating multiple issue links or duplicate attachments. When all GitHub issue actions are available, their full labels remain on one touch-safe action row. 3. While a candidate detail sheet is open, use the platform Back gesture or control. Expected outcome: the first Back dismisses only the issue, pull request, or GitLab detail and returns to the import candidate list; a second Back dismisses the import form. diff --git a/packages/dashboard/app/components/GitHubImportModal.tsx b/packages/dashboard/app/components/GitHubImportModal.tsx index 22a33817f2..3f36e6f78c 100644 --- a/packages/dashboard/app/components/GitHubImportModal.tsx +++ b/packages/dashboard/app/components/GitHubImportModal.tsx @@ -44,13 +44,14 @@ import { useEmbeddedPresentation, type ModalPresentation } from "../hooks/useEmb import { getGitHubImportState, saveGitHubImportState } from "../hooks/modalPersistence"; import { FloatingWindow } from "./FloatingWindow"; import { NavigationHistoryContext } from "../hooks/useNavigationHistory"; +import { containsIssueImageMarkup, PER_BODY_MAX_CHARS, TRANSPORT_MAX_CHARS } from "../../src/issue-image-markup"; interface GitHubImportModalProps { isOpen: boolean; onClose: () => void; onImport: (task: Task) => void; /** Optional because callers without Planning Mode retain the direct-import-only surface. */ - onPlanningMode?: (initialPlan: string, workflowId?: string | null, sourceIssue?: { provider: "github"; repository: string; issueNumber: number; url: string; title?: string }) => void; + onPlanningMode?: (initialPlan: string, workflowId?: string | null, sourceIssue?: { provider: "github"; repository: string; issueNumber: number; url: string; title?: string; imageBodies?: string[]; commentsUnavailable?: boolean; droppedBodyCount?: number }) => void; /* FNXC:GitHubImport 2026-07-30-12:00: Chat is deliberately separate from direct import: it seeds a GitHub issue/PR link in the composer, @@ -71,6 +72,13 @@ type TabType = "issues" | "pulls"; type ImportProvider = "github" | "gitlab"; type GitLabResourceTab = "project_issue" | "group_issue" | "merge_request"; +/** + * FNXC:GitHubPlanningSourceIssue 2026-08-09-14:59: Cache identity includes the remote because GitHub issue numbers are repository-local. + */ +function issueDetailCacheKey(owner: string, repo: string, issueNumber: number): string { + return `${owner.trim().toLowerCase()}/${repo.trim().toLowerCase()}#${issueNumber}`; +} + /* FNXC:GitHubImport 2026-06-23-03:30: Comment-thread filter modes: DEFAULT is "all" so both human AND bot comments show. "human"/"bot" narrow the thread. @@ -559,9 +567,13 @@ export function GitHubImportModal({ isOpen, onClose, onImport, onPlanningMode, o /* FNXC:GitHubImport 2026-06-23-03:15: The issue preview pane mirrors the PR preview: the SELECTED issue's full comment thread is fetched ON SELECTION (issues have no checks rollup, so comments only). - Cached by issue number in a ref so re-selecting does not refetch; the body renders immediately while comments stream in (loading/error tracked separately, never blocking the body). + Cached by repository plus issue number in a ref so re-selecting does not refetch; the body renders immediately while comments stream in (loading/error tracked separately, never blocking the body). + + FNXC:GitHubPlanningSourceIssue 2026-08-09-14:59: + Planning capture must never carry comments from another repository that happens to reuse an issue number. + The cache key therefore includes the normalized repository as well as the issue number. */ - const issueDetailCacheRef = useRef>(new Map()); + const issueDetailCacheRef = useRef>(new Map()); const [issueDetail, setIssueDetail] = useState(null); const [issueDetailLoading, setIssueDetailLoading] = useState(false); const [issueDetailError, setIssueDetailError] = useState(null); @@ -1188,9 +1200,22 @@ export function GitHubImportModal({ isOpen, onClose, onImport, onPlanningMode, o if (!selectedIssue || !onPlanningMode || importing || isUrlImported(selectedIssue.html_url)) return; const seed = buildIssuePlanningSeed(selectedIssue); + // FNXC:GitHubPlanningSourceIssue 2026-08-09-14:30: State can briefly retain the prior selection, so only the repository-and-number cache proves captured comments belong to this issue. + const detail = issueDetailCacheRef.current.get(issueDetailCacheKey(owner, repo, selectedIssue.number)); + // FNXC:GitHubPlanningSourceIssue 2026-08-09-14:51: Availability is scoped to the selected issue's cache; global loading/error state can belong to a different, newly selected issue. + const commentsUnavailable = !detail; + const candidates = [selectedIssue.body ?? "", ...(detail?.comments ?? []).map((comment) => comment.body ?? "")].filter(containsIssueImageMarkup); + let transportedChars = 0; + let droppedBodyCount = 0; + const imageBodies = candidates.flatMap((body) => { + if (body.length > PER_BODY_MAX_CHARS || transportedChars + body.length > TRANSPORT_MAX_CHARS) { droppedBodyCount++; return []; } + transportedChars += body.length; + return [body]; + }); + /* FNXC:GitHubPlanningSourceIssue 2026-08-09-14:09: Plan must not await or re-fetch comments; transport ordered image-bearing bodies only, while the server resolves URLs and records partial capture. */ // FNXC:GitHubImport 2026-07-30-00:00: Embedded close navigates to Board, so close first and open Planning last to preserve Planning as the final destination. onClose(); - onPlanningMode(seed, undefined, { provider: "github", repository: `${owner}/${repo}`, issueNumber: selectedIssue.number, url: selectedIssue.html_url, title: selectedIssue.title }); + onPlanningMode(seed, undefined, { provider: "github", repository: `${owner}/${repo}`, issueNumber: selectedIssue.number, url: selectedIssue.html_url, title: selectedIssue.title, ...(imageBodies.length ? { imageBodies } : {}), ...(commentsUnavailable ? { commentsUnavailable: true } : {}), ...(droppedBodyCount ? { droppedBodyCount } : {}) }); }, [activeTab, importing, isUrlImported, issues, onClose, onPlanningMode, owner, repo, selectedIssueNumber]); const fetchPullDetail = useCallback((force: boolean) => { @@ -1260,7 +1285,8 @@ export function GitHubImportModal({ isOpen, onClose, onImport, onPlanningMode, o return; } - const cached = issueDetailCacheRef.current.get(selectedIssueNumber); + const cacheKey = issueDetailCacheKey(owner, repo, selectedIssueNumber); + const cached = issueDetailCacheRef.current.get(cacheKey); if (cached) { setIssueDetail(cached); setIssueDetailLoading(false); @@ -1275,7 +1301,7 @@ export function GitHubImportModal({ isOpen, onClose, onImport, onPlanningMode, o apiFetchGitHubIssueDetail(`${owner.trim()}/${repo.trim()}`, selectedIssueNumber) .then((detail) => { - issueDetailCacheRef.current.set(selectedIssueNumber, detail); + issueDetailCacheRef.current.set(cacheKey, detail); if (issueDetailRequestRef.current !== requestId) return; setIssueDetail(detail); setIssueDetailLoading(false); @@ -1345,6 +1371,7 @@ export function GitHubImportModal({ isOpen, onClose, onImport, onPlanningMode, o const issueNumber = selectedIssueNumber; const repository = `${owner.trim()}/${repo.trim()}`; + const cacheKey = issueDetailCacheKey(owner, repo, issueNumber); setAddingComment(true); if (closeToastTimerRef.current) clearTimeout(closeToastTimerRef.current); setCloseToast(null); @@ -1356,8 +1383,8 @@ export function GitHubImportModal({ isOpen, onClose, onImport, onPlanningMode, o createdAt: new Date().toISOString(), authorIsBot: false, }; - const cachedDetail = issueDetailCacheRef.current.get(issueNumber) ?? { comments: [] }; - issueDetailCacheRef.current.set(issueNumber, { + const cachedDetail = issueDetailCacheRef.current.get(cacheKey) ?? { comments: [] }; + issueDetailCacheRef.current.set(cacheKey, { ...cachedDetail, comments: [...cachedDetail.comments, postedComment], }); diff --git a/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx b/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx index 04f28145de..56702134b2 100644 --- a/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/GitHubImportModal.test.tsx @@ -436,6 +436,7 @@ describe("GitHubImportModal", () => { issueNumber: issue.number, url: issue.html_url, title: issue.title, + commentsUnavailable: true, }, ); expect(apiImportGitHubIssue).not.toHaveBeenCalled(); @@ -460,10 +461,120 @@ describe("GitHubImportModal", () => { issueNumber: issue.number, url: issue.html_url, title: issue.title, + commentsUnavailable: true, }, ); }); + /* + FNXC:GitHubPlanningSourceIssue 2026-08-09-14:31: + Planning must never capture a prior issue's visible detail while a newly selected issue's detail request is pending. + The per-issue cache is the provenance source; absent selected-issue cache records the L1 partial-capture marker. + */ + it("does not capture stale selected-issue comments while the next issue detail loads", async () => { + const firstIssue = { number: 46, title: "First issue", body: "![first body](https://github.com/user-attachments/assets/first-body)", html_url: "https://github.com/dustinbyrne/kb/issues/46", labels: [], state: "open" }; + const secondIssue = { number: 47, title: "Second issue", body: "![second body](https://github.com/user-attachments/assets/second-body)", html_url: "https://github.com/dustinbyrne/kb/issues/47", labels: [], state: "open" }; + const onPlanningMode = vi.fn(); + vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote); + vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([firstIssue, secondIssue]); + vi.mocked(apiFetchGitHubIssueDetail) + .mockResolvedValueOnce({ comments: [{ author: "octocat", body: "![first comment](https://github.com/user-attachments/assets/first-comment)", createdAt: "2026-08-09T00:00:00Z", authorIsBot: false }] }) + .mockImplementationOnce(() => new Promise(() => {})); + + render(); + fireEvent.click(await screen.findByRole("button", { name: /Select issue #46/i })); + await waitFor(() => expect(apiFetchGitHubIssueDetail).toHaveBeenCalledWith("dustinbyrne/kb", 46)); + await act(async () => { await Promise.resolve(); }); + + fireEvent.click(screen.getByRole("button", { name: /Select issue #47/i })); + fireEvent.click(screen.getByTestId("github-import-action-plan")); + + expect(onPlanningMode).toHaveBeenCalledWith( + buildIssuePlanningSeed(secondIssue), + undefined, + expect.objectContaining({ + provider: "github", + issueNumber: secondIssue.number, + imageBodies: [secondIssue.body], + commentsUnavailable: true, + }), + ); + expect(onPlanningMode.mock.calls[0]?.[2]).not.toEqual(expect.objectContaining({ + imageBodies: expect.arrayContaining([expect.stringContaining("first-comment")]), + })); + }); + + it("captures every loaded image-bearing comment in direct-import order without a client image budget", async () => { + const issue = { number: 48, title: "Comment screenshots", body: "![body](https://github.com/user-attachments/assets/body)", html_url: "https://github.com/dustinbyrne/kb/issues/48", labels: [], state: "open" }; + const unresolved = Array.from({ length: 12 }, (_, index) => `![bad-${index}](https://example.com/${index}.png)`).join("\n"); + const later = "![later](https://github.com/user-attachments/assets/later)"; + const onPlanningMode = vi.fn(); + vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote); + vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([issue]); + vi.mocked(apiFetchGitHubIssueDetail).mockResolvedValueOnce({ comments: [ + { author: "one", body: "plain prose only", createdAt: "2026-08-09T00:00:00Z", authorIsBot: false }, + { author: "two", body: unresolved, createdAt: "2026-08-09T00:01:00Z", authorIsBot: false }, + { author: "three", body: later, createdAt: "2026-08-09T00:02:00Z", authorIsBot: false }, + ] }); + + render(); + fireEvent.click(await screen.findByRole("button", { name: /Select issue #48/i })); + await waitFor(() => expect(apiFetchGitHubIssueDetail).toHaveBeenCalledWith("dustinbyrne/kb", 48)); + await act(async () => { await Promise.resolve(); }); + fireEvent.click(screen.getByTestId("github-import-action-plan")); + + expect(onPlanningMode.mock.calls[0]?.[2]).toEqual(expect.objectContaining({ + imageBodies: [issue.body, unresolved, later], + })); + }); + + /* + FNXC:GitHubPlanningSourceIssue 2026-08-09-14:59: + Issue numbers are repository-local. Switching remotes before Plan must not reuse the first + repository's cached comments for an identically numbered issue. + */ + it("keeps captured comment bodies isolated when repositories reuse an issue number", async () => { + const originIssue = { number: 48, title: "Origin screenshot", body: "![origin](https://github.com/user-attachments/assets/origin-body)", html_url: "https://github.com/dustinbyrne/kb/issues/48", labels: [], state: "open" }; + const upstreamIssue = { number: 48, title: "Upstream screenshot", body: "![upstream](https://github.com/user-attachments/assets/upstream-body)", html_url: "https://github.com/upstream/kb/issues/48", labels: [], state: "open" }; + const onPlanningMode = vi.fn(); + vi.mocked(fetchGitRemotes).mockResolvedValueOnce(multipleRemotes); + vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([originIssue]).mockResolvedValueOnce([upstreamIssue]); + vi.mocked(apiFetchGitHubIssueDetail) + .mockResolvedValueOnce({ comments: [{ author: "origin", body: "![origin comment](https://github.com/user-attachments/assets/origin-comment)", createdAt: "2026-08-09T00:00:00Z", authorIsBot: false }] }) + .mockResolvedValueOnce({ comments: [{ author: "upstream", body: "![upstream comment](https://github.com/user-attachments/assets/upstream-comment)", createdAt: "2026-08-09T00:01:00Z", authorIsBot: false }] }); + + render(); + await screen.findByText("Origin screenshot"); + fireEvent.click(screen.getByRole("button", { name: /Select issue #48/i })); + await waitFor(() => expect(apiFetchGitHubIssueDetail).toHaveBeenCalledWith("dustinbyrne/kb", 48)); + fireEvent.change(screen.getByRole("combobox"), { target: { value: "upstream" } }); + await screen.findByText("Upstream screenshot"); + fireEvent.click(screen.getByRole("button", { name: /Select issue #48/i })); + await waitFor(() => expect(apiFetchGitHubIssueDetail).toHaveBeenCalledWith("upstream/kb", 48)); + fireEvent.click(screen.getByTestId("github-import-action-plan")); + + expect(onPlanningMode.mock.calls[0]?.[2]).toEqual(expect.objectContaining({ + repository: "upstream/kb", + imageBodies: [upstreamIssue.body, "![upstream comment](https://github.com/user-attachments/assets/upstream-comment)"], + })); + expect(onPlanningMode.mock.calls[0]?.[2]?.imageBodies).not.toEqual(expect.arrayContaining([expect.stringContaining("origin-comment")])); + }); + + it("drops an oversized image-bearing body whole and records the partial capture", async () => { + const oversized = `![large](https://github.com/user-attachments/assets/large)${"x".repeat(256_000)}`; + const issue = { number: 49, title: "Large screenshot", body: oversized, html_url: "https://github.com/dustinbyrne/kb/issues/49", labels: [], state: "open" }; + const onPlanningMode = vi.fn(); + vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote); + vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([issue]); + + render(); + fireEvent.click(await screen.findByRole("button", { name: /Select issue #49/i })); + fireEvent.click(screen.getByTestId("github-import-action-plan")); + + expect(onPlanningMode.mock.calls[0]?.[2]).toEqual(expect.objectContaining({ droppedBodyCount: 1, commentsUnavailable: true })); + expect(onPlanningMode.mock.calls[0]?.[2]).not.toEqual(expect.objectContaining({ imageBodies: expect.any(Array) })); + }); + it("renders Plan only for selectable GitHub issues with Planning Mode", async () => { const issue = { number: 43, title: "Optional plan", body: "Issue body", html_url: "https://github.com/owner/repo/issues/43", labels: [], state: "open" }; vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote); diff --git a/packages/dashboard/src/__tests__/issue-image-attachments.test.ts b/packages/dashboard/src/__tests__/issue-image-attachments.test.ts index e8660c8689..7901060777 100644 --- a/packages/dashboard/src/__tests__/issue-image-attachments.test.ts +++ b/packages/dashboard/src/__tests__/issue-image-attachments.test.ts @@ -30,7 +30,7 @@ vi.mock("@fusion/core", () => ({ runGhAsync: vi.fn(async () => ""), })); -const { extractIssueImageUrls, importIssueImageAttachments, githubImagePolicy, gitlabImagePolicy } = +const { containsIssueImageMarkup, extractIssueImageUrls, importIssueImageAttachments, importIssueImagesFromUrls, githubImagePolicy, gitlabImagePolicy } = await import("../issue-image-attachments.js"); const PNG = Buffer.from("89504e470d0a1a0a", "hex"); @@ -348,4 +348,35 @@ describe("importIssueImageAttachments", () => { expect(result).toEqual({ attached: 0, failed: 0 }); expect(globalThis.fetch).not.toHaveBeenCalled(); }); + + it("re-resolves persisted URLs before downloading them", async () => { + const result = await importIssueImagesFromUrls( + store as never, + "FN-1", + ["https://169.254.169.254/x.png"], + GH, + ); + expect(result).toEqual({ attached: 0, failed: 1 }); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }); + + it("downloads a policy-allowed persisted URL", async () => { + await expect(importIssueImagesFromUrls( + store as never, + "FN-1", + ["https://github.com/user-attachments/assets/abc-123"], + GH, + )).resolves.toEqual({ attached: 1, failed: 0 }); + expect(store.addAttachment).toHaveBeenCalledTimes(1); + }); + + it("uses a containment-only image-bearing predicate", () => { + const markdown = "![shot](https://github.com/user-attachments/assets/abc-123)"; + const html = ''; + expect(containsIssueImageMarkup(markdown)).toBe(true); + expect(containsIssueImageMarkup(html)).toBe(true); + expect(containsIssueImageMarkup("prose only")).toBe(false); + expect(containsIssueImageMarkup("![foreign](https://example.com/a.png)")).toBe(true); + expect(extractIssueImageUrls([markdown, html], GH)).toHaveLength(1); + }); }); diff --git a/packages/dashboard/src/__tests__/planning-body-parser-integration.test.ts b/packages/dashboard/src/__tests__/planning-body-parser-integration.test.ts new file mode 100644 index 0000000000..acffeb9830 --- /dev/null +++ b/packages/dashboard/src/__tests__/planning-body-parser-integration.test.ts @@ -0,0 +1,58 @@ +// @vitest-environment node + +import { EventEmitter } from "node:events"; +import { describe, expect, it, vi } from "vitest"; +import type { Settings, TaskStore } from "@fusion/core"; +import { createServer } from "../server.js"; +import { request } from "../test-request.js"; + +/** + * FNXC:GitHubPlanningSourceIssue 2026-08-09-15:18: + * This boots server.ts rather than a registrar-only router because the global 100 KiB parser runs + * before planning routes. A production-sized capture must reach route validation so the capture + * contract is usable outside unit harnesses that install an unlimited express.json() parser. + */ +class PlanningParserStore extends EventEmitter { + getRootDir() { return process.cwd(); } + getFusionDir() { return `${process.cwd()}/.fusion`; } + getSettings = vi.fn(async (): Promise => ({} as Settings)); + getSettingsFast = this.getSettings; + getGlobalSettingsStore = () => ({ getSettings: async () => ({}) }); + getAsyncLayer = vi.fn(() => ({ db: { update: vi.fn(() => ({ set: vi.fn(() => ({ where: vi.fn(() => ({ returning: vi.fn(async () => []) })) })) })) } })); + getProjectScopedPluginMcpServers = vi.fn().mockResolvedValue([]); + getTaskWorkflowSelection = vi.fn(); + getWorkflowDefinition = vi.fn(async () => undefined); + getWorkflowSettingValues = vi.fn(() => ({})); + getWorkflowSettingsProjectId = vi.fn(() => "default"); +} + +const app = () => createServer(new PlanningParserStore() as unknown as TaskStore, { noAuth: true }); + +function productionSizedCaptureBodies(): string[] { + // Four valid, whole image-bearing bodies total just below the 1,000,000-character transport cap. + return Array.from({ length: 4 }, () => `![capture](https://github.com/user-attachments/assets/image.png)${"x".repeat(249_900)}`); +} + +describe("planning image capture body parser boundary", () => { + it("passes a production-sized capture to planning route validation instead of the global 100 KiB rejection", async () => { + const body = JSON.stringify({ + initialPlan: "Plan GitHub issue evidence import", + sourceIssue: { + provider: "github", + repository: "owner/repo", + issueNumber: 42, + url: "https://github.com/owner/repo/issues/42", + imageBodies: productionSizedCaptureBodies(), + // Deliberately invalid after an otherwise contract-sized payload: 400 proves the route, + // not the global parser, consumed and validated the full request. + commentsUnavailable: "false", + }, + }); + + expect(Buffer.byteLength(body)).toBeGreaterThan(100 * 1024); + const response = await request(app(), "POST", "/api/planning/start-streaming", body, { "content-type": "application/json" }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: "sourceIssue commentsUnavailable must be boolean" }); + }); +}); diff --git a/packages/dashboard/src/__tests__/planning-e2e-plan-creation.test.ts b/packages/dashboard/src/__tests__/planning-e2e-plan-creation.test.ts index 85a6ea5bd5..b9d67761ae 100644 --- a/packages/dashboard/src/__tests__/planning-e2e-plan-creation.test.ts +++ b/packages/dashboard/src/__tests__/planning-e2e-plan-creation.test.ts @@ -148,6 +148,7 @@ function createStore() { }), createTask, updateTask: vi.fn().mockResolvedValue(undefined), + addAttachment: vi.fn().mockResolvedValue(undefined), logEntry: vi.fn().mockResolvedValue(undefined), upsertTaskDocument: vi.fn().mockResolvedValue(undefined), getGlobalSettingsStore: vi.fn(() => ({ getSettings: vi.fn().mockResolvedValue({}) })), @@ -226,6 +227,7 @@ describe("Planning Mode plan creation E2E", () => { }); afterEach(() => { + vi.unstubAllGlobals(); __resetPlanningState(); __setCreateFnAgent(undefined as never); }); @@ -267,6 +269,38 @@ describe("Planning Mode plan creation E2E", () => { expect(store.createTask).toHaveBeenCalledTimes(1); }); + it("attaches persisted GitHub image URLs through the production CLI planning twin", async () => { + const initialPlan = [ + "Plan work for GitHub issue: Screenshot report", + "", + "Issue description:", + "Captured body.", + "", + "Source: https://github.com/owner/repo/issues/42", + ].join("\n"); + const started = await post(app, "/api/planning/start", { initialPlan }); + expect(started.status).toBe(201); + const planningSession = await getSession(started.body.sessionId); + expect(planningSession).toBeDefined(); + planningSession!.sourceIssue = { + provider: "github", + repository: "owner/repo", + externalIssueId: "42", + issueNumber: 42, + url: "https://github.com/owner/repo/issues/42", + imageUrls: ["https://github.com/user-attachments/assets/body", "https://github.com/user-attachments/assets/comment"], + }; + const fetchMock = vi.fn(async () => new Response(new Uint8Array([1, 2, 3]), { status: 200, headers: { "content-type": "image/png" } })); + vi.stubGlobal("fetch", fetchMock); + + const created = await createTaskFromPlanSession(started.body.sessionId, store); + + expect(created.alreadyCreated).toBe(false); + expect(store.addAttachment).toHaveBeenCalledTimes(2); + expect(store.logEntry).toHaveBeenCalledWith(created.task.id, "Imported 2 image attachments from GitHub issue", "https://github.com/owner/repo/issues/42"); + expect(fetchMock.mock.calls.every(([url]) => String(url).startsWith("https://github.com/user-attachments/assets/"))).toBe(true); + }); + it("converts the lean running plan into a task without a separate validation step", async () => { const start = await post(app, "/api/planning/start", { initialPlan: "Build secure account recovery" }); expect(start.status).toBe(201); diff --git a/packages/dashboard/src/__tests__/routes-planning-issue-images.test.ts b/packages/dashboard/src/__tests__/routes-planning-issue-images.test.ts new file mode 100644 index 0000000000..46e2aed033 --- /dev/null +++ b/packages/dashboard/src/__tests__/routes-planning-issue-images.test.ts @@ -0,0 +1,127 @@ +// @vitest-environment node + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import express from "express"; +import type { TaskStore } from "@fusion/core"; +import { request as performRequest } from "../test-request.js"; + +const sessions = new Map(); + +vi.mock("../planning.js", () => ({ + getSession: (id: string) => sessions.get(id), + getSummary: (id: string) => sessions.get(id)?.summary, + updatePlanningCreateClaim: vi.fn(async () => undefined), + getDurablePlanningSession: vi.fn(async (id: string) => sessions.get(id)), + claimPlanningTaskCreation: vi.fn(async (id: string) => sessions.get(id)), + finalizePlanningTaskCreation: vi.fn(async () => undefined), + reconcilePlanningTaskCreation: vi.fn(async () => undefined), + releasePlanningTaskCreation: vi.fn(async () => undefined), + advancePlanningTaskCreationEpoch: vi.fn(async (id: string) => sessions.get(id)), + formatPlanningTaskHandoff: vi.fn((summary: { description: string }) => summary.description), + validateSession: vi.fn(async () => undefined), + planningProposalClaimId: (sessionId: string) => `planning-session:${sessionId}`, + resolvePlanningSourceIssue: (session: any) => session.sourceIssue ? { + sourceIssue: session.sourceIssue, + sourceMetadata: { issueUrl: session.sourceIssue.url, issueNumber: session.sourceIssue.issueNumber }, + markdown: "## Source Issue\n\n### Original issue description\n\nCaptured body.", + } : undefined, + resolvePlanningIssueImageUrls: (session: any) => ({ + urls: session.sourceIssue?.imageUrls ?? [], + commentsUnavailable: session.sourceIssue?.commentsUnavailable === true, + droppedBodyCount: session.sourceIssue?.droppedBodyCount ?? 0, + }), +})); + +import { registerPlanningSubtaskRoutes } from "../routes/register-planning-subtask-routes.js"; + +const imageUrl = (id: string) => `https://github.com/user-attachments/assets/${id}`; + +function sourceIssue(overrides: Record = {}) { + return { + provider: "github" as const, + repository: "owner/repo", + externalIssueId: "42", + issueNumber: 42, + url: "https://github.com/owner/repo/issues/42", + title: "Screenshot report", + ...overrides, + }; +} + +function session(overrides: Record = {}) { + return { + validated: true, + summary: { title: "Planned task", description: "Implement the fix", suggestedSize: "M", priority: "normal", suggestedDependencies: [], keyDeliverables: [] }, + initialPlan: "initial plan", + history: [], + ...overrides, + }; +} + +function createHarness() { + const addAttachment = vi.fn(async () => undefined); + const logEntry = vi.fn(async () => undefined); + const createTask = vi.fn(async (input: Record) => ({ ...input, id: "FN-IMAGE-1" })); + const store = { + createTask, + addAttachment, + logEntry, + upsertTaskDocument: vi.fn(async () => undefined), + getSettings: vi.fn(async () => ({ githubLinkImportedIssuesToTracking: false })), + getRootDir: vi.fn(() => "/tmp/planning-images"), + getGlobalSettingsStore: vi.fn(() => ({ getSettings: vi.fn(async () => ({})) })), + listTasks: vi.fn(async () => []), + getTask: vi.fn(async () => undefined), + } as unknown as TaskStore; + const warn = vi.fn(); + const app = express(); + app.use(express.json()); + registerPlanningSubtaskRoutes({ + router: app, + getProjectContext: async () => ({ store, projectId: "project-images" }), + planningLogger: { warn, info: vi.fn() }, + rethrowAsApiError: (error: unknown) => { throw error; }, + } as never, { store, parseLastEventId: () => undefined, replayBufferedSSE: () => true }); + app.use((error: { statusCode?: number; message?: string }, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + res.status(error.statusCode ?? 500).json({ error: error.message }); + }); + return { app, addAttachment, logEntry, warn }; +} + +describe("planning create-task GitHub image attachments", () => { + beforeEach(() => { + sessions.clear(); + vi.stubGlobal("fetch", vi.fn(async (url: string) => new Response(new Uint8Array([1, 2, 3]), { + status: 200, + headers: { "content-type": "image/png" }, + }))); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it("attaches captured issue and comment images without reading GitHub again", async () => { + const { app, addAttachment, logEntry } = createHarness(); + sessions.set("images", session({ sourceIssue: sourceIssue({ imageUrls: [imageUrl("body"), imageUrl("comment")] }) })); + + const response = await performRequest(app, "POST", "/planning/create-task", JSON.stringify({ sessionId: "images" }), { "content-type": "application/json" }); + + expect(response.status, JSON.stringify(response.body)).toBe(201); + expect(addAttachment).toHaveBeenCalledTimes(2); + expect(logEntry).toHaveBeenCalledWith("FN-IMAGE-1", "Imported 2 image attachments from GitHub issue", "https://github.com/owner/repo/issues/42"); + expect(fetch).toHaveBeenCalledTimes(2); + expect(vi.mocked(fetch).mock.calls.every(([url]) => String(url).startsWith("https://github.com/user-attachments/assets/"))).toBe(true); + }); + + it("keeps a created task when image download fails and warns for recorded partial capture", async () => { + const { app, addAttachment, logEntry, warn } = createHarness(); + vi.stubGlobal("fetch", vi.fn(async () => { throw new Error("download failed"); })); + sessions.set("partial", session({ sourceIssue: sourceIssue({ imageUrls: [imageUrl("body")], commentsUnavailable: true, droppedBodyCount: 2 }) })); + + const response = await performRequest(app, "POST", "/planning/create-task", JSON.stringify({ sessionId: "partial" }), { "content-type": "application/json" }); + + expect(response.status, JSON.stringify(response.body)).toBe(201); + expect(addAttachment).not.toHaveBeenCalled(); + expect(logEntry).not.toHaveBeenCalledWith("FN-IMAGE-1", expect.stringContaining("image attachment"), expect.anything()); + expect(warn).toHaveBeenCalledWith("Planning GitHub image capture was partial", expect.objectContaining({ commentsUnavailable: true, droppedBodyCount: 2 })); + }); +}); diff --git a/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts b/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts index 47cbd5d102..7d90de77a5 100644 --- a/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts +++ b/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts @@ -241,6 +241,8 @@ describe("planning routes github tracking background dispatch", () => { issueNumber: 42, url: "https://github.com/owner/repo/issues/42", title: "Imported issue", + // An image-bearing body with no policy-allowed URL must still mark this as a post-capture session with an explicit empty list. + imageBodies: ["![foreign](https://example.com/not-ours.png)"], }; const valid = await performRequest(app, "POST", "/planning/start-streaming", JSON.stringify({ @@ -256,6 +258,7 @@ describe("planning routes github tracking background dispatch", () => { externalIssueId: "42", issueNumber: 42, url: sourceIssue.url, + imageUrls: [], }), })); diff --git a/packages/dashboard/src/issue-image-attachments.ts b/packages/dashboard/src/issue-image-attachments.ts index 089c6ccee7..bc008ccf82 100644 --- a/packages/dashboard/src/issue-image-attachments.ts +++ b/packages/dashboard/src/issue-image-attachments.ts @@ -2,6 +2,7 @@ import { createLogger } from "@fusion/core"; const severityAuditLog = createLogger("dashboard-issue-image-attachments"); import { runGhAsync, isGhAvailable, isGhAuthenticated, type TaskStore } from "@fusion/core"; +export { containsIssueImageMarkup, PER_BODY_MAX_CHARS, TRANSPORT_MAX_CHARS } from "./issue-image-markup.js"; /* FNXC:IssueImportAttachments 2026-07-15-11:20: @@ -25,7 +26,7 @@ export const ALLOWED_IMAGE_MIMES = new Set(["image/png", "image/jpeg", "image/gi export const MAX_IMAGE_BYTES = 5 * 1024 * 1024; /** Bound per-issue work: a pathological issue (or a long comment thread) must not stall the import request. */ -const MAX_IMAGES_PER_ISSUE = 10; +export const MAX_IMAGES_PER_ISSUE = 10; const DOWNLOAD_TIMEOUT_MS = 15_000; const MAX_DOWNLOAD_REDIRECTS = 3; @@ -163,6 +164,10 @@ export function gitlabImagePolicy(options: { webBaseUrl: string; webUrl: string; * FNXC:IssueImportAttachments 2026-07-15-11:20: * Bodies embed images two ways and both must be covered: markdown `![alt](url)` (the upload default) and raw `` (common when authors resize a screenshot). */ +/* +FNXC:IssueImportAttachments 2026-08-09-14:09: +Planning Mode captures image-bearing bodies at Plan time but downloads resolved URLs only after task creation, so extraction and download need independent entry points. This containment-only predicate runs on untrusted client data: it must not decide fetchability, and markup occurrence counts cannot prove the authoritative URL cap has been reached. +*/ export function extractIssueImageUrls( bodies: string | null | undefined | Array, policy: ImageImportPolicy, @@ -273,14 +278,19 @@ async function downloadImage( * FNXC:IssueImportAttachments 2026-07-15-11:20: * Best-effort by contract — the caller has already created the task, so a throw here would fail an import that actually succeeded. Returns counts so the caller can log what landed. */ -export async function importIssueImageAttachments( +export async function importIssueImagesFromUrls( store: Pick, taskId: string, - bodies: string | null | undefined | Array, + urls: string[], policy: ImageImportPolicy, ): Promise { - const urls = extractIssueImageUrls(bodies, policy); - if (urls.length === 0) return { attached: 0, failed: 0 }; + // Persisted URLs are untrusted too: resolve again before any credentialed fetch. + const approvedUrls = urls.slice(0, MAX_IMAGES_PER_ISSUE).flatMap((url) => { + const resolved = policy.resolve(url); + return resolved ? [resolved] : []; + }); + const rejectedCount = Math.min(urls.length, MAX_IMAGES_PER_ISSUE) - approvedUrls.length; + if (approvedUrls.length === 0) return { attached: 0, failed: rejectedCount }; const authHeaders = await policy.headers(); let attached = 0; @@ -306,12 +316,21 @@ export async function importIssueImageAttachments( // FNXC:IssueImportAttachments 2026-07-15-14:10: Bound simultaneous remote // work so ten slow screenshots cannot serialize an issue import for minutes. let nextIndex = 0; - await Promise.all(Array.from({ length: Math.min(IMAGE_DOWNLOAD_CONCURRENCY, urls.length) }, async () => { - while (nextIndex < urls.length) { + await Promise.all(Array.from({ length: Math.min(IMAGE_DOWNLOAD_CONCURRENCY, approvedUrls.length) }, async () => { + while (nextIndex < approvedUrls.length) { const index = nextIndex++; - await importOne(index, urls[index]!); + await importOne(index, approvedUrls[index]!); } })); - return { attached, failed }; + return { attached, failed: failed + rejectedCount }; +} + +export async function importIssueImageAttachments( + store: Pick, + taskId: string, + bodies: string | null | undefined | Array, + policy: ImageImportPolicy, +): Promise { + return importIssueImagesFromUrls(store, taskId, extractIssueImageUrls(bodies, policy), policy); } diff --git a/packages/dashboard/src/issue-image-markup.ts b/packages/dashboard/src/issue-image-markup.ts new file mode 100644 index 0000000000..7039834f31 --- /dev/null +++ b/packages/dashboard/src/issue-image-markup.ts @@ -0,0 +1,6 @@ +/* FNXC:IssueImportAttachments 2026-08-09-14:09: This browser-safe predicate only identifies possible image markup; URL policy and attachment caps remain server-side. */ +export function containsIssueImageMarkup(text: string): boolean { + return text.includes("![") || />[ FNXC:PlanningMode 2026-07-21-09:15: Planning questions must never create dashboard Mailbox messages. Retain the optional MessageStore input only as a source-compatible no-op for callers compiled against the prior planning API while route and session code omit every mailbox read/write path. */ +export type PlanningSourceIssue = TaskSourceIssue & { + title?: string; + imageUrls?: string[]; + commentsUnavailable?: boolean; + droppedBodyCount?: number; +}; + type PlanningSessionOptions = { projectId?: string; /** Structured import provenance; only GitHub is accepted by the route boundary. */ - sourceIssue?: TaskSourceIssue & { title?: string }; + sourceIssue?: PlanningSourceIssue; ntfyConfig?: PlanningNtfyConfig; clarificationEnabled?: boolean; /** Workflow selected by the planning entry point; retained for agent rebuilds. */ @@ -336,7 +344,7 @@ export interface DraftInputPayload { summarizedFor?: string; validated?: boolean; workflowId?: string; - sourceIssue?: (TaskSourceIssue & { title?: string }); + sourceIssue?: PlanningSourceIssue; createdTaskId?: string; createClaimStatus?: "none" | "creating" | "created"; claimOwnerToken?: string; @@ -407,7 +415,7 @@ interface Session { /** Workflow selected at session start, retained for agent reconstruction. */ workflowId?: string; /** Structured GitHub import provenance persisted in inputPayload. */ - sourceIssue?: TaskSourceIssue & { title?: string }; + sourceIssue?: PlanningSourceIssue; /** Model override the user picked at draft-create time. Persisted in inputPayload so reopen restores it. */ draftModelProvider?: string; draftModelId?: string; @@ -499,6 +507,16 @@ export function resolvePlanningSourceIssue(session: Pick): { urls: string[]; commentsUnavailable: boolean; droppedBodyCount: number } { + const persisted = session.sourceIssue; + // FNXC:GitHubPlanningSourceIssue 2026-08-09-14:51: An empty persisted list is an intentional post-capture result (including L2 drops), not a legacy omission eligible for seed fallback. + if (Array.isArray(persisted?.imageUrls)) return { urls: persisted.imageUrls, commentsUnavailable: persisted.commentsUnavailable === true, droppedBodyCount: persisted.droppedBodyCount ?? 0 }; + const seed = extractSeedIssueContext(session.initialPlan); + if (!seed) return { urls: [], commentsUnavailable: false, droppedBodyCount: 0 }; + return { urls: extractIssueImageUrls(seed.body, githubImagePolicy()), commentsUnavailable: true, droppedBodyCount: 0 }; +} + interface RateLimitEntry { count: number; firstRequestAt: Date; @@ -4427,6 +4445,13 @@ export async function createTaskFromPlanSession( if (sourceContext) { await sideEffect("Planning create-task GitHub issue document write failed", () => store.upsertTaskDocument?.(task.id, { key: "github-issue", content: sourceContext.markdown, author: "planning", metadata: { planningSessionId: sessionId, source: "github-source-issue" } })); await sideEffect("Planning create-task GitHub source log failed", () => store.logEntry?.(task.id, "Imported from GitHub", sourceContext.sourceIssue.url)); + const images = resolvePlanningIssueImageUrls(session); + /* FNXC:GitHubPlanningSourceIssue 2026-08-09-14:09: CLI planning shares post-create best-effort image download and never reads GitHub at creation time. */ + await sideEffect("Planning create-task GitHub image import failed", async () => { + const result = await importIssueImagesFromUrls(store, task.id, images.urls, githubImagePolicy()); + if (result.attached) await store.logEntry?.(task.id, `Imported ${result.attached} image attachment${result.attached === 1 ? "" : "s"} from GitHub issue`, sourceContext.sourceIssue.url); + }); + if (images.commentsUnavailable || images.droppedBodyCount) diagnostics.warn("Planning GitHub image capture was partial", { taskId: task.id, issueUrl: sourceContext.sourceIssue.url, commentsUnavailable: images.commentsUnavailable, droppedBodyCount: images.droppedBodyCount }); } if (trackingDecision?.suppressedByTaskId) await sideEffect("Planning create-task duplicate source issue log failed", () => store.logEntry?.(task.id, `Source issue already tracked by ${trackingDecision.suppressedByTaskId}`)); await sideEffect("Planning create-task log entry failed", () => store.logEntry?.(task.id, "Created via Planning Mode", `Initial plan: ${(session?.initialPlan ?? "").slice(0, 200)}`)); diff --git a/packages/dashboard/src/routes/register-planning-subtask-routes.ts b/packages/dashboard/src/routes/register-planning-subtask-routes.ts index 69a9176bf2..42a35af16e 100644 --- a/packages/dashboard/src/routes/register-planning-subtask-routes.ts +++ b/packages/dashboard/src/routes/register-planning-subtask-routes.ts @@ -11,6 +11,8 @@ import { } from "@fusion/core"; import { createAgentTask } from "@fusion/engine"; import { normalizePlanningSummaryPayload } from "../planning.js"; +import { extractIssueImageUrls, githubImagePolicy, importIssueImagesFromUrls } from "../issue-image-attachments.js"; +import { PER_BODY_MAX_CHARS, TRANSPORT_MAX_CHARS } from "../issue-image-markup.js"; import { ApiError, badRequest, conflict, notFound, rateLimited } from "../api-error.js"; import { writeSSEEvent, type SessionBufferedEvent } from "../sse-buffer.js"; import type { AiSessionStore } from "../ai-session-store.js"; @@ -697,11 +699,23 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann const validatedSourceIssue = (() => { if (sourceIssue === undefined) return undefined; if (!sourceIssue || typeof sourceIssue !== "object" || (sourceIssue as { provider?: unknown }).provider !== "github") throw badRequest("sourceIssue must be a GitHub issue"); - const value = sourceIssue as { repository?: unknown; issueNumber?: unknown; url?: unknown; title?: unknown }; + const value = sourceIssue as { repository?: unknown; issueNumber?: unknown; url?: unknown; title?: unknown; imageBodies?: unknown; commentsUnavailable?: unknown; droppedBodyCount?: unknown }; if (typeof value.repository !== "string" || typeof value.issueNumber !== "number" || !Number.isInteger(value.issueNumber) || value.issueNumber <= 0 || typeof value.url !== "string") throw badRequest("sourceIssue is malformed"); + if (value.imageBodies !== undefined && (!Array.isArray(value.imageBodies) || value.imageBodies.some((body) => typeof body !== "string"))) throw badRequest("sourceIssue imageBodies must be strings"); + if (value.commentsUnavailable !== undefined && typeof value.commentsUnavailable !== "boolean") throw badRequest("sourceIssue commentsUnavailable must be boolean"); + if (value.droppedBodyCount !== undefined && (typeof value.droppedBodyCount !== "number" || !Number.isInteger(value.droppedBodyCount) || value.droppedBodyCount < 0)) throw badRequest("sourceIssue droppedBodyCount must be a non-negative integer"); const match = value.url.match(/^https:\/\/github\.com\/([^/]+)\/([^/]+)\/issues\/(\d+)\/?$/i); if (!match || match[3] !== String(value.issueNumber) || `${match[1]}/${match[2]}`.toLowerCase() !== value.repository.toLowerCase()) throw badRequest("sourceIssue URL must match repository and issue number"); - return { provider: "github" as const, repository: value.repository, externalIssueId: String(value.issueNumber), issueNumber: value.issueNumber, url: value.url, ...(typeof value.title === "string" ? { title: value.title } : {}) }; + let totalChars = 0; + let droppedBodyCount = value.droppedBodyCount ?? 0; + const bodies = (value.imageBodies ?? []).flatMap((body) => { + if (body.length > PER_BODY_MAX_CHARS || totalChars + body.length > TRANSPORT_MAX_CHARS) { droppedBodyCount++; return []; } + totalChars += body.length; + return [body]; + }); + /* FNXC:GitHubPlanningSourceIssue 2026-08-09-14:09: Bodies are transport-only; server-side policy resolution applies the SSRF boundary and authoritative cap before session persistence. */ + /* FNXC:GitHubPlanningSourceIssue 2026-08-09-14:51: Every newly captured context persists an array, including empty, so L2-dropped bodies cannot fall through to the legacy seed parser and bypass the recorded capture limit. */ + return { provider: "github" as const, repository: value.repository, externalIssueId: String(value.issueNumber), issueNumber: value.issueNumber, url: value.url, ...(typeof value.title === "string" ? { title: value.title } : {}), imageUrls: extractIssueImageUrls(bodies, githubImagePolicy()), ...(value.commentsUnavailable === true ? { commentsUnavailable: true } : {}), ...(droppedBodyCount > 0 ? { droppedBodyCount } : {}) }; })(); if (thinkingLevel !== undefined && !THINKING_LEVELS.includes(thinkingLevel as ThinkingLevel)) { @@ -1227,6 +1241,9 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann const resolvePlanningSourceIssue = "resolvePlanningSourceIssue" in planningSourceModule ? planningSourceModule.resolvePlanningSourceIssue : undefined; + const resolvePlanningIssueImageUrls = "resolvePlanningIssueImageUrls" in planningSourceModule + ? planningSourceModule.resolvePlanningIssueImageUrls + : undefined; const { resolvePlanningGithubTrackingDecision } = await import("../github-tracking.js"); const { appendSourceIssueBlock } = await import("../github.js"); @@ -1542,6 +1559,13 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann if (trackingDecision?.suppressedByTaskId) { await runPlanningCreateSideEffect("Planning create-task duplicate source issue log failed", () => scopedStore.logEntry(task.id, `Source issue already tracked by ${trackingDecision.suppressedByTaskId}`), { taskId: task.id, sessionId }); } + const images = resolvePlanningIssueImageUrls && session ? resolvePlanningIssueImageUrls(session) : { urls: [], commentsUnavailable: false, droppedBodyCount: 0 }; + /* FNXC:GitHubPlanningSourceIssue 2026-08-09-14:09: Attach only after a new task exists; downloads are best-effort and never re-fetch GitHub issue/comment APIs. */ + await runPlanningCreateSideEffect("Planning create-task GitHub image import failed", async () => { + const result = await importIssueImagesFromUrls(scopedStore, task.id, images.urls, githubImagePolicy()); + if (result.attached) await scopedStore.logEntry(task.id, `Imported ${result.attached} image attachment${result.attached === 1 ? "" : "s"} from GitHub issue`, sourceContext.sourceIssue.url); + }, { taskId: task.id, sessionId }); + if (images.commentsUnavailable || images.droppedBodyCount) planningLogger.warn("Planning GitHub image capture was partial", { taskId: task.id, issueUrl: sourceContext.sourceIssue.url, commentsUnavailable: images.commentsUnavailable, droppedBodyCount: images.droppedBodyCount }); } // Log the planning mode creation. diff --git a/packages/dashboard/src/server.ts b/packages/dashboard/src/server.ts index 39c135becc..451c0402cd 100644 --- a/packages/dashboard/src/server.ts +++ b/packages/dashboard/src/server.ts @@ -995,21 +995,30 @@ export function createServer(store: TaskStore, options?: ServerOptions): ReturnT Voice chunks have a route-only 2 MiB parser. The global 100 KiB parser must skip only this endpoint (with or without Express's optional trailing slash) or it rejects before the voice error mapper; rawBody/HMAC behavior remains unchanged elsewhere. + + FNXC:GitHubPlanningSourceIssue 2026-08-09-15:18: + Planning's GitHub image capture legitimately transports up to 1,000,000 characters of + image-bearing issue/comment bodies before the route applies its server-side SSRF policy and + drops the bodies. Give only its start-streaming endpoint a 5 MiB JSON parser, which covers the + worst-case UTF-8 transport plus JSON framing without weakening the global 100 KiB budget. */ - const jsonParser = express.json({ - verify: (req, _res, buf) => { - if (buf.length > 0) { - (req as express.Request & { rawBody?: Buffer }).rawBody = Buffer.from(buf); - } - }, - }); + const preserveRawBody = (req: express.Request, _res: express.Response, buf: Buffer) => { + if (buf.length > 0) { + (req as express.Request & { rawBody?: Buffer }).rawBody = Buffer.from(buf); + } + }; + const jsonParser = express.json({ verify: preserveRawBody }); + const planningImageCaptureParser = express.json({ limit: "5mb", verify: preserveRawBody }); app.use((req, res, next) => { - // Express treats the trailing-slash spelling as the same route, so its parser boundary must, - // too; no broader prefix is exempted from the global rawBody-preserving parser. + // Express treats trailing slashes as equivalent, so parser boundaries must do the same; + // no broader prefix is exempted from the global rawBody-preserving parser. if (req.path === "/api/voice/transcribe" || req.path === "/api/voice/transcribe/") return next(); - return jsonParser(req, res, (error) => { - // Keep the established global 100 KiB rejection observable as 413 instead of allowing - // Express's parser error to fall through to the generic 500 handler. + const parser = req.path === "/api/planning/start-streaming" || req.path === "/api/planning/start-streaming/" + ? planningImageCaptureParser + : jsonParser; + return parser(req, res, (error) => { + // Keep the established global and route-specific size rejections observable as 413 instead + // of allowing Express's parser error to fall through to the generic 500 handler. if ((error as { type?: string } | undefined)?.type === "entity.too.large") return res.status(413).json({ error: "payload-too-large" }); return next(error); });