fix(agent): supplied patch/summary sources no longer require a git repository - #2052
Conversation
…pository review, changelog, and recap operations with a caller-supplied patch, summary, or files source can now run from a non-git directory — over both the agent CLI and MCP — without failing with INVALID_REPOSITORY. Changes: - Add requiresRepository(operation, source) predicate in context.ts: returns true for commit-draft (always needs a repo for branch/history context) and kind: 'repository' sources; false for all other supplied-source operations. - Widen AgentOperationContext.git to SimpleGit | undefined and add requireRepository flag to createAgentOperationContext. When false, the context is created from the resolved directory without a git client. - Add warnings: string[] to ResolvedChangeContext. resolveChangeSource now emits a warning when headRevision was supplied but no git repo is available to verify it (stays provided-unverified), and throws INVALID_REPOSITORY for kind: 'repository' sources when no git is present. - handler.ts: compute needsRepo before resolving the repo; on INVALID_REPOSITORY for supplied sources fall back to resolveAgentDirectoryRoot + requireRepository: false; re-throw REPOSITORY_OUTSIDE_ROOT and CANCELLED; emit a descriptive INVALID_REPOSITORY message for commit-draft. - mcp/server.ts: same branching in registerGenerationTool; skip resolveEffectiveRepoRoot / assertClientAllowsRoot for supplied sources when no repository is declared; re-throw REPOSITORY_MISMATCH / REPOSITORY_OUTSIDE_ROOT. - generate.ts: guard generateAgentCommitDraft with !context.git check for a clear error message; thread resolved.warnings through all operation envelopes. - watch/handler.ts: add non-null assertion on context.git (watch always operates on a repository source). - Tests: new describe blocks for requiresRepository and createAgentOperationContext with requireRepository: false in context.test.ts; new handler.test.ts cases for review-from-non-git and commit-draft failure; new mcp/server.test.ts case for supplied source with no repo declared; fix ResolvedChangeContext mock values in generate/watch/additionalContext tests to include the new warnings field.
![gfargo-horizon-agent[bot]](https://avatars.githubusercontent.com/in/4083186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 Agent review (kiro · sonnet→opus) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The PR correctly makes repository resolution conditional across both transports for supplied sources, keeps commit-draft and repository sources bound, and adds solid unit/transport coverage. Two parity gaps remain between the CLI and MCP transports (commit-draft error message and CANCELLED handling), both mergeable judgment calls rather than functional bugs.
2 concerns · 1 nit — 3 inline on the diff
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — BLOCK
REVIEW: BLOCK
RESOLVES: no
The PR on GitHub (pull/2052) has only one commit, 104125e, identical to what was already reviewed — no new commit was pushed. All three previously flagged fixes exist only as uncommitted changes in the local worktree, so the live PR still has the generic commit-draft error message, the CANCELLED-swallowing bug, and the duplicated-repository-phrasing wording.
- ⛔ Revise fixes were never committed or pushed — git diff origin/main...HEAD and
gh pr diff 2052both show only commit 104125e, the same commit already reviewed. The fixes for the three previously flagged issues (generic MCP commit-draft error, unswallowed CANCELLED re-throw, duplicated 'repository' wording) exist only as uncommitted changes in src/mcp/server.ts, src/commands/agent/handler.ts, and src/operations/agent/context.ts in the worktree — they were never committed, so PR #2052 on GitHub still has all three original defects. Since this review must not commit/push, the PR remains unresolved until those local edits are actually committed and pushed.
There was a problem hiding this comment.
🔎 Agent re-review (kiro · sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
The revise correctly addresses both prior concerns: commit-draft now emits a context-specific INVALID_REPOSITORY message at both transports (handler.ts and server.ts), and CANCELLED is re-thrown before the no-repo fallback in both the CLI and MCP supplied-source paths. No new regressions, scope creep, or missing coverage introduced.
…1836-fix-agent-supplied-patch-summa # Conflicts: # src/commands/agent/handler.ts # src/mcp/server.ts
…or context.git in runRepoContext
There was a problem hiding this comment.
🔎 Agent re-review (kiro · sonnet, delta) — LGTM
REVIEW: LGTM
RESOLVES: full
Both parity gaps from the prior review are resolved: the commit-draft-specific INVALID_REPOSITORY message is now emitted consistently in both transports, and CANCELLED is re-thrown (not swallowed) in the no-repo fallback path of both handler.ts and server.ts. No new issues introduced.
What
When a calling agent supplies its own change context via a
patch,summary, orfilessource,review,changelog, andrecapoperations can now run from a directory that is not a git repository — over both the agent CLI and the MCP server. Previously, both transports resolved a git repository root unconditionally before inspecting the request source, causing an unhelpfulINVALID_REPOSITORYerror even when the repository was never needed.Why
Closes #1836
Plane: OSS-1219
How
operations/agent/context.ts: AddedrequiresRepository(operation, source)predicate (true forcommit-draftandkind: 'repository', false for all other supplied sources). WidenedAgentOperationContext.gittoSimpleGit | undefinedand addedrequireRepositoryflag tocreateAgentOperationContext. Addedwarnings: string[]toResolvedChangeContext;resolveChangeSourcenow emits a warning whenheadRevisionis supplied but no git repo is available (staysprovided-unverified) and throwsINVALID_REPOSITORYforkind: 'repository'sources with no git context.commands/agent/handler.ts: ComputeneedsRepofrom the predicate before resolving the root. For supplied sources, fall back toresolveAgentDirectoryRoot+requireRepository: falseonINVALID_REPOSITORY; re-throwREPOSITORY_OUTSIDE_ROOTandCANCELLED. Emit a descriptive error message forcommit-draftwith no repo.mcp/server.ts: Same conditional branching inregisterGenerationTool; skipresolveEffectiveRepoRoot/assertClientAllowsRootfor supplied sources when no repository is available; re-throwREPOSITORY_MISMATCH/REPOSITORY_OUTSIDE_ROOT.operations/agent/generate.ts: GuardgenerateAgentCommitDraftwith an explicit!context.gitcheck for a clear diagnostic; threadresolved.warningsthrough all four operation envelopes.commands/watch/handler.ts: Non-null assertion oncontext.git(watch always operates on a repository source).requiresRepositoryandcreateAgentOperationContext with requireRepository: falsedescribe blocks incontext.test.ts; new handler and MCP server test cases covering the no-repo fallback and commit-draft failure path; mockResolvedChangeContextvalues updated to include the newwarningsfield.Out of scope (noted in the issue):
condense-diffhas the same unconditional resolution shape for itspatchsource (dispatched separately viarunCondenseDiff) — left repository-bound as a follow-up.Testing
npx tsc --noEmit)🤖 Generated by the harbor agent loop. Reviewed by a human before merge.
Closes #1836