mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 18:00:54 -10:00
* fix(tui): plan panel reflects live progress + prompt cadence
The plan panel could read as stuck mid-task even while work continued. Three
fixes:
- Header showed steps[0] forever, so the title never advanced as the plan
progressed; show the current step (in_progress, else first incomplete).
- updateFromItems matched steps by content only, so an in-place reword reset a
step's elapsed timer to zero; add positional carry-over when the step count is
unchanged.
- Prompt: tell the model to mark each step completed + the next in_progress
before starting it, so the plan tracks real progress instead of jumping at the
end.
Regression tests included.
* feat(tui): click a plan step to see what was built for it
Each plan step now records the file mutations (write_file/edit_file/apply_patch)
made while it was in_progress, captured from tool-result rows. Clicking a step
row in the context sidebar drops a transcript card listing those changes — the
implementation for that step. Step->line mapping mirrors sidebarAgentSelectables'
offset accounting (one line per step); capture is keyed to the active step and
cleared per run. Read-only and additive. Tests cover the offset math + capture.
* feat(tui): plan-step detail shows diffs + commands
Extend the clickable plan-step detail: capture bash/exec_command runs in addition
to file mutations, store each tool's full output (the diff for edit/apply_patch,
stdout/stderr for commands), and render the card grouped into Changes and
Commands with a short truncated excerpt of each item's diff/output.
* fix(tui): plan-step detail toggles instead of stacking duplicates
Re-clicking a plan step now hides its card (a stable transcript id + drop-by-id),
and clicking a different step replaces it, so at most one detail card is shown.
Previously each click appended a fresh card and they piled up.
* feat(tui): elaborate plan-step detail by status (what we did / will do)
* feat(tui): plan-step card leads with a prose explanation (agent's own narration)
* feat(tui): write plan-step explanations with the model on click
Clicking a PLAN step now shows a fresh, plain-English write-up of that
step, generated on demand by the active model:
- past tense ('what we did') for completed/failed steps, future tense
('what we'll do') for pending/in_progress steps
- immediate feedback: the card drops in showing 'Writing explanation…'
and updates in place (by stable row id) when the text returns
- cached per step (content+status) so re-clicking is instant with no
second model call; re-clicking still toggles the card closed
- the prompt is built from already-captured local data (step text,
status, notes, plus a compact digest of file edits, commands, and the
agent's narration) and asks for short, non-technical prose
- reuses the TUI's existing provider via a one-shot StreamCompletion;
no new client, no hardcoded provider. Falls back to the local summary
when no provider is configured or the request fails.
* fix(tui): minimal plan-step card styling (dim border, status-tinted title)
* feat(tui): auto-hide context sidebar when it has no agents or plan
* feat(tui): widen chat reading column to fill wide terminals with breathing room
* fix(tui): toggle context sidebar silently (no transcript notice)
* fix(tui): mark plan complete when a finished task forgot the final update_plan
* fix(tui): remove the lime streaming-text glow (render responses in static ink)
* fix(tui): remove the shaded panel band behind tool-result cards
* fix(tui): Ctrl+B hides the plan entirely (no pinned-panel fallback)
* style(tui): gofmt-align model struct fields
* feat: narrate the build as a clean story (hide plumbing cards, live plan progress, narration prompt, write_file line counts)
* feat(tui): render agent narration bright (ink) so the build reads as a story
* feat(agent): demand an elaborate, structured closing summary (not a single line)
* feat(tui): mark agent narration with a leading bullet (selection-aligned)
* feat: post-turn recap line (※ recap:) with /config recaps on|off toggle
* feat(tui): show 'still generating…' during quiet stretches + count tool-call streaming
* feat(tui): ACTIVITY panel under PLAN + push the model to step the plan per file
* fix(tui): show the quiet 'still generating' hint only when the sidebar is hidden
* feat(tui): code preview in write/edit/apply cards (card-only, no model token cost)
* fix(tui): address CodeRabbit review — stale plan timestamps, stale explanation, Ctrl+B persistence, UTF-8 truncation
* fix: address CodeRabbit re-review round 2 (7 actionable + 2 nitpicks)
* chore: drop accidentally-committed WIP internal/cli/dryrun_test.go
It is an unrelated, pre-existing dry-run test that does not compile (references a
non-existent agent.Options.DryRun field, an unused import, and out-of-scope
sandbox symbols). It was swept into the prior commit by 'git add -A' and is not
part of this PR's scope.
286 lines
9.8 KiB
Go
286 lines
9.8 KiB
Go
package tools
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"os"
|
|
"os/exec"
|
|
"path/filepath"
|
|
"strconv"
|
|
"strings"
|
|
)
|
|
|
|
type applyPatchTool struct {
|
|
baseTool
|
|
workspaceRoot string
|
|
scope PathScope
|
|
}
|
|
|
|
func NewApplyPatchTool(workspaceRoot string) Tool {
|
|
return NewScopedApplyPatchTool(workspaceRoot, nil)
|
|
}
|
|
|
|
func NewScopedApplyPatchTool(workspaceRoot string, scope PathScope) Tool {
|
|
return applyPatchTool{
|
|
baseTool: baseTool{
|
|
name: "apply_patch",
|
|
description: "Apply a unified diff patch inside the workspace or an explicitly granted extra write root.",
|
|
parameters: Schema{
|
|
Type: "object",
|
|
Properties: map[string]PropertySchema{
|
|
"patch": {Type: "string", Description: "Unified diff patch to apply."},
|
|
"cwd": {Type: "string", Description: "Directory where the patch should be applied. Relative paths stay in the workspace; use an absolute path to target a granted extra write root. Defaults to workspace root.", Default: "."},
|
|
},
|
|
Required: []string{"patch"},
|
|
AdditionalProperties: false,
|
|
},
|
|
safety: promptSafety(SideEffectWrite, "Applies patch hunks that can create, edit, or delete files."),
|
|
},
|
|
workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
|
|
scope: scope,
|
|
}
|
|
}
|
|
|
|
func (tool applyPatchTool) Run(ctx context.Context, args map[string]any) Result {
|
|
return tool.RunWithOptions(ctx, args, RunOptions{})
|
|
}
|
|
|
|
func (tool applyPatchTool) RunWithOptions(ctx context.Context, args map[string]any, options RunOptions) Result {
|
|
patch, err := aliasedStringArg(args, []string{"patch", "diff"}, "", true, false)
|
|
if err != nil {
|
|
return errorResult("Error: Invalid arguments for apply_patch: " + err.Error())
|
|
}
|
|
cwd, err := stringArg(args, "cwd", ".", false)
|
|
if err != nil {
|
|
return errorResult("Error: Invalid arguments for apply_patch: " + err.Error())
|
|
}
|
|
|
|
applyRoot, relativeRoot, err := resolveScopedPath(tool.workspaceRoot, tool.scope, cwd)
|
|
if err != nil {
|
|
return errorResult("Error applying patch: " + err.Error())
|
|
}
|
|
if err := validatePatchPaths(applyRoot, patch); err != nil {
|
|
return errorResult("Error applying patch: " + err.Error())
|
|
}
|
|
|
|
tempFile, err := os.CreateTemp("", "zero-patch-*.patch")
|
|
if err != nil {
|
|
return errorResult("Error applying patch: " + err.Error())
|
|
}
|
|
patchPath := tempFile.Name()
|
|
defer func() {
|
|
_ = os.Remove(patchPath)
|
|
}()
|
|
if _, err := tempFile.WriteString(patch); err != nil {
|
|
_ = tempFile.Close()
|
|
return errorResult("Error applying patch: " + err.Error())
|
|
}
|
|
if err := tempFile.Close(); err != nil {
|
|
return errorResult("Error applying patch: " + err.Error())
|
|
}
|
|
|
|
if err := recheckPatchWriteTargets(applyRoot, patch); err != nil {
|
|
return errorResult("Error applying patch: " + err.Error())
|
|
}
|
|
|
|
command := exec.CommandContext(ctx, "git", "apply", "--whitespace=nowarn", patchPath)
|
|
command.Dir = applyRoot
|
|
output, err := command.CombinedOutput()
|
|
if err != nil {
|
|
message := strings.TrimSpace(string(output))
|
|
if message == "" {
|
|
message = err.Error()
|
|
}
|
|
return errorResult("Error applying patch: " + message)
|
|
}
|
|
|
|
summary := "Patch applied successfully."
|
|
if relativeRoot != "." {
|
|
summary = "Patch applied successfully in " + relativeRoot + "."
|
|
}
|
|
result := okResult(summary)
|
|
result.ChangedFiles = changedFilesFromPatch(relativeRoot, patch)
|
|
result.Display = Display{Summary: summary, Kind: "diff", Preview: capPreviewDiff(patch)}
|
|
// git apply already rejects a patch whose context drifted, so it has its own
|
|
// staleness guard. Drop any tracked baseline for the files it rewrote so a
|
|
// subsequent edit_file/write_file re-reads instead of false-flagging the
|
|
// patch's own change as an external modification.
|
|
for _, changed := range result.ChangedFiles {
|
|
if absolute, _, rerr := resolveScopedPath(tool.workspaceRoot, tool.scope, changed); rerr == nil {
|
|
options.FileTracker.Forget(absolute)
|
|
}
|
|
}
|
|
return result
|
|
}
|
|
|
|
// changedFilesFromPatch extracts the unique, WORKSPACE-relative paths a patch
|
|
// touches, reusing the same per-line parser used for validation. Patch paths are
|
|
// relative to the apply cwd, so relativeRoot (the workspace-relative cwd, e.g.
|
|
// "sub/dir", or "." for the workspace root) is prefixed so callers get true
|
|
// workspace-relative paths regardless of cwd. When the apply cwd resolves to an
|
|
// extra write root, resolveScopedPath returns the absolute path as relativeRoot;
|
|
// in that case the entries in the returned slice are absolute paths, since
|
|
// workspace-relative would be ambiguous there.
|
|
func changedFilesFromPatch(relativeRoot string, patch string) []string {
|
|
seen := map[string]bool{}
|
|
var paths []string
|
|
for _, path := range patchHeaderPaths(patch) {
|
|
if path == "" || path == "/dev/null" {
|
|
continue
|
|
}
|
|
workspacePath := path
|
|
if relativeRoot != "" && relativeRoot != "." {
|
|
workspacePath = filepath.ToSlash(filepath.Join(relativeRoot, path))
|
|
}
|
|
if seen[workspacePath] {
|
|
continue
|
|
}
|
|
seen[workspacePath] = true
|
|
paths = append(paths, workspacePath)
|
|
}
|
|
return paths
|
|
}
|
|
|
|
func validatePatchPaths(root string, patch string) error {
|
|
for _, path := range patchHeaderPaths(patch) {
|
|
if path == "" || path == "/dev/null" {
|
|
continue
|
|
}
|
|
if filepath.IsAbs(path) || path == ".." || strings.HasPrefix(path, "../") {
|
|
return fmt.Errorf("patch path %q must stay inside the workspace", path)
|
|
}
|
|
if _, _, err := resolveWorkspaceTargetPath(root, path); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func recheckPatchWriteTargets(root string, patch string) error {
|
|
for _, path := range patchHeaderPaths(patch) {
|
|
if path == "" || path == "/dev/null" {
|
|
continue
|
|
}
|
|
if err := recheckWorkspaceWriteTarget(root, path); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// patchHeaderPaths returns the file paths declared in a unified diff's headers
|
|
// (`diff --git` and `---`/`+++` lines). It tracks hunk state by counting body
|
|
// lines from each `@@ -a,b +c,d @@` header, so a removed/added content line that
|
|
// merely begins with "--- "/"+++ " (e.g. the removal of a markdown line "-- x")
|
|
// is NOT mistaken for a file header. This mirrors how `git apply` parses hunks,
|
|
// so a line this skips is content git won't write to either — no security gap.
|
|
func patchHeaderPaths(patch string) []string {
|
|
var paths []string
|
|
oldRemaining, newRemaining := 0, 0
|
|
inHunk := false
|
|
for _, line := range strings.Split(strings.ReplaceAll(patch, "\r\n", "\n"), "\n") {
|
|
if inHunk && (oldRemaining > 0 || newRemaining > 0) {
|
|
switch {
|
|
case strings.HasPrefix(line, "-"):
|
|
oldRemaining--
|
|
case strings.HasPrefix(line, "+"):
|
|
newRemaining--
|
|
case strings.HasPrefix(line, "\\"):
|
|
// "\ No newline at end of file" — not a content line.
|
|
default: // context line (" ...") or a blank context line
|
|
oldRemaining--
|
|
newRemaining--
|
|
}
|
|
continue
|
|
}
|
|
inHunk = false
|
|
switch {
|
|
case strings.HasPrefix(line, "diff --git "):
|
|
fields := strings.Fields(line)
|
|
if len(fields) >= 4 {
|
|
paths = append(paths, stripPatchPrefix(fields[2]), stripPatchPrefix(fields[3]))
|
|
}
|
|
case strings.HasPrefix(line, "@@"):
|
|
oldRemaining, newRemaining = parseHunkCounts(line)
|
|
inHunk = oldRemaining > 0 || newRemaining > 0
|
|
case strings.HasPrefix(line, "--- "), strings.HasPrefix(line, "+++ "):
|
|
// Take everything after the fixed 4-char prefix instead of splitting on
|
|
// spaces, so a path that contains spaces survives. Drop a trailing
|
|
// tab-delimited timestamp (unified-diff convention) and unquote a
|
|
// C-quoted git path (git quotes names with spaces/specials) (L18).
|
|
rest := line[len("--- "):] // "--- " and "+++ " are both 4 bytes
|
|
if tab := strings.IndexByte(rest, '\t'); tab >= 0 {
|
|
rest = rest[:tab]
|
|
}
|
|
if p := strings.TrimSpace(unquoteGitPath(rest)); p != "" && p != "/dev/null" {
|
|
paths = append(paths, stripPatchPrefix(p))
|
|
}
|
|
}
|
|
}
|
|
return paths
|
|
}
|
|
|
|
// parseHunkCounts reads the old/new line counts from a "@@ -a,b +c,d @@" header.
|
|
// A missing count (e.g. "@@ -a +c @@") means 1 per unified-diff convention.
|
|
//
|
|
// Only the range section BETWEEN the opening and closing "@@" is parsed. A hunk
|
|
// header may carry a free-form section heading after the closing "@@" (e.g.
|
|
// "@@ -1,1 +1,1 @@ func foo()"), and that text can itself contain "+"/"-"
|
|
// tokens. Scanning the whole line would let a crafted heading like
|
|
// "@@ -1,1 +1,1 @@ +1,999999" overwrite the real count, keep the parser stuck in
|
|
// hunk mode, and swallow later "--- "/"+++ " file headers so they escape
|
|
// validatePatchPaths / recheckPatchWriteTargets — a workspace-confinement bypass.
|
|
func parseHunkCounts(line string) (int, int) {
|
|
_, rest, ok := strings.Cut(line, "@@")
|
|
if !ok {
|
|
return 0, 0
|
|
}
|
|
rangeSection := rest
|
|
if before, _, ok := strings.Cut(rest, "@@"); ok {
|
|
rangeSection = before // drop the section heading after the closing "@@"
|
|
}
|
|
old, next := 0, 0
|
|
for _, field := range strings.Fields(rangeSection) {
|
|
switch {
|
|
case strings.HasPrefix(field, "-"):
|
|
old = hunkCount(field[1:])
|
|
case strings.HasPrefix(field, "+"):
|
|
next = hunkCount(field[1:])
|
|
}
|
|
}
|
|
return old, next
|
|
}
|
|
|
|
func hunkCount(spec string) int {
|
|
if _, count, ok := strings.Cut(spec, ","); ok {
|
|
if n, err := strconv.Atoi(count); err == nil {
|
|
return n
|
|
}
|
|
return 0
|
|
}
|
|
return 1
|
|
}
|
|
|
|
// unquoteGitPath undoes git's C-style quoting of a diff path. Git wraps a path in
|
|
// double quotes and backslash-escapes special bytes (spaces, tabs, high bytes as
|
|
// octal) when it contains anything unusual; an unquoted path is returned as-is.
|
|
func unquoteGitPath(s string) string {
|
|
s = strings.TrimSpace(s)
|
|
if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' {
|
|
if unquoted, err := strconv.Unquote(s); err == nil {
|
|
return unquoted
|
|
}
|
|
}
|
|
return s
|
|
}
|
|
|
|
func stripPatchPrefix(path string) string {
|
|
path = strings.TrimSpace(path)
|
|
// A unified-diff path carries exactly one of the a/ or b/ prefixes; strip a
|
|
// single one so a real directory literally named "a" or "b" is preserved.
|
|
if strings.HasPrefix(path, "a/") || strings.HasPrefix(path, "b/") {
|
|
path = path[2:]
|
|
}
|
|
return filepath.ToSlash(path)
|
|
}
|