Resolve all defect-hunt findings: wire dead features, fix bugs, prune dead code

cmd/nilcore/autonomy.go

@@ -70,19 +70,34 @@ func runAutonomyDaemon(ctx context.Context, orch *agent.Orchestrator, log *event
 		return
 	}
 
+	// Source 1: the operator standing-objectives backlog (idle-gated). Constructed
+	// before the handler so the handler can close over it and record a VERIFIED outcome
+	// (MarkSuccess) — advancing LastSuccess so the full MinPeriod cadence (not the
+	// shorter RetryPeriod) gates the next pull. The BacklogSource already advances the
+	// debounce clock (MarkAttempt) at selection; the handler completes the cadence loop
+	// on a green verdict, so a failed/gate-denied objective re-arms after RetryPeriod
+	// while a verified one waits a full MinPeriod.
+	backlog := objective.New(s.ObjectiveStore())
+
 	handler := func(ctx context.Context, sig trigger.Signal) error {
 		// Run the inert goal (objective / file signal / wake note) through the verified
 		// orchestrator: reversible by construction (a disposable worktree), with every
 		// irreversible step hitting the headless gate the caller wired onto orch.Approver.
-		_, err := runViaKernel(ctx, orch, backend.Task{
+		outcome, err := runViaKernel(ctx, orch, backend.Task{
 			ID:   fmt.Sprintf("auto-%d", time.Now().UnixNano()),
 			Goal: sig.Goal,
 		})
+		// A backlog objective that VERIFIED (I2: outcome.Verified is the verifier's
+		// verdict, never a self-report) advances LastSuccess so its success-aware cadence
+		// applies. Only backlog signals carry a standing objective; file/wake signals have
+		// no cadence to record. A MarkSuccess failure is non-fatal to the run — the work is
+		// done and verified; a missed timestamp only means the objective re-arms sooner.
+		if err == nil && outcome.Verified && sig.Source == "backlog" {
+			markObjectiveSuccess(ctx, backlog, sig.Goal)
+		}
 		return err
 	}
 
-	// Source 1: the operator standing-objectives backlog (idle-gated).
-	backlog := objective.New(s.ObjectiveStore())
 	sources := []autosrc.Source{autosrc.NewBacklogSource(backlog, autosrc.BacklogConfig{Idle: idle})}
 
 	// Source 2: dropped file signals (reactive). A feeder goroutine owns the directory
@@ -107,6 +122,37 @@ func runAutonomyDaemon(ctx context.Context, orch *agent.Orchestrator, log *event
 	}
 }
 
+// markObjectiveSuccess records a VERIFIED backlog outcome against its standing objective,
+// advancing LastSuccess (and LastRun) so the full MinPeriod cadence — not the shorter
+// RetryPeriod — gates the next pull. The backlog signal carries only the operator-authored
+// Goal (trigger.Signal has no objective ID), so the objective is resolved by matching that
+// Goal against the enabled backlog: it is marked only when EXACTLY ONE enabled objective
+// has that goal, so an ambiguous (duplicate-goal) or vanished objective is left to re-arm
+// after RetryPeriod rather than crediting the wrong record. Best-effort: any store error is
+// swallowed with a stderr note — the verified work already shipped (I2); a missed success
+// timestamp only re-services the objective sooner, never wrongly marks it done.
+func markObjectiveSuccess(ctx context.Context, backlog *objective.Backlog, goal string) {
+	objs, err := backlog.List(ctx)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "nilcore: autonomy MarkSuccess lookup failed: %v\n", err)
+		return
+	}
+	var match string
+	var n int
+	for _, o := range objs {
+		if o.Enabled && o.Goal == goal {
+			match = o.ID
+			n++
+		}
+	}
+	if n != 1 {
+		return // no unambiguous objective to credit; it re-arms after RetryPeriod
+	}
+	if err := backlog.MarkSuccess(ctx, match, time.Now()); err != nil {
+		fmt.Fprintf(os.Stderr, "nilcore: autonomy MarkSuccess(%q) failed: %v\n", match, err)
+	}
+}
+
 // fileSignalFeeder polls dir on a ticker, reading each file as a FileSignal (name +
 // trimmed contents) and removing it (processed once), then sending it onto ch for the
 // FileSource adapter. It owns the host I/O so the adapter stays a pure mapper. It

cmd/nilcore/autonomy_test.go

@@ -2,16 +2,134 @@ package main
 
 import (
 	"context"
+	"errors"
 	"os"
 	"path/filepath"
 	"sync"
 	"testing"
 	"time"
 
 	"nilcore/internal/autosrc"
+	"nilcore/internal/objective"
 	"nilcore/internal/wake"
 )
 
+// fakeObjectiveStore is a tiny in-memory objective.Store for exercising the autonomy
+// handler's MarkSuccess wiring without standing up SQLite.
+type fakeObjectiveStore struct {
+	m map[string]objective.Objective
+}
+
+func newFakeObjectiveStore(objs ...objective.Objective) *fakeObjectiveStore {
+	m := make(map[string]objective.Objective, len(objs))
+	for _, o := range objs {
+		m[o.ID] = o
+	}
+	return &fakeObjectiveStore{m: m}
+}
+
+func (f *fakeObjectiveStore) Put(_ context.Context, o objective.Objective) error {
+	f.m[o.ID] = o
+	return nil
+}
+func (f *fakeObjectiveStore) Get(_ context.Context, id string) (objective.Objective, error) {
+	o, ok := f.m[id]
+	if !ok {
+		return objective.Objective{}, objective.ErrNotFound
+	}
+	return o, nil
+}
+func (f *fakeObjectiveStore) List(_ context.Context) ([]objective.Objective, error) {
+	out := make([]objective.Objective, 0, len(f.m))
+	for _, o := range f.m {
+		out = append(out, o)
+	}
+	return out, nil
+}
+func (f *fakeObjectiveStore) Disable(_ context.Context, id string) error {
+	o, ok := f.m[id]
+	if !ok {
+		return objective.ErrNotFound
+	}
+	o.Enabled = false
+	f.m[id] = o
+	return nil
+}
+
+// TestMarkObjectiveSuccessAdvancesLastSuccess proves the handler's verified-outcome
+// wiring: markObjectiveSuccess resolves the enabled objective by its goal and advances
+// LastSuccess (and LastRun), so the success-aware cadence (MinPeriod, not the shorter
+// RetryPeriod) gates the next pull.
+func TestMarkObjectiveSuccessAdvancesLastSuccess(t *testing.T) {
+	ctx := context.Background()
+	fs := newFakeObjectiveStore(
+		objective.Objective{ID: "ci", Goal: "keep CI green", Enabled: true,
+			MinPeriod: time.Hour, RetryPeriod: time.Minute},
+	)
+	backlog := objective.New(fs)
+
+	markObjectiveSuccess(ctx, backlog, "keep CI green")
+
+	got, err := backlog.Get(ctx, "ci")
+	if err != nil {
+		t.Fatalf("get: %v", err)
+	}
+	if got.LastSuccess.IsZero() || got.LastRun.IsZero() {
+		t.Fatalf("MarkSuccess must advance LastRun and LastSuccess: %+v", got)
+	}
+	if !got.LastSuccess.Equal(got.LastRun) {
+		t.Fatalf("MarkSuccess should set LastRun == LastSuccess: %+v", got)
+	}
+}
+
+// TestMarkObjectiveSuccessSkipsAmbiguous proves the handler credits nothing when the
+// goal does not resolve to exactly one enabled objective — an unmatched goal (a
+// file/wake signal that happened to reach here) or a duplicate-goal backlog is left to
+// re-arm after RetryPeriod rather than crediting the wrong record.
+func TestMarkObjectiveSuccessSkipsAmbiguous(t *testing.T) {
+	ctx := context.Background()
+	fs := newFakeObjectiveStore(
+		objective.Objective{ID: "a", Goal: "dup goal", Enabled: true},
+		objective.Objective{ID: "b", Goal: "dup goal", Enabled: true},
+	)
+	backlog := objective.New(fs)
+
+	markObjectiveSuccess(ctx, backlog, "dup goal")     // ambiguous ⇒ no-op
+	markObjectiveSuccess(ctx, backlog, "no such goal") // unmatched ⇒ no-op
+
+	for _, id := range []string{"a", "b"} {
+		got, err := backlog.Get(ctx, id)
+		if err != nil {
+			t.Fatalf("get %q: %v", id, err)
+		}
+		if !got.LastSuccess.IsZero() {
+			t.Fatalf("ambiguous/unmatched goal must not credit %q: %+v", id, got)
+		}
+	}
+}
+
+// TestMarkObjectiveSuccessListErrorIsNonFatal proves a store error during the lookup is
+// swallowed (best-effort): the verified work already shipped, so a missed success
+// timestamp must never panic or propagate — it only re-services the objective sooner.
+func TestMarkObjectiveSuccessListErrorIsNonFatal(t *testing.T) {
+	backlog := objective.New(errObjectiveStore{})
+	// Must not panic; nothing to assert beyond a clean return.
+	markObjectiveSuccess(context.Background(), backlog, "anything")
+}
+
+type errObjectiveStore struct{}
+
+func (errObjectiveStore) Put(context.Context, objective.Objective) error { return errObjBoom }
+func (errObjectiveStore) Get(context.Context, string) (objective.Objective, error) {
+	return objective.Objective{}, errObjBoom
+}
+func (errObjectiveStore) List(context.Context) ([]objective.Objective, error) {
+	return nil, errObjBoom
+}
+func (errObjectiveStore) Disable(context.Context, string) error { return errObjBoom }
+
+var errObjBoom = errors.New("objective store unavailable")
+
 // TestFileSignalFeeder proves the file funnel: a dropped goal file is read into a
 // FileSignal (name + trimmed contents), emitted on the channel the FileSource drains,
 // and removed (processed once).

cmd/nilcore/browse.go

@@ -204,7 +204,7 @@ func browseMain(args []string) {
 	// The tool surface: the stateful browse tool (no shell — a browse agent has no
 	// arbitrary-execution path, structurally). In extraction mode, record_finding
 	// writes verifier-gated findings. Optionally read-only repo tools.
-	bt := &browseragent.BrowseTool{Sess: sess, MaxSteps: *bf.maxSteps, EventSink: browseEventSink(log)}
+	bt := buildBrowseTool(sess, *bf.maxSteps, browseEventSink(log), approver)
 	reg := tools.NewRegistry(bt)
 	if extractID != "" {
 		reg.Register(&browseragent.FindingTool{Root: absDir, ArtifactID: extractID, Title: *bf.goal, Sess: sess})
@@ -246,6 +246,17 @@ func browseMain(args []string) {
 	fmt.Println(strings.TrimSpace(out.Summary))
 }
 
+// buildBrowseTool constructs the stateful browse tool with its per-action irreversible
+// gate wired in. The console approver (also the Rule-of-Two session gate) is passed as
+// BrowseTool.Approver so browseragent.irreversibleTarget routes a purchase/pay/delete/
+// accept-terms click (or an Enter-to-submit on such a page) through the human gate.
+// Without an Approver, browseragent's per-action check fails CLOSED on EVERY irreversible
+// action — a production tool built without one would dead-block them permanently rather
+// than gate them. Extracted so the wiring is unit-testable (the approver is non-nil).
+func buildBrowseTool(sess browseragent.Session, maxSteps int, sink browseragent.EventSink, approver browseragent.Approver) *browseragent.BrowseTool {
+	return &browseragent.BrowseTool{Sess: sess, MaxSteps: maxSteps, EventSink: sink, Approver: approver}
+}
+
 // ternary is a tiny helper for the capguard reason map.
 func ternary(cond bool, a, b string) string {
 	if cond {

cmd/nilcore/browse_test.go

@@ -0,0 +1,84 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"nilcore/internal/browseragent"
+	"nilcore/internal/browserwire"
+)
+
+// fakeBrowseSess is a minimal browseragent.Session for the wiring test: it records the
+// acts it receives and reports a "Pay now" button as the latest snapshot so the browse
+// tool classifies a click on it as irreversible.
+type fakeBrowseSess struct {
+	got    []browserwire.Act
+	latest browserwire.Observation
+}
+
+func (f *fakeBrowseSess) Act(_ context.Context, a browserwire.Act) (browserwire.Observation, error) {
+	f.got = append(f.got, a)
+	return f.latest, nil
+}
+func (f *fakeBrowseSess) Latest() browserwire.Observation { return f.latest }
+
+// recordingApprover captures the gate prompt and returns a fixed verdict.
+type recordingApprover struct {
+	verdict bool
+	prompts []string
+}
+
+func (r *recordingApprover) Approve(action string) bool {
+	r.prompts = append(r.prompts, action)
+	return r.verdict
+}
+
+// TestBuildBrowseToolWiresApprover proves the production wiring fix: the browse tool
+// built by buildBrowseTool routes an irreversible action through the injected Approver
+// instead of failing closed. A tool built WITHOUT an Approver (the pre-fix bug) would
+// dead-block every irreversible action permanently. We assert both halves: with an
+// approving gate the click reaches the session; with a nil approver it fails closed.
+func TestBuildBrowseToolWiresApprover(t *testing.T) {
+	latest := browserwire.Observation{
+		Version: 1,
+		Refs:    []browserwire.Ref{{ID: 1, Role: "button", Name: "Pay now", Version: 1}},
+	}
+	click, _ := json.Marshal(map[string]any{"op": "click", "ref": 1})
+
+	// (a) Approver present + approves → the irreversible click is gated, then performed.
+	sess := &fakeBrowseSess{latest: latest}
+	appr := &recordingApprover{verdict: true}
+	bt := buildBrowseTool(sess, 10, nil, appr)
+	if bt.Approver == nil {
+		t.Fatal("buildBrowseTool must wire the Approver (nil ⇒ every irreversible action dead-blocked)")
+	}
+	if _, _, err := bt.RunWithImage(context.Background(), ".", click); err != nil {
+		t.Fatalf("RunWithImage: %v", err)
+	}
+	if len(appr.prompts) != 1 || !strings.Contains(appr.prompts[0], "pay") {
+		t.Fatalf("the irreversible click must route through the approver, prompts=%v", appr.prompts)
+	}
+	if len(sess.got) != 1 {
+		t.Fatalf("an approved irreversible action must reach the session, got %d acts", len(sess.got))
+	}
+
+	// (b) No Approver → the same click fails closed (blocked, never performed) — the
+	// pre-fix production behavior, shown here to be the failure mode the wiring avoids.
+	sess2 := &fakeBrowseSess{latest: latest}
+	bt2 := buildBrowseTool(sess2, 10, nil, nil)
+	out, _, err := bt2.RunWithImage(context.Background(), ".", click)
+	if err != nil {
+		t.Fatalf("RunWithImage(no approver): %v", err)
+	}
+	if !strings.Contains(out, "BLOCKED") {
+		t.Fatalf("a nil-approver irreversible click must be blocked, got %q", out)
+	}
+	if len(sess2.got) != 0 {
+		t.Fatal("a blocked irreversible action must never reach the session")
+	}
+}
+
+// ensure the console-approver type satisfies the browse Approver seam (compile guard).
+var _ browseragent.Approver = (*recordingApprover)(nil)

cmd/nilcore/desktop.go

@@ -237,12 +237,17 @@ func desktopMain(args []string) {
 	// Path B (default) advertises our generic `computer` tool; Path A (-native /
 	// NILCORE_COMPUTER_NATIVE) advertises Anthropic's native `computer` beta tool,
 	// translating its actions to the SAME driver. Both share the governed body.
+	// The console approver (also the Rule-of-Two / host-control session gate above) is the
+	// per-action irreversible gate: the computer tool routes a delete/pay/accept-terms
+	// click, or an Enter-to-submit on such a dialog, through it (deny-default headless),
+	// symmetric with the browse tier. Without it, an irreversible desktop action would
+	// only be checked by the prompt + the one-time session gate.
 	var computer tools.Tool
 	if native {
-		computer = &desktopagent.NativeComputerTool{Sess: sess, MaxSteps: *df.maxSteps, EventSink: desktopEventSink(log)}
+		computer = &desktopagent.NativeComputerTool{Sess: sess, MaxSteps: *df.maxSteps, EventSink: desktopEventSink(log), Approver: approver}
 		fmt.Fprintln(os.Stderr, "nilcore desktop: Path A (native Anthropic computer tool) enabled — pixel-mode, vendor-locked to Anthropic for this run.")
 	} else {
-		computer = &desktopagent.ComputerTool{Sess: sess, MaxSteps: *df.maxSteps, EventSink: desktopEventSink(log)}
+		computer = &desktopagent.ComputerTool{Sess: sess, MaxSteps: *df.maxSteps, EventSink: desktopEventSink(log), Approver: approver}
 	}
 	reg := computerToolRegistry(computer, *df.readRepo)
 

cmd/nilcore/flows.go

@@ -120,7 +120,7 @@ func (d flowDoc) toAdapterFlow() agenticflows.Flow {
 			tool = n.ID
 		}
 		nodes = append(nodes, agenticflows.Node{
-			ID: n.ID, Type: n.Type, Title: n.Title, Description: n.Description, Agent: n.Agent, Tool: tool,
+			ID: n.ID, Type: n.Type, Title: n.Title, Description: n.Description, Tool: tool,
 		})
 	}
 	return agenticflows.Flow{

cmd/nilcore/live.go

@@ -17,16 +17,17 @@ import (
 // closeFn() releases the graph (called when the loop returns, so the handle is
 // task-scoped — no leak). A graph-open failure degrades to nil funcs (the loop runs
 // without the `live` tool). Enabled opt-in via NILCORE_LIVE_INDEX.
-func liveSession(mem *memory.Memory, project string) func(string) (func(context.Context, string), func(context.Context, string) string, func()) {
-	return func(dir string) (func(context.Context, string), func(context.Context, string) string, func()) {
+func liveSession(mem *memory.Memory, project string) func(string) (func(context.Context, string), func(context.Context, string), func(context.Context, string) string, func()) {
+	return func(dir string) (func(context.Context, string), func(context.Context, string), func(context.Context, string) string, func()) {
 		g, err := graph.Open(":memory:")
 		if err != nil {
-			return nil, nil, nil
+			return nil, nil, nil, nil
 		}
 		ix := &live.Index{Graph: g, Memory: mem, Project: project}
 		_ = ix.IndexDir(context.Background(), dir) // best-effort initial index
 
 		update := func(ctx context.Context, path string) { _ = ix.Update(ctx, path) }
+		remove := func(ctx context.Context, path string) { _ = ix.Remove(ctx, path) }
 		query := func(ctx context.Context, symbol string) string {
 			facts, err := ix.Query(ctx, symbol)
 			if err != nil {
@@ -35,7 +36,7 @@ func liveSession(mem *memory.Memory, project string) func(string) (func(context.
 			return renderLiveFacts(symbol, facts)
 		}
 		closeFn := func() { _ = g.Close() }
-		return update, query, closeFn
+		return update, remove, query, closeFn
 	}
 }