Add read-only Gira work-map import#19
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a read-only Gira work-map import path, introducing the POST /api/work-map/import/gira endpoint along with associated parsing logic, unit tests, and documentation. The review feedback highlights two key areas for improvement: resolving asymmetric branch matching in findNodeForRef to ensure consistent fallback behavior, and enhancing edge import logic by providing more descriptive warnings for skipped edges and filtering out self-referencing edges.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function findNodeForRef(ref: WorkIdentityRef, nodesById: Map<string, WorkNode>): WorkNode | undefined { | ||
| const exact = nodesById.get(workNodeId(ref)) | ||
| if (exact || ref.branch !== null) return exact | ||
| return [...nodesById.values()].find((node) => node.identity.repo === ref.repo && node.identity.issue === ref.issue) | ||
| } |
There was a problem hiding this comment.
The branch matching logic in findNodeForRef is asymmetric. If a reference has branch: null but the imported node has a branch, it successfully falls back to matching by repo and issue. However, if the reference has a specific branch (e.g., from a parent link) but the imported node has branch: null (unknown branch), the fallback is skipped and the edge is not resolved. Removing the ref.branch !== null check ensures consistent fallback behavior for both cases.
| function findNodeForRef(ref: WorkIdentityRef, nodesById: Map<string, WorkNode>): WorkNode | undefined { | |
| const exact = nodesById.get(workNodeId(ref)) | |
| if (exact || ref.branch !== null) return exact | |
| return [...nodesById.values()].find((node) => node.identity.repo === ref.repo && node.identity.issue === ref.issue) | |
| } | |
| function findNodeForRef(ref: WorkIdentityRef, nodesById: Map<string, WorkNode>): WorkNode | undefined { | |
| const exact = nodesById.get(workNodeId(ref)) | |
| if (exact) return exact | |
| return [...nodesById.values()].find((node) => node.identity.repo === ref.repo && node.identity.issue === ref.issue) | |
| } |
| for (const ref of edgeRefs) { | ||
| const source = findNodeForRef(ref.from, nodesById) | ||
| const target = findNodeForRef(ref.to, nodesById) | ||
| if (!source || !target) { | ||
| warnings.push(`skipped ${ref.kind} edge because one endpoint is not present in the imported queue items`) | ||
| continue | ||
| } |
There was a problem hiding this comment.
The warning message for skipped edges is very generic and does not specify which endpoints are missing, making it difficult to debug import issues. Additionally, there is no check to prevent self-referencing edges (where a node has an edge to itself), which could cause infinite loops or rendering issues in the UI. We should make the warning more descriptive and filter out self-referencing edges.
for (const ref of edgeRefs) {
const source = findNodeForRef(ref.from, nodesById)
const target = findNodeForRef(ref.to, nodesById)
if (!source || !target) {
const missing = !source && !target ? 'both endpoints' : !source ? 'source' : 'target'
warnings.push(`skipped ${ref.kind} edge from ${ref.from.repo}#${ref.from.issue} to ${ref.to.repo}#${ref.to.issue} because ${missing} not present in the imported queue items`)
continue
}
if (source.id === target.id) {
warnings.push(`skipped self-referencing ${ref.kind} edge for ${source.id}`)
continue
}
|
Strict review findings:
Verification run:
Residual risk: I did not run the full test suite or deploy. pnpm is not installed in this environment, so I used the checked-out local binaries under node_modules/.bin. |
|
Fixed the review findings in 9834c5f. Changes:
Verification:
Note: |
|
Review finding (material):
|
|
Fixed in Changes:
Verification:
Residual risk: no deploy was run. Full Vitest needed Node 24.12.0 because the available shared |
|
Severity: Low - public contract docs mismatch. docs/WORK_MAP_IMPORT.md:29 says a missing branch uses an |
|
Fixed the docs mismatch in Verification:
|
1 participant
Closes #18
What
POST /api/work-map/import/giraendpoint that mapsgira workspace status --json/workspace-queues/v1input intoagentree-work-map/v1.Why
Agentree needs a first Gira/GitHub read model so it can render a work map while Gira and GitHub remain canonical. This slice intentionally avoids DB writes, GitHub/Gira mutation, hosted runtime changes, credentials, and worker execution control.
Test plan
npm exec --yes pnpm@10.24.0 -- vitest run src/server/work-map/giraWorkspaceImport.test.ts src/server/routes/workMap.test.tspassesnpm exec --yes pnpm@10.24.0 -- testpassesnpm exec --yes pnpm@10.24.0 -- run buildpassesgit diff --checkpasses