Select repo, run goal, edit code in worktrees, and push to GitHub#14
Conversation
Make the desktop app able to take a goal end-to-end: pick a local repo, run the actor-critic loop so a CLI agent edits files in isolated worktrees, integrate the task branches, and push the result to GitHub (optionally as a PR). Engine: - workspace/git: add isGitRepo, remoteUrl, hasRemote, commitCount helpers. - config: add repoDir, branchPrefix, dryRun, push/PR options (pushToRemote, remote, pushBranch, openPr, prBase/prTitle/prBody/prDraft, pushOverrideSafety). - engine/publisher: push the integration branch + open a PR (gh CLI), behind a safety gate that refuses on conflicts / failed verification / needs-human unless explicitly overridden; clear errors for missing remote/auth. Injectable git + PR creator. - session/runGoal: name the integration branch <prefix>/<session>/<goal-slug>, run the publisher (gated by pushToRemote && !dryRun), record a publish event, and surface the result on SessionResult. - Thread the per-task worktree cwd into Actor.build/selfReview and Critic.review so a file-editing CLI runner actually edits inside the task's worktree (worktrees now genuinely isolate real changes). - server: accept repoDir on POST /api/runs, validate it is a git repo, and pass it through to the run. Desktop: - Start screen: repo folder picker + recent repos, validation, Codex/Kiro CLI presets that edit files, role auto-fill, branch prefix, dry run, and a push/PR panel with a tool-detection (installed/missing) readout. - Results: a Push & pull request card; Sessions: resume-in-same-repo. - Tauri: pick_directory, check_git_repo, which_commands commands + dialog plugin. Tests: add git, publisher, and an end-to-end worktree->integrate->push suite (168 tests pass). Co-authored-by: Inzimam Ul Haq <27832433+inhaq@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds end-to-end repository selection and git publishing to the loopwright engine and desktop app. New git utilities ( ChangesRepo Selection, cwd Threading, and Git Publishing
Sequence Diagram(s)sequenceDiagram
participant UI as desktop/src/main.ts
participant API as desktop/src/api.ts
participant TauriRepo as repo.rs
participant Server as server.ts
participant Session as session.ts
participant Publisher as publisher.ts
participant Git as git.ts / spawnGit
rect rgba(30, 120, 200, 0.5)
note over UI,TauriRepo: Repo selection (desktop only)
UI->>API: pickDirectory()
API->>TauriRepo: pick_directory
TauriRepo-->>API: path | null
API-->>UI: path | null
UI->>API: checkGitRepo(path)
API->>TauriRepo: check_git_repo
TauriRepo->>Git: git -C path rev-parse --is-inside-work-tree
Git-->>TauriRepo: bool
TauriRepo-->>API: bool
API-->>UI: bool
end
rect rgba(200, 80, 30, 0.5)
note over UI,Session: Run start + publishing
UI->>Server: POST /api/runs { repoDir, ... }
Server->>Git: isGitRepo(repoDir)
Git-->>Server: bool
Server->>Session: runGoal(goal, config, { repoDir, git, prCreator })
Session->>Session: integrate branch
Session->>Publisher: publish({ repoDir, branch, push, openPr, ... })
Publisher->>Git: hasRemote(repoDir, remote)
Git-->>Publisher: bool
Publisher->>Git: git push origin branch
Git-->>Publisher: pushed
Publisher->>Publisher: ghPrCreator (if openPr)
Publisher-->>Session: PublishResult
Session-->>Server: SessionResult { publish }
Server-->>UI: session started
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/workspace/git.ts (1)
95-96: ⚡ Quick winFix stale JSDoc on
commitCount.Line 95 describes a commit subject, but this function returns a numeric count. This will mislead callers and generated docs.
✏️ Suggested doc correction
-/** The short subject line of the latest commit on `ref` (default HEAD). */ +/** Number of commits reachable from `ref`, or from `baseRef..ref` when provided. */ export async function commitCount(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workspace/git.ts` around lines 95 - 96, The JSDoc comment on the `commitCount` function incorrectly describes it as returning "The short subject line of the latest commit on `ref`", but this function actually returns a numeric count. Update the JSDoc to accurately reflect that this function returns the number of commits on the specified ref (defaulting to HEAD), removing the misleading description about commit subject lines.test/git.test.ts (1)
16-23: ⚡ Quick winFail fast in tests by using the throwing git wrapper instead of raw
spawnGit.At Line 16 and Lines 44-56, command failures can be silently ignored because
spawnGitreturns{ exitCode }without throwing.✅ Suggested refactor
import { commitCount, + git, hasRemote, isGitRepo, remoteUrl, spawnGit, } from "../src/workspace/git.js"; @@ - const run = (args: string[]) => spawnGit(args, dir); + const run = (args: string[]) => git(spawnGit, args, dir); @@ - await spawnGit(["remote", "add", "origin", "https://example.com/x.git"], repo); + await git(spawnGit, ["remote", "add", "origin", "https://example.com/x.git"], repo); @@ - await spawnGit(["checkout", "-q", "-b", "feature"], repo); + await git(spawnGit, ["checkout", "-q", "-b", "feature"], repo); @@ - await spawnGit(["add", "-A"], repo); - await spawnGit(["commit", "-q", "-m", "feature work"], repo); + await git(spawnGit, ["add", "-A"], repo); + await git(spawnGit, ["commit", "-q", "-m", "feature work"], repo);Also applies to: 44-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/git.test.ts` around lines 16 - 23, Replace the direct use of spawnGit with a throwing git wrapper function that will immediately fail tests when git commands fail. In the test file where the run function calls spawnGit, update it to use a wrapper that throws an error on non-zero exit codes instead of silently continuing. This ensures that any git command failures are caught immediately rather than being silently ignored, making tests fail fast as intended.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src-tauri/src/repo.rs`:
- Around line 55-59: The is_on_path function currently only checks if a file
exists using is_file(), but does not verify that the file is executable, which
can lead to false positives on Unix systems where non-executable files on PATH
should not be reported as installed. Modify the is_on_path function to
additionally check for executable permissions on the file to ensure only actual
executable binaries are reported as installed. This check applies to the
explicit path handling logic (when the name contains separators) and also to the
implicit PATH search logic elsewhere in the file that performs similar file
existence checks.
In `@desktop/src/main.ts`:
- Around line 649-668: The validation check using repoValid is relying on stale
state that was set during prior change or blur events. When a user selects a new
repository path and immediately submits without triggering a change/blur
handler, the repoValid flag may not reflect the current directory state.
Revalidate the repoDir path at submit time (where the validation check occurs)
to determine if it is actually a valid git repository, rather than depending on
the potentially stale repoValid variable that was computed earlier.
In `@src/config.ts`:
- Around line 95-97: The branchPrefix field in src/config.ts at lines 95-97
(along with pushBranch and prBase fields at lines 106-111) currently accept any
string value without validating that they are git-safe ref names. This causes
validation failures to occur later during git/gh execution rather than at config
load time. Add validation or normalization logic to the schema definitions for
branchPrefix, pushBranch, and prBase to ensure they contain only valid git ref
characters and reject or normalize values with whitespace or invalid ref tokens.
This validation should happen in the loadConfig() function as the fail-fast
boundary, preventing bad configurations from burning a full session during later
git operations.
In `@src/engine/publisher.ts`:
- Around line 188-203: The PR creation logic invoked by the `creator` function
call in the try block does not check whether the abort signal has been raised
before attempting to create the PR. Add a check for the abort signal status
using opts.signal?.aborted before calling the creator function to short-circuit
PR creation if the operation has been cancelled. This same signal abort check
should also be applied at the second location mentioned (lines 222-255) to
ensure that cancellations consistently prevent PR creation and avoid reporting
incorrect terminal states when an abort occurs after the push completes.
- Around line 164-186: The remoteUrl read from the remote configuration is being
included in the PublishResult object at multiple locations without redacting
embedded credentials, causing HTTPS URLs with usernames and passwords to be
leaked when the result is persisted as a publish event. Create a helper function
that parses the URL and strips the username and password fields (returning the
original URL on parse failure), then apply this function to redact the url
before assigning it to the remoteUrl field in all three places where
PublishResult is returned with a remoteUrl value (the early return for no-remote
case, the error case in the catch block, and the final success case).
In `@src/server/server.ts`:
- Around line 365-378: The repoDir validation currently accepts any JSON type
and calls .trim() on the untyped body.repoDir value, which will throw if it's
not a string (e.g., a number), causing a 500 error instead of a 400.
Additionally, relative paths like "../repo" are accepted when only absolute
paths should be allowed. Add type validation to ensure body.repoDir is a string
before calling .trim(), then add path validation to reject non-absolute paths by
checking if the resolved repoDir is absolute. Normalize both the body and
runConfig repoDir values before the isGitRepo check, and return a 400 error with
a descriptive message if either the type validation or absolute path validation
fails.
In `@src/workspace/git.ts`:
- Around line 71-83: The `remoteUrl` function treats all non-zero exit codes and
exceptions the same way by returning undefined, which masks real git execution
failures behind the "remote missing" signal. Modify the function to distinguish
between the expected failure case (remote does not exist) and actual operational
errors. When the git command returns a non-zero exit code, check if the error
message indicates "no such remote" specifically—only return undefined in that
case. For the catch block, instead of silently returning undefined for all
exceptions, re-throw or handle exceptions separately since they indicate actual
git/binary execution failures rather than a missing remote configuration. This
ensures that downstream publish logic in publisher.ts can properly differentiate
between configuration issues and operational failures.
---
Nitpick comments:
In `@src/workspace/git.ts`:
- Around line 95-96: The JSDoc comment on the `commitCount` function incorrectly
describes it as returning "The short subject line of the latest commit on
`ref`", but this function actually returns a numeric count. Update the JSDoc to
accurately reflect that this function returns the number of commits on the
specified ref (defaulting to HEAD), removing the misleading description about
commit subject lines.
In `@test/git.test.ts`:
- Around line 16-23: Replace the direct use of spawnGit with a throwing git
wrapper function that will immediately fail tests when git commands fail. In the
test file where the run function calls spawnGit, update it to use a wrapper that
throws an error on non-zero exit codes instead of silently continuing. This
ensures that any git command failures are caught immediately rather than being
silently ignored, making tests fail fast as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d20d2bb6-4276-454c-b4f4-abb9319f469a
⛔ Files ignored due to path filters (1)
desktop/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
desktop/src-tauri/Cargo.tomldesktop/src-tauri/capabilities/default.jsondesktop/src-tauri/src/lib.rsdesktop/src-tauri/src/repo.rsdesktop/src/api.tsdesktop/src/main.tsdesktop/src/styles.csssrc/adapters/agents.tssrc/adapters/runnerRoles.tssrc/config.tssrc/engine/loop.tssrc/engine/publisher.tssrc/observability/events.tssrc/server/server.tssrc/session.tssrc/workspace/git.tstest/git.test.tstest/publisher.test.tstest/sessionPublish.test.ts
CI: the desktop "Cargo test" job failed because regenerating the whole Cargo.lock bumped brotli-decompressor and pulled two incompatible alloc-no-stdlib versions (2.0.4 + 3.0.0) into the tree, breaking brotli's compile. Restore the original lock and add only tauri-plugin-dialog (and its rfd / tauri-plugin-fs deps) as a minimal, purely-additive update so existing crate versions stay pinned. Review fixes: - publisher: redact embedded credentials from remote URLs before they are returned (and persisted as a publish event); stop PR creation when the run is cancelled (check signal before creating, and in ghPrCreator); handle remoteUrl now throwing on real git failures. - git: remoteUrl distinguishes "no such remote" (-> undefined) from real git execution failures (-> throws); hasRemote stays total. - config: validate branchPrefix / pushBranch / prBase as git-safe ref names at load time (fail-fast) instead of failing mid-run during branch/push/PR. - server: reject non-string repoDir (avoids a 500) and require an absolute path. - desktop: revalidate repoDir at submit time to avoid stale validation state. - repo.rs: which_commands now checks the executable bit on Unix, not just file existence, so non-executable PATH entries aren't reported as installed. Tests: add redactRemoteUrl, credential-redaction-on-failed-push, and config ref-validation cases (173 passing).
This pull request was created by @kiro-agent on behalf of @inhaq 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro Web
Makes Loopwright usable end-to-end: pick a local repo, run the actor-critic loop so a CLI agent edits files in isolated worktrees, integrate the task branches, and push the result to GitHub (optionally as a PR). Implements the dev-handover checklist.
Engine
workspace/git.ts):isGitRepo,remoteUrl,hasRemote,commitCount.repoDir,branchPrefix,dryRun, and push/PR options (pushToRemote,remote,pushBranch,openPr,prBase/prTitle/prBody/prDraft,pushOverrideSafety) — all opt-in, OFF by default.engine/publisher.ts): pushes the integration branch and opens a PR via theghCLI, behind a safety gate that refuses on merge conflicts / failed verification / needs-human unless explicitly overridden, with clear errors for a missing remote or auth. git + PR creator are injectable.session.ts): names the integration branch<prefix>/<session>/<goal-slug>, runs the publisher (gated bypushToRemote && !dryRun), records apublishevent, and exposes the result onSessionResult.cwdintoActor.build/selfReviewandCritic.review(optional, backward-compatible param) so a file-editing CLI runner edits inside the task's worktree rather than a shared dir.POST /api/runsacceptsrepoDir, validates it is a git repo (400 otherwise), and threads it into the run.Desktop
pick_directory,check_git_repo,which_commandscommands + the dialog plugin and capability.Product decision (checklist item 5)
Preferred path implemented: require file-editing CLI runners (Codex/Kiro) to change a selected repo. HTTP model runners only return a diff and do not edit files; the UI makes this explicit.
git applyfor actor diffs was intentionally not added.Testing
npm test: 168 passing (addedtest/git.test.ts,test/publisher.test.ts, and an end-to-endtest/sessionPublish.test.tsthat edits files in a worktree, integrates, and pushes to a bare remote — plus a dry-run-does-not-push case).npm run typecheck(engine) andnpm run build(desktop tsc + vite) both pass.Known limitations
Cargo.lockwas updated fortauri-plugin-dialogand the Rust parses viarustfmt, but the desktop CI job is what will fully validate the native build.ghCLI on the engine host (or aGITHUB_TOKEN); pushing uses the host's git credentials.Summary by CodeRabbit