mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
* stage 01: web_search tool Adds a web_search tool alongside web_fetch: discovers URLs via a pluggable searchBackend (env-configured generic JSON backend; swappable for Exa/You/ Tavily/Brave) and formats ranked results as a numbered list. Registered in CoreNetworkTools(). Deviation from the stage spec: the spec said readOnlySafety, but the repo enforces (TestCoreNetworkToolsExposePromptMetadata) that every core network tool is prompt-gated network egress — so web_search matches web_fetch's posture instead of being auto-allowed. Necessary fallout from adding the tool: knownToolNames (+web_search), the network-tools metadata test (now covers both), and a stale 'unknown tool' fixture that had used web_search. * stage 02: LSP client foundation New internal/lsp package: a direct (self-spawned, stdio) Language Server Protocol client so unattended runs see real compiler diagnostics without an open editor. - client.go: JSON-RPC 2.0 with LSP Content-Length framing; concurrent Call/Notify, id-matched responses, server->client request auto-reply, notification handler, context cancellation, read-loop teardown. - server.go: process lifecycle (spawn via os/exec, initialize/initialized handshake, graceful shutdown+exit with kill fallback). Handshake factored out (performInitialize) for pipe-based testing. - registry.go: extension -> server command (.go/.ts/.tsx/.js/.jsx/.py/.rs), languageId map, PATH availability via exec.LookPath. - types.go: minimal LSP types + PathToURI/URIToPath (Windows-drive aware). Stdlib-only; tested over in-memory pipes with a stub server (no real gopls), including -race for the concurrent id router. * stage 03: LSP document sync + diagnostics Makes the LSP client produce actionable signal so ZERO can ask 'did my edit compile?' headlessly. - documents.go: per-server session with document lifecycle (didOpen/didChange full-sync/didClose), a URI->diagnostics store fed by publishDiagnostics, and WaitForDiagnostics with a publish-baseline + 300ms quiet debounce (servers never signal 'analysis complete'). - diagnostics.go: FormatDiagnostics (path:line:col: severity: message, 1-based), HasErrors, FilterBySeverity, severity labels. - manager.go: Manager — one lazily-started, reused server per language (keyed by binary), routes a file to the right server, Check(ctx,path,text) -> settled diagnostics, graceful Shutdown; unknown extension -> (nil,nil), not an error. Concurrency-safe; tested under -race over in-memory pipes with a stub server (no real gopls). FormatDiagnostics takes the path explicitly since a Diagnostic carries only a range, not its document. * wedge: address review on #175 (LSP robustness + web_search provider) - lsp/client.go: Close() now disables the client — Call/Notify reject after close instead of leaking a pending entry / writing to a dead conn. - lsp/documents.go: serialize same-document syncs through the Notify write via a per-URI lock and commit version/open only on success; drop delayed publishDiagnostics for a superseded (older positive) version; WaitForDiagnostics now reports whether a fresh publish arrived. - lsp/manager.go: Check returns (nil,nil) when no publish newer than baseline arrives (don't return a stale prior result for new text); Shutdown joins and returns per-server shutdown errors instead of always reporting success. - lsp/diagnostics.go: FormatDiagnostics collapses embedded whitespace so each diagnostic stays one physical line. - tools/web_search.go: forward ZERO_WEBSEARCH_PROVIDER in the request so the config knob isn't inert. Adds tests: close-disables-client, stale-version drop, multi-line message collapse, Shutdown error propagation, httptest backend (provider+parse). * wedge: degrade gracefully when an LSP server binary is missing (#175 review) Manager.Check translated a missing-binary start error (exec.ErrNotFound) into (nil,nil) so a configured extension on a machine without gopls/ pyright/rust-analyzer degrades exactly like an unsupported extension — LSP diagnostics are an opportunistic layer, not a hard dependency. Other start failures still surface. Adds a test injecting an exec.ErrNotFound starter. * wedge stage 04: self-correct loop (#176) * stage 04: self-correct loop After a successful mutating tool call, verify the changed files (LSP diagnostics + project tests) and feed failures back to the model to fix — bounded and autonomy-gated, so an unattended run fixes its own mistakes without looping forever. - internal/agent/selfcorrect.go: SelfCorrector with injectable diagnosticsChecker (lsp.Manager adapter) and projectVerifier (verify.DetectPlan+Run); AfterEdit produces a redacted, model-facing corrective message; per-run attempt ceiling; low autonomy reports without auto-attempting; medium/high auto-correct. - internal/agent/loop.go: invoke AfterEdit after a successful mutating tool call (ChangedFiles non-empty), appending feedback to messages. nil SelfCorrect (the default) is a no-op — read-only tools never trigger it. - internal/agent/types.go: add Options.SelfCorrect *SelfCorrector (the explicit options-struct change). Single-pass verify per call (the agent loop + model edits provide the multi- attempt cycle, not selfverify's same-command re-runs). Tested with fakes (no gopls/shell): fail-then-pass, max-attempts abort, read-only skip, low-autonomy report-only, LSP-only failure, secret redaction; -race clean. CLI activation (a --self-correct flag building the SelfCorrector in exec.go) is intentionally left out to honor this stage's file scope — the engine is exported and ready to wire. * wedge: propagate file-read error in self-correct LSP checker (#176 review) CodeRabbit/golangci-lint (nilerr) flagged lspDiagnosticsChecker.Check returning (nil, nil) after os.ReadFile fails. Propagate the error instead; the sole caller (inspect) already skips files whose Check errors, so a deleted/unreadable file still degrades quietly while future callers can decide. Adds a regression test. * wedge: surface non-degradable verify errors in self-correct (#175 review) SelfCorrector.inspect silently continued on every checker.Check error and ignored verifier errors, so a real LSP startup/sync failure, an unreadable changed file, or a failed plan detection produced report.Failed=false -> OutcomePassed with no signal. Manager.Check already degrades missing servers to (nil,nil) and DetectPlan returns an empty plan (not an error) when nothing is detected, so a non-nil error from either is a genuine failure. Record such errors in CorrectionReport.InspectErrors, fail the pass, and surface them in the model feedback. Adds regression tests for both paths. * wedge: drop third-party search-provider names from web_search comments Keep code/comments provider-neutral: describe the backend interface as 'any hosted search API' rather than naming specific services. * wedge: defer self-correct feedback until after the tool-call batch (#175 review) Self-correct feedback was appended as a user message inside the per-tool-call loop, so a multi-tool-call turn whose first (mutating) call failed verification produced: assistant tool_use batch -> tool_result 1 -> user feedback -> tool_result 2. A user message interleaved between tool_results breaks strict provider replay and later session replay/compaction. Accumulate feedback during the batch and append it once after the loop (same after-batch pattern already used for mid-run model escalation), so every advertised tool call keeps its tool_result contiguous. Adds a two-tool-call regression test asserting the feedback lands after both tool_results. * wedge: batch self-correct AfterEdit once per turn over union of changes (#175 review) Deferring only the feedback emission still ran SelfCorrect.AfterEdit once per mutating tool call, which (a) consumes the per-run attempt budget multiple times in a single turn and (b) verifies an intermediate edit that a later call in the same batch already superseded — AfterEdit is documented as one per-run instance holding attempt state, so this is a correctness issue, not just transcript shape. Aggregate every mutating tool's ChangedFiles during the batch and call AfterEdit once afterward over the deduped union, appending its feedback once (still after all tool_results, preserving contiguity). Adds dedupeStrings and a regression test asserting two writes in one turn consume exactly one attempt and emit one feedback message.
261 lines
9.2 KiB
Go
261 lines
9.2 KiB
Go
package agent
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"github.com/Gitlawb/zero/internal/lsp"
|
|
"github.com/Gitlawb/zero/internal/redaction"
|
|
"github.com/Gitlawb/zero/internal/verify"
|
|
)
|
|
|
|
// defaultSelfCorrectMaxAttempts bounds how many corrective rounds a single run
|
|
// will drive before giving up, so an unattended run can never loop forever.
|
|
const defaultSelfCorrectMaxAttempts = 3
|
|
|
|
// diagnosticsChecker returns the diagnostics for a single (absolute) file path.
|
|
// *lsp.Manager is adapted to this via lspDiagnosticsChecker; tests inject a fake.
|
|
type diagnosticsChecker interface {
|
|
Check(ctx context.Context, path string) ([]lsp.Diagnostic, error)
|
|
}
|
|
|
|
// projectVerifier runs the workspace's verification plan once and reports it.
|
|
// The default implementation is DetectPlan + verify.Run; tests inject a fake.
|
|
type projectVerifier interface {
|
|
Verify(ctx context.Context) (verify.Report, error)
|
|
}
|
|
|
|
// SelfCorrectConfig controls the post-edit verify-and-correct cycle.
|
|
type SelfCorrectConfig struct {
|
|
Enabled bool
|
|
MaxAttempts int
|
|
IncludeTests bool
|
|
IncludeLSP bool
|
|
Autonomy string // low | medium | high
|
|
}
|
|
|
|
// Outcome reports what a self-correct round decided, for observability/tests.
|
|
type Outcome string
|
|
|
|
const (
|
|
OutcomeDisabled Outcome = "disabled" // self-correct off, or nothing to check
|
|
OutcomePassed Outcome = "passed" // verification passed; nothing to do
|
|
OutcomeCorrecting Outcome = "correcting" // failed; feedback issued for the model to fix
|
|
OutcomeReported Outcome = "reported" // failed; low autonomy, reported but no auto-fix
|
|
OutcomeAborted Outcome = "aborted" // failed but the correction budget is exhausted
|
|
)
|
|
|
|
// CorrectionReport is the unified result of a post-edit verification pass.
|
|
type CorrectionReport struct {
|
|
LSPDiagnostics map[string][]lsp.Diagnostic // path -> error diagnostics
|
|
VerifyReport verify.Report
|
|
// InspectErrors holds non-degradable verification failures (an LSP
|
|
// startup/sync error, an unreadable changed file, or plan detection failing).
|
|
// These are distinct from diagnostics and must fail the pass rather than be
|
|
// silently swallowed into a false "passed".
|
|
InspectErrors []string
|
|
Failed bool
|
|
}
|
|
|
|
// SelfCorrector runs verification after a mutating edit and, when it fails,
|
|
// synthesizes corrective feedback for the model — bounded by a per-run attempt
|
|
// budget and gated by the autonomy ceiling. One instance per run (it holds the
|
|
// attempt counter). A nil *SelfCorrector is a no-op.
|
|
type SelfCorrector struct {
|
|
workspaceRoot string
|
|
checker diagnosticsChecker
|
|
verifier projectVerifier
|
|
cfg SelfCorrectConfig
|
|
attempts int
|
|
}
|
|
|
|
// NewSelfCorrector builds a self-corrector. checker/verifier may be nil to
|
|
// disable that half of verification (e.g. no LSP server, or tests off).
|
|
func NewSelfCorrector(workspaceRoot string, checker diagnosticsChecker, verifier projectVerifier, cfg SelfCorrectConfig) *SelfCorrector {
|
|
if cfg.MaxAttempts <= 0 {
|
|
cfg.MaxAttempts = defaultSelfCorrectMaxAttempts
|
|
}
|
|
return &SelfCorrector{workspaceRoot: workspaceRoot, checker: checker, verifier: verifier, cfg: cfg}
|
|
}
|
|
|
|
// AfterEdit runs a verification pass over the files a mutating tool changed and
|
|
// returns corrective feedback (empty when nothing actionable) plus the outcome.
|
|
// The loop appends any non-empty feedback to the conversation so the model can
|
|
// fix the problem on its next turn.
|
|
func (sc *SelfCorrector) AfterEdit(ctx context.Context, changedFiles []string) (string, Outcome) {
|
|
if sc == nil || !sc.cfg.Enabled || len(changedFiles) == 0 {
|
|
return "", OutcomeDisabled
|
|
}
|
|
|
|
report := sc.inspect(ctx, changedFiles)
|
|
if !report.Failed {
|
|
return "", OutcomePassed
|
|
}
|
|
|
|
// Low autonomy reports the failure but never starts an auto-fix round.
|
|
if !autonomyAllowsAutoCorrect(sc.cfg.Autonomy) {
|
|
return sc.feedback(report, false), OutcomeReported
|
|
}
|
|
|
|
if sc.attempts >= sc.cfg.MaxAttempts {
|
|
return abortedSelfCorrectNotice(sc.cfg.MaxAttempts), OutcomeAborted
|
|
}
|
|
sc.attempts++
|
|
return sc.feedback(report, true), OutcomeCorrecting
|
|
}
|
|
|
|
// inspect collects LSP error diagnostics for the changed files and runs the
|
|
// project verification plan, marking Failed on any LSP error or verify failure.
|
|
func (sc *SelfCorrector) inspect(ctx context.Context, changedFiles []string) CorrectionReport {
|
|
report := CorrectionReport{LSPDiagnostics: map[string][]lsp.Diagnostic{}}
|
|
|
|
if sc.cfg.IncludeLSP && sc.checker != nil {
|
|
for _, file := range changedFiles {
|
|
abs := sc.absPath(file)
|
|
diags, err := sc.checker.Check(ctx, abs)
|
|
if err != nil {
|
|
// Manager.Check degrades missing servers / unsupported files to
|
|
// (nil, nil), so a non-nil error here is a real failure (LSP
|
|
// startup/sync, or an unreadable changed file). Surface it instead
|
|
// of silently passing.
|
|
report.InspectErrors = append(report.InspectErrors, fmt.Sprintf("%s: %v", file, err))
|
|
report.Failed = true
|
|
continue
|
|
}
|
|
if errs := lsp.FilterBySeverity(diags, lsp.SeverityError); len(errs) > 0 {
|
|
report.LSPDiagnostics[file] = errs
|
|
report.Failed = true
|
|
}
|
|
}
|
|
}
|
|
|
|
if sc.cfg.IncludeTests && sc.verifier != nil {
|
|
vr, err := sc.verifier.Verify(ctx)
|
|
if err != nil {
|
|
// "No plan / no tests" is not an error (DetectPlan returns an empty
|
|
// plan), so a non-nil error means verification could not run at all —
|
|
// don't let that masquerade as a pass.
|
|
report.InspectErrors = append(report.InspectErrors, fmt.Sprintf("verification could not run: %v", err))
|
|
report.Failed = true
|
|
} else {
|
|
report.VerifyReport = vr
|
|
if !vr.OK {
|
|
report.Failed = true
|
|
}
|
|
}
|
|
}
|
|
return report
|
|
}
|
|
|
|
// feedback synthesizes the model-facing message. corrective=false is the
|
|
// low-autonomy reporting variant.
|
|
func (sc *SelfCorrector) feedback(report CorrectionReport, corrective bool) string {
|
|
var b strings.Builder
|
|
if corrective {
|
|
b.WriteString("Verification failed after your edit. Fix these and continue:\n")
|
|
} else {
|
|
b.WriteString("Verification failed after your edit (auto-correction is off at low autonomy; reporting only):\n")
|
|
}
|
|
|
|
for _, msg := range report.InspectErrors {
|
|
fmt.Fprintf(&b, "could not verify after edit: %s\n", msg)
|
|
}
|
|
for file, diags := range report.LSPDiagnostics {
|
|
b.WriteString(lsp.FormatDiagnostics(file, diags))
|
|
b.WriteString("\n")
|
|
}
|
|
for _, result := range report.VerifyReport.Results {
|
|
if result.Status == verify.StatusPass {
|
|
continue
|
|
}
|
|
fmt.Fprintf(&b, "%s (%s): %s\n", result.Name, strings.Join(result.Command, " "), result.Status)
|
|
for _, line := range verifyResultLines(result) {
|
|
b.WriteString(" " + line + "\n")
|
|
}
|
|
}
|
|
|
|
// Secret-scrub the whole message: verify/LSP output can echo args, paths, env.
|
|
return strings.TrimRight(redaction.RedactString(b.String(), redaction.Options{}), "\n")
|
|
}
|
|
|
|
// verifyResultLines returns the most useful, already-trimmed output for a failing
|
|
// check: the captured summary lines if present, else a trimmed stderr/stdout/err.
|
|
func verifyResultLines(result verify.Result) []string {
|
|
if result.OutputSummary != nil && len(result.OutputSummary.Lines) > 0 {
|
|
return result.OutputSummary.Lines
|
|
}
|
|
for _, candidate := range []string{result.Stderr, result.Stdout, result.Error} {
|
|
if trimmed := strings.TrimSpace(candidate); trimmed != "" {
|
|
return strings.Split(trimmed, "\n")
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func (sc *SelfCorrector) absPath(path string) string {
|
|
if filepath.IsAbs(path) {
|
|
return path
|
|
}
|
|
return filepath.Join(sc.workspaceRoot, path)
|
|
}
|
|
|
|
func autonomyAllowsAutoCorrect(autonomy string) bool {
|
|
switch strings.ToLower(strings.TrimSpace(autonomy)) {
|
|
case "medium", "high":
|
|
return true
|
|
default:
|
|
// low / unset: conservative — report, don't auto-attempt.
|
|
return false
|
|
}
|
|
}
|
|
|
|
func abortedSelfCorrectNotice(maxAttempts int) string {
|
|
return fmt.Sprintf("Verification is still failing after %d self-correction attempts; stopping auto-correction. Review the remaining failures and decide how to proceed.", maxAttempts)
|
|
}
|
|
|
|
// lspDiagnosticsChecker adapts *lsp.Manager to diagnosticsChecker by reading the
|
|
// file's current contents before asking the manager to check it.
|
|
type lspDiagnosticsChecker struct {
|
|
manager *lsp.Manager
|
|
}
|
|
|
|
// NewLSPDiagnosticsChecker wraps an *lsp.Manager for use as a SelfCorrector
|
|
// checker. Returns nil when manager is nil so self-correct cleanly skips LSP.
|
|
func NewLSPDiagnosticsChecker(manager *lsp.Manager) diagnosticsChecker {
|
|
if manager == nil {
|
|
return nil
|
|
}
|
|
return lspDiagnosticsChecker{manager: manager}
|
|
}
|
|
|
|
func (c lspDiagnosticsChecker) Check(ctx context.Context, path string) ([]lsp.Diagnostic, error) {
|
|
text, err := os.ReadFile(path)
|
|
if err != nil {
|
|
return nil, err // unreadable (e.g. deleted) -> let the caller decide
|
|
}
|
|
return c.manager.Check(ctx, path, string(text))
|
|
}
|
|
|
|
// detectPlanVerifier is the default projectVerifier: detect the plan once per
|
|
// run and execute it with verify.Run.
|
|
type detectPlanVerifier struct {
|
|
root string
|
|
options verify.RunOptions
|
|
}
|
|
|
|
// NewProjectVerifier builds the default verifier rooted at workspaceRoot.
|
|
func NewProjectVerifier(workspaceRoot string) projectVerifier {
|
|
return detectPlanVerifier{root: workspaceRoot}
|
|
}
|
|
|
|
func (v detectPlanVerifier) Verify(ctx context.Context) (verify.Report, error) {
|
|
plan, err := verify.DetectPlan(v.root)
|
|
if err != nil {
|
|
return verify.Report{}, err
|
|
}
|
|
return verify.Run(ctx, plan, v.options), nil
|
|
}
|