fix(api): ingest graph uses approved research and guards checkpoint threads#969
Conversation
- Register graph id ingest-planner with prepare_ingest → P1 research nodes → plan_ingest - Reuse research nodes, shouldRefine, and ZediChatModel (no duplicate tools) - Add POST /api/ingest/graph/run and /graph/resume with TSDoc vs /api/ingest/plan - Extract shouldRefine for shared conditional routing - Vitest: ingest graph wiring and research HITL resume path Co-authored-by: Akimasa Sugai <otomatty@users.noreply.github.com>
…ership plan_ingest ignored HITL-approved sources, so graph runs produced the same plan as /api/ingest/plan despite the research loop. Append approved research to the planner prompt and reject cross-user checkpoint threadId reuse. Co-authored-by: akimasa.sugai <akimasa.sugai@saedgewell.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e983d774cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| threadId: string, | ||
| checkpointer: BaseCheckpointSaver, | ||
| ): Promise<string | null> { | ||
| const registered = getRegisteredGraph(ING_PLANNER_GRAPH_ID); |
There was a problem hiding this comment.
readIngestCheckpointUserId calls getRegisteredGraph(ING_PLANNER_GRAPH_ID), but this identifier is not defined in the module (the imported constant is INGEST_PLANNER_GRAPH_ID). When checkpointing is enabled, /api/ingest/graph/run and /api/ingest/graph/resume both call this path via assertIngestThreadAccessible, so the request can fail with a runtime ReferenceError (or fail typecheck), breaking graph run/resume instead of returning normal responses.
Useful? React with 👍 / 👎.
Combine scoped ingest thread IDs and checkpoint owner checks with formatResearchForIngest for approved research in plan_ingest. Co-authored-by: Akimasa Sugai <otomatty@users.noreply.github.com>
1 participant
Summary
Deep review of PR #968 (
feat(api): Wiki Compose P4 — Ingest research subgraph) found two high-severity issues in the new ingest-planner graph path.Bug 1 — Approved research ignored (user-facing breakage)
Impact: After
POST /api/ingest/graph/run→ HITL athuman_review_research→POST /api/ingest/graph/resume,plan_ingestcalledbuildIngestPlannerPromptwith onlyarticle+candidates. User-approved research from the shared P1 loop never reached the LLM, so graph runs wasted research cost and could return the same plan as/api/ingest/planwithout research context.Root cause:
planIngestdid not readstate.approvedResearch.Fix:
appendApprovedResearchToPlannerMessagesadds an## APPROVED RESEARCHblock to the planner user message (mirrors wiki-composestructure_dialogue).Bug 2 — Checkpoint threadId not bound to user (security)
Impact: Any authenticated user who knows another user's
threadIdcould resume or overwrite that checkpoint (article/candidates in graph state).Root cause:
/api/ingest/graph/runand/graph/resumeaccept clientthreadIdwith no ownership check (unlike compose sessions backed bywiki_compose_sessions).Fix: Before run/resume, read checkpoint
userIdviagetStateand return 403 when it differs from the authenticated user.Validation
bun test src/__tests__/agents/graphs/ingest/planIngest.test.ts(2 tests pass)