Swap nodegit for simple-git library#3
Merged
Merged
Conversation
The `simple-git` package has been added to the project dependencies. This package provides a simple interface for executing Git commands programmatically.
- Renamed `constants.ts`, `createCommit.ts`, `getChanges.ts`, `getStatus.ts`, `getSummaryText.ts`, and `parsePatches.ts` from the `utils/git` directory to the `nodegit` directory. - Updated import statements in the renamed files to reflect the new file structure. - Changed import paths for types in the renamed files.
Swap `nodegit` functionality with new `simple-git` helpers; `createCommit`, `getChanges`, `getDiff`, `getStatus`, and `getSummaryText`. - Modified `index.ts` to import fileChangeParser, getChanges, createCommit, and simpleGit. - Renamed `fileChangeParser.ts` to `index.ts`. - Modified `types.ts` to replace `Repository` with `SimpleGit`. - Modified `summarizeDiffs.ts` to update output callback.
Refactored `src/lib/parsers/noResult.ts` to use `simple-git` instead of `nodegit` for getting changes. Updated the function signature to accept `SimpleGit` instead of `Repository` and removed the `ignoreUnstaged` and `ignoreUntracked` options from `getChanges` call.
Remove NodeGit files from `src/lib/nodegit` and update `package.json` to remove the corresponding dependency.
Updated README.md to include an emoji between the `coco` name and description.
This was referenced May 2, 2026
This was referenced May 20, 2026
gfargo
added a commit
that referenced
this pull request
May 25, 2026
…input handling) (#1033) * fix(workstation): resolve 13 audit findings across streaming, persistence, input handling Audit of the post-0.54 workstation surfaced a mix of real bugs, latent risks, and hygiene cleanups. This lands all 13 findings as a single coordinated change so the test sweep stays green throughout. ## HIGH-priority bugs - **#1 Dead salvage branch in generateCommitDraft.** Phase 2 introduced a `salvaged` local that was declared, never assigned, and gated an unreachable branch. Every parse-failed stream paid for a second LLM call when the accumulated text was perfectly recoverable via `salvageCommitMessageFromText`. Now captures the streamed text out-of-band and runs the salvager on it before falling through to the non-streaming retry path. - **#2 Sidebar / diff-mode save effects key off stale repoRootRef.** Save effects' deps listed only the saved value, not `git`. After the #995 synchronous pop-restore, the parent's freshly restored tab got written into the submodule's cache during the brief window before the resolver re-settled. Both save effects now re-resolve the root inline with proper cancellation; deps include `git` so the effect re-fires on frame swap. - **#5 cancelAiCommitDraft Esc gated on activeView === 'compose'.** Phase 3 over-scoped the cancel binding. User starts AI draft on compose, chord-navs to history mid-stream, presses Esc to bail — Esc fell through to popView instead of cancelling. Dropped the gate; cancel works from any view while loading is true. Test updated to pin the new behaviour. ## MEDIUM defensive fixes - **#3 Loading stuck if workflow throws unexpectedly.** Added a catch block in `runAiCommitDraft` that clears loading + surfaces an error status. Latent today (workflow catches its own errors) but a future refactor letting one escape would have stranded the spinner. - **#4 Unmount race.** Three guards on `mountedRef.current` in `runAiCommitDraft`: skip per-chunk dispatch when unmounted, skip post-await dispatch when unmounted, skip the new catch-block dispatch too. Matches the pattern other async paths in app.ts use. - **#6 Soft-cancel messaging.** Status copy was "Esc to cancel" — but PR-body cancel is soft today (background LLM call still completes). Reworded to "Esc to skip prompt" so users aren't misled into thinking they're saving tokens. - **#8 Cancel classifier `.includes('aborted')`.** The substring heuristic would misclassify legitimate provider errors that happened to contain the word. Dropped; rely on `signal?.aborted` and `error.name === 'AbortError'` only. ## LOW-priority UX + hygiene - **#7 setDraft silently clobbers user-typed content.** User types, fires AI draft, draft lands → typing was lost with no undo. New `pendingAiDraft` state + `acceptPendingAiDraft` / `dismissPendingAiDraft` actions stage the AI draft when the fields have content. Input handler binds `R` to accept (swap into fields) and `Esc` to dismiss (keep typing, drop AI draft). 6 new reducer tests cover the routing + accept / dismiss flow. - **#9 Reducer purity — Date.now() in 3 cases.** `setChangelogReady`, `setChangelogText`, `markRecentCommits` now carry timestamps on the action payload; dispatchers call `Date.now()` at dispatch time. Reducer stays pure / testable / replay-friendly. - **#10 activeRepoRoot resolver race.** Investigated; the per-effect `cancelled` flag is the depth-equivalent safety mechanism and prevents the race the agent flagged (React fires cleanup before next effect body). Added a clarifying comment instead of code. - **#11 pendingPullRequestBodyDraft clear order.** Moved the flag clear from `finally` to the success path BEFORE openInputPrompt so no future-added await between them can race a cancel keystroke into a stale state. Belt-and-suspenders second clear in `finally` catches the error / cancel paths. - **#12 streamingPreview not cleared on setEditing(false).** Defensive reducer fix: clear the preview when editing toggles off AND no draft is in flight. Current input pipeline never triggers this, but the reducer is the source of truth. - **#13 onChunk handler errors silently swallowed forever.** Added a consecutive-failure counter; the stream now bails with a thrown LangChainExecutionError after 5 strikes. A flaky-but-recoverable handler resets the counter on each success so isolated blips don't trigger spurious bails. ## Validation - `npx tsc --noEmit` clean - `npm run lint` clean - 1760/1760 broader sweep passes (`(workstation|commands/log|commit|langchain|git/)`) - 14 new tests covering the new behaviour: - 6 for `pendingAiDraft` accept / dismiss / clobber-routing - 2 for `streamingPreview` defensive clear on setEditing - 3 for `onChunk` failure counter (transient / bail / streak reset) - 1 updated for #5 (Esc cancel from non-compose view) - 1 updated for #9 (markRecentCommits takes timestamp on action) - 1 updated for compose-cancel precedence (drop activeView assertion) The setDraft clobber fix (#7) is the biggest behaviour change; users mid-typing who fire the AI draft will now see a confirmation prompt instead of silent replacement. Documented in the new test as the intended behaviour. * docs(workstation): document async LLM cancel + streaming + pendingAiDraft patterns Adds a new "Async LLM calls + cancellation (#881)" section to the workstation README so future contributors don't have to reverse-engineer the conventions from app.ts. Covers: - When to use `executeChainStreaming` vs `executeChain` - The `AbortController`-per-call ref pattern for cancel - Why `LangChainCancelledError` is its own class - Streaming preview as preview-only (validated output is unchanged) - Why stdout commands stay non-streaming - The non-view-gated cancel keystroke convention (audit finding #5) - The `pendingAiDraft` confirmation flow for the clobber guard (audit finding #7) Also updates the reducer-purity bullet to mention timestamps as a side effect that belongs in workflows / dispatchers, not the reducer (audit finding #9).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR mimics the previous functionality provided with
nodegitwithsimple-git. Drastically improves install times and general compatibility.Closes #2
This could also potentially resolve the issues we were seeing in #1