From e5ec4abe043cb790c684516c2716da6364d23c1b Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:30:46 +0400 Subject: [PATCH 1/7] a corrected turn is promoted to a label that outlives the trace (V-630) Migration #24 adds routing_labels, and CorrectTurn writes it. Nothing calls it yet; the wire and the surface are the next commits. Separate table, and that is the whole retention argument. A trace is a transcript and expires in 14 days. A correction is a label the owner wrote by hand, and it is the only supervised signal this box will ever get, so it is promoted out at the moment he writes it and kept. should_be may be empty. "That was wrong" with no target is a usable negative and must not cost more to give than the full answer. UNIQUE(utterance) so a second correction replaces the first, because his second answer is the one he meant. The label and the trace stamp go in one transaction: a stamp with no label loses the signal when the trace expires. ErrNoSuchTrace is held apart from a write failure. Correcting a turn older than the bound is the expected case, and the surface should say so rather than report a broken database. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0117tgnmbgZpHVV3XSNw8Qua --- internal/store/migrations.go | 21 +++++ internal/store/routinglabels.go | 111 +++++++++++++++++++++++++ internal/store/routinglabels_test.go | 118 +++++++++++++++++++++++++++ 3 files changed, 250 insertions(+) create mode 100644 internal/store/routinglabels.go create mode 100644 internal/store/routinglabels_test.go diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 720b822..3381064 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -330,6 +330,27 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2 claims TEXT NOT NULL DEFAULT '[]' ); CREATE INDEX IF NOT EXISTS idx_routing_traces_ts ON routing_traces (ts DESC);`, + // #24 — the corrected pairs (V-630). Separate from routing_traces on + // purpose, and this is the whole retention argument: a trace is a transcript + // and expires in 14 days, while a correction is a label the owner wrote by + // hand and is the only supervised signal the box will ever get. Promoting it + // out at the moment he writes it means the label survives the transcript + // that carried it. + // + // should_be may be empty. "That was wrong" with no target is a usable + // negative and must not cost more to give than the full answer would. + // + // UNIQUE(utterance) so correcting the same sentence twice replaces the + // label rather than stacking two. His second answer is the one he meant. + `CREATE TABLE IF NOT EXISTS routing_labels ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ts INTEGER NOT NULL, + utterance TEXT NOT NULL UNIQUE, + was TEXT NOT NULL DEFAULT '', + should_be TEXT NOT NULL DEFAULT '', + source TEXT NOT NULL DEFAULT '', + encoder_id TEXT NOT NULL DEFAULT '' + );`, } // migrate applies every migration with a number greater than the DB's current diff --git a/internal/store/routinglabels.go b/internal/store/routinglabels.go new file mode 100644 index 0000000..b21c584 --- /dev/null +++ b/internal/store/routinglabels.go @@ -0,0 +1,111 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + "time" +) + +// ErrNoSuchTrace — the trace the correction names is gone or never existed. +// Held apart from a write failure because it is the expected outcome of +// correcting a turn older than the 14-day bound, and the surface should say that +// rather than report a broken database. +var ErrNoSuchTrace = errors.New("no such routing trace") + +// RoutingLabel is one correction: what he said, what she made of it, and what it +// should have been. It is the only supervised signal in the box, so it outlives +// the trace it came from (V-630, docs/plans/22-correcting-a-turn.md). +type RoutingLabel struct { + ID int64 `json:"id"` + Ts time.Time `json:"ts"` + Utterance string `json:"utterance"` + // Was is the intent the cascade chose. Kept beside the target because the + // pair is what names the confusion, and a label with no "was" cannot say + // which boundary moved. + Was string `json:"was"` + // ShouldBe is the owner's target, and may be empty. "That was wrong, I am + // not going to tell you what it was" is a usable negative, and requiring the + // target would cost the cheap half of the gesture. + ShouldBe string `json:"should_be"` + Source string `json:"source"` + EncoderID string `json:"encoder_id"` +} + +// CorrectTurn records the owner's correction of one persisted turn. It promotes +// the pair into routing_labels and stamps the trace, both in one transaction: +// a stamped trace with no label would lose the signal when the trace expires, +// and a label with no stamp would let the same turn be corrected twice. +// +// shouldBe empty is allowed and means "wrong, target unstated". +func (s *Store) CorrectTurn(ctx context.Context, traceID int64, shouldBe string, now time.Time) error { + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("correct turn: begin: %w", err) + } + defer func() { _ = tx.Rollback() }() + + var utterance, was, source, encoderID string + err = tx.QueryRowContext(ctx, ` + SELECT utterance, intent, source, encoder_id FROM routing_traces WHERE id = ?`, + traceID).Scan(&utterance, &was, &source, &encoderID) + if errors.Is(err, sql.ErrNoRows) { + return ErrNoSuchTrace + } + if err != nil { + return fmt.Errorf("correct turn: read trace: %w", err) + } + + shouldBe = strings.TrimSpace(shouldBe) + if _, err := tx.ExecContext(ctx, ` + INSERT INTO routing_labels (ts, utterance, was, should_be, source, encoder_id) + VALUES (?,?,?,?,?,?) + ON CONFLICT(utterance) DO UPDATE SET + ts = excluded.ts, was = excluded.was, should_be = excluded.should_be, + source = excluded.source, encoder_id = excluded.encoder_id`, + now.UnixMilli(), utterance, was, shouldBe, source, encoderID); err != nil { + return fmt.Errorf("correct turn: write label: %w", err) + } + // The stamp is what the trace itself carries: "corrected", or the target he + // gave. It expires with the trace, and that is fine — the label above is the + // durable half. + stamp := shouldBe + if stamp == "" { + stamp = "wrong" + } + if _, err := tx.ExecContext(ctx, + `UPDATE routing_traces SET correction = ? WHERE id = ?`, stamp, traceID); err != nil { + return fmt.Errorf("correct turn: stamp trace: %w", err) + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("correct turn: commit: %w", err) + } + return nil +} + +// RoutingLabels returns the newest n corrections, newest first. Nothing prunes +// them: 31 modes and 9 of them with no example at all is the problem this table +// exists to solve, and a label is a few dozen bytes. +func (s *Store) RoutingLabels(ctx context.Context, n int) ([]RoutingLabel, error) { + rows, err := s.db.QueryContext(ctx, ` + SELECT id, ts, utterance, was, should_be, source, encoder_id + FROM routing_labels ORDER BY id DESC LIMIT ?`, n) + if err != nil { + return nil, fmt.Errorf("routing labels: %w", err) + } + defer rows.Close() + var out []RoutingLabel + for rows.Next() { + var l RoutingLabel + var tsMilli int64 + if err := rows.Scan(&l.ID, &tsMilli, &l.Utterance, &l.Was, &l.ShouldBe, + &l.Source, &l.EncoderID); err != nil { + return nil, err + } + l.Ts = time.UnixMilli(tsMilli).UTC() + out = append(out, l) + } + return out, rows.Err() +} diff --git a/internal/store/routinglabels_test.go b/internal/store/routinglabels_test.go new file mode 100644 index 0000000..c858569 --- /dev/null +++ b/internal/store/routinglabels_test.go @@ -0,0 +1,118 @@ +package store + +import ( + "context" + "errors" + "testing" + "time" +) + +func seedTrace(t *testing.T, s *Store, utterance, intent string, now time.Time) int64 { + t.Helper() + id, err := s.WriteRoutingTrace(context.Background(), RoutingTrace{ + Ts: now, Utterance: utterance, Intent: intent, Source: "tap:text", EncoderID: "e5-small", + }) + if err != nil { + t.Fatal(err) + } + return id +} + +// The label carries the pair, and it is what survives the transcript. +func TestCorrectTurnPromotesTheLabel(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Date(2026, 8, 6, 12, 0, 0, 0, time.UTC) + id := seedTrace(t, s, "поужинал", "query", now) + + if err := s.CorrectTurn(ctx, id, "fact", now); err != nil { + t.Fatal(err) + } + labels, err := s.RoutingLabels(ctx, 10) + if err != nil { + t.Fatal(err) + } + if len(labels) != 1 { + t.Fatalf("got %d labels, want 1", len(labels)) + } + l := labels[0] + if l.Utterance != "поужинал" || l.Was != "query" || l.ShouldBe != "fact" { + t.Errorf("label %+v: the pair is what names the confusion", l) + } + if l.EncoderID != "e5-small" { + t.Errorf("encoder_id %q: a fitted distance means nothing without the body", l.EncoderID) + } + // The trace is stamped too, so the same turn cannot be corrected twice into + // two labels without the surface knowing. + traces, err := s.RecentRoutingTraces(ctx, 10) + if err != nil { + t.Fatal(err) + } + if traces[0].Correction != "fact" { + t.Errorf("trace correction %q, want fact", traces[0].Correction) + } +} + +// "Wrong, and I am not telling you what it was" is the cheap half of the +// gesture, and it must not cost more than the full answer. +func TestCorrectTurnWithNoTarget(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Date(2026, 8, 6, 12, 0, 0, 0, time.UTC) + id := seedTrace(t, s, "закрывай", "act", now) + + if err := s.CorrectTurn(ctx, id, " ", now); err != nil { + t.Fatal(err) + } + labels, err := s.RoutingLabels(ctx, 10) + if err != nil { + t.Fatal(err) + } + if len(labels) != 1 || labels[0].ShouldBe != "" { + t.Fatalf("labels %+v: an untargeted negative is still a label", labels) + } + traces, _ := s.RecentRoutingTraces(ctx, 10) + if traces[0].Correction != "wrong" { + t.Errorf("trace correction %q, want wrong", traces[0].Correction) + } +} + +// His second answer is the one he meant, so a re-correction replaces. +func TestCorrectTurnTwiceReplaces(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Date(2026, 8, 6, 12, 0, 0, 0, time.UTC) + first := seedTrace(t, s, "поужинал", "query", now) + second := seedTrace(t, s, "поужинал", "chat", now.Add(time.Minute)) + + if err := s.CorrectTurn(ctx, first, "note", now); err != nil { + t.Fatal(err) + } + if err := s.CorrectTurn(ctx, second, "fact", now.Add(time.Minute)); err != nil { + t.Fatal(err) + } + labels, err := s.RoutingLabels(ctx, 10) + if err != nil { + t.Fatal(err) + } + if len(labels) != 1 { + t.Fatalf("got %d labels for one sentence, want 1", len(labels)) + } + if labels[0].ShouldBe != "fact" || labels[0].Was != "chat" { + t.Errorf("label %+v, want the second correction", labels[0]) + } +} + +// A turn past the 14-day bound cannot be corrected, and the surface has to be +// able to say that rather than report a broken database. +func TestCorrectTurnUnknownTrace(t *testing.T) { + s := newTestStore(t) + err := s.CorrectTurn(context.Background(), 999, "fact", time.Now()) + if !errors.Is(err, ErrNoSuchTrace) { + t.Fatalf("err %v, want ErrNoSuchTrace", err) + } + labels, _ := s.RoutingLabels(context.Background(), 10) + if len(labels) != 0 { + t.Errorf("wrote %d labels for a trace that does not exist", len(labels)) + } +} From 4d97280d74fdcf71c4dc9622b7b890dc4c4cf8a6 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:45:37 +0400 Subject: [PATCH 2/7] a turn hands back its trace id, and one wire op corrects it (V-630) The correction is the only supervised signal in the box, so the cost of giving one has to be near zero. That means the surface needs the trace id of the turn it is showing, which it had no way to learn: handleText returns one string and the trace was written after the reply left. The id rides back on ChatReply through the same context sink querySource uses, so the mic, telegram and the web keep the one signature they share. CorrectTurn takes a trace id and an optional target, which is deliberately reach-agnostic: nothing about it assumes a browser. store.ErrNoSuchTrace gets a wire twin. A turn past the retention bound is gone, and that is the expected outcome of correcting an old turn, not a broken database. --- cmd/mavend/routingtrace.go | 50 +++++++++++++++++++++++++++++++-- cmd/mavend/routingtrace_test.go | 46 ++++++++++++++++++++++++++++++ cmd/mavend/tick_api.go | 11 +++++++- internal/ipc/api.go | 23 +++++++++++++-- internal/ipc/client.go | 8 +++++- internal/ipc/coreapi.go | 8 ++++++ internal/ipc/maperr_test.go | 1 + internal/ipc/server.go | 5 +++- internal/ipc/storeapi.go | 9 ++++++ internal/ipc/unimplemented.go | 4 +++ internal/ipc/wire.go | 4 +++ 11 files changed, 162 insertions(+), 7 deletions(-) diff --git a/cmd/mavend/routingtrace.go b/cmd/mavend/routingtrace.go index 02e531b..5c6832e 100644 --- a/cmd/mavend/routingtrace.go +++ b/cmd/mavend/routingtrace.go @@ -16,6 +16,7 @@ import ( "encoding/json" "log" "strings" + "sync" "time" "github.com/kami/maven/internal/decision" @@ -38,6 +39,44 @@ func traceSink(s *store.Store) traceWriter { return s } +// The trace id rides the context, the same seam querysource.go uses and for the +// same reason: handleText answers every reach through one string, and threading +// a second value through the whole action dispatch would change a signature the +// mic, telegram and the web all share. A caller that wants the id asks for a +// sink; the mic path does not, and pays nothing. +type traceIDKey struct{} + +type traceIDSink struct { + mu sync.Mutex + id int64 +} + +func (s *traceIDSink) note(id int64) { + s.mu.Lock() + defer s.mu.Unlock() + s.id = id +} + +// ID is the persisted trace for the turn, or 0 when nothing was persisted. +func (s *traceIDSink) ID() int64 { + s.mu.Lock() + defer s.mu.Unlock() + return s.id +} + +// withTraceIDSink returns a context that collects the persisted trace id, and +// the sink to read after the turn has answered. +func withTraceIDSink(ctx context.Context) (context.Context, *traceIDSink) { + sink := &traceIDSink{} + return context.WithValue(ctx, traceIDKey{}, sink), sink +} + +func noteTraceID(ctx context.Context, id int64) { + if sink, ok := ctx.Value(traceIDKey{}).(*traceIDSink); ok { + sink.note(id) + } +} + // pruneTracesOnStart enforces retention once at wiring time. Pruning on write // alone is not enough: a box that goes quiet for a month keeps every row until // the next sixty-fourth turn, and "kept for fourteen days" would then be true @@ -58,7 +97,8 @@ func pruneTracesOnStart(s *store.Store, now time.Time) { // // Errors are logged and swallowed. A trace is diagnostic and training data, and // a failed insert must never change what the owner hears. -func (h *reactiveHandler) persistDecision(ctx context.Context, rec *decision.Record, src turnSource) { +func (h *reactiveHandler) persistDecision(turnCtx context.Context, rec *decision.Record, src turnSource) { + ctx := turnCtx if h.traces == nil || rec == nil || strings.TrimSpace(rec.Utterance) == "" { return } @@ -85,9 +125,15 @@ func (h *reactiveHandler) persistDecision(ctx context.Context, rec *decision.Rec Outcome: wonAt(rec, decision.StageAction), Claims: claims, } - if _, err := h.traces.WriteRoutingTrace(ctx, tr); err != nil { + id, err := h.traces.WriteRoutingTrace(ctx, tr) + if err != nil { log.Printf("routing trace: write: %v", err) + return } + // The id goes back to whoever asked for it, so /chat can offer a correction + // on the turn it is already showing (V-630). Noted on the ORIGINAL context, + // not the detached one above: the sink belongs to the caller's turn. + noteTraceID(turnCtx, id) } // wonIntent — what the winning claimant made the turn. Read from the claim diff --git a/cmd/mavend/routingtrace_test.go b/cmd/mavend/routingtrace_test.go index 2ed45fb..88aa251 100644 --- a/cmd/mavend/routingtrace_test.go +++ b/cmd/mavend/routingtrace_test.go @@ -77,3 +77,49 @@ func TestEmptyUtteranceIsNotPersisted(t *testing.T) { } var _ traceWriter = (*store.Store)(nil) + +// The trace id rides back to the caller, which is what makes a correction one +// gesture: /chat already has the id, so saying "that was wrong" costs a button +// and no lookup (V-630). +func TestTurnHandsBackItsTraceID(t *testing.T) { + h := traceHandler(t, decision.NewRing()) + h.traces = traceSink(h.dataStore) + + ctx, sink := withTraceIDSink(context.Background()) + if reply := h.handleText(ctx, "web", "сколько сейчас времени"); reply == "" { + t.Fatal("turn produced no reply") + } + id := sink.ID() + if id == 0 { + t.Fatal("no trace id came back, so /chat can offer no correction") + } + // And it names the turn that just ran, so the correction lands on the right + // utterance. + if err := h.dataStore.CorrectTurn(context.Background(), id, "query", h.now()); err != nil { + t.Fatal(err) + } + labels, err := h.dataStore.RoutingLabels(context.Background(), 5) + if err != nil { + t.Fatal(err) + } + if len(labels) != 1 || labels[0].Utterance != "сколько сейчас времени" { + t.Fatalf("labels %+v, want the turn that just ran", labels) + } +} + +// A turn nobody asked the id of costs nothing, which is the mic path. +func TestTurnWithNoSinkStillPersists(t *testing.T) { + h := traceHandler(t, decision.NewRing()) + h.traces = traceSink(h.dataStore) + + if reply := h.handleText(context.Background(), "web", "сколько сейчас времени"); reply == "" { + t.Fatal("turn produced no reply") + } + got, err := h.dataStore.RecentRoutingTraces(context.Background(), 5) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 { + t.Fatalf("persisted %d traces, want 1", len(got)) + } +} diff --git a/cmd/mavend/tick_api.go b/cmd/mavend/tick_api.go index 236b521..adb63e4 100644 --- a/cmd/mavend/tick_api.go +++ b/cmd/mavend/tick_api.go @@ -97,10 +97,19 @@ func (d *daemonAPI) Chat(ctx context.Context, conversation, text string) (ipc.Ch return ipc.ChatReply{}, errors.New("mavend: chat not available") } ctx, sink := withQuerySourceSink(ctx) + // The trace id rides back the same way (V-630), so /chat can offer a + // correction on the turn it is already showing. 0 when nothing persisted. + ctx, traces := withTraceIDSink(ctx) reply := d.chatFn(ctx, conversation, text) - return ipc.ChatReply{Reply: reply, Source: sink.Name()}, nil + return ipc.ChatReply{Reply: reply, Source: sink.Name(), TraceID: traces.ID()}, nil } +// CorrectTurn is NOT overridden here, and that is deliberate (V-630). Every other +// diagnostic on this type exists because the daemon holds something the store +// cannot answer from a table. A correction is a table, so the embedded store +// adapter is already the right answer and a second implementation here would be +// a second place for it to drift. + // MCPServers — the configured MCP servers and their health (Vikunja #251). // Empty, not an error, when the mcp block is absent: "not configured" is the // default state and the web surface renders it as such. diff --git a/internal/ipc/api.go b/internal/ipc/api.go index 113c825..db90185 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -724,8 +724,15 @@ type chatReq struct { Conversation string `json:"conversation,omitempty"` } type chatResp struct { - Reply string `json:"reply"` - Source string `json:"source,omitempty"` + Reply string `json:"reply"` + Source string `json:"source,omitempty"` + TraceID int64 `json:"trace_id,omitempty"` +} + +// correctTurnReq — the owner correcting one persisted turn (V-630). +type correctTurnReq struct { + TraceID int64 `json:"trace_id"` + ShouldBe string `json:"should_be,omitempty"` } // ChatReply — one text turn's answer plus which query source claimed it. @@ -738,6 +745,12 @@ type chatResp struct { type ChatReply struct { Reply string Source string + // TraceID is the persisted routing trace for this turn (V-629), and it is + // what makes a correction one gesture: the surface already has the id, so + // saying "that was wrong" costs a button and no lookup. 0 ⇒ nothing was + // persisted, which is a box with no database, and the surface offers no + // correction rather than a broken one. + TraceID int64 } type proposeToolReq struct { @@ -937,6 +950,12 @@ var ErrTaskDuplicate = errors.New("ipc: another live task already has this text" // is down" must not read the same to a caller deciding whether to store an id. var ErrNoEntity = errors.New("ipc: no such entity") +// ErrNoSuchTrace — the turn a correction names is not in routing_traces. Given +// a wire twin because it is the expected outcome of correcting a turn older than +// the retention bound, and "that turn is gone" and "the database is broken" must +// not read the same to the surface offering the gesture. +var ErrNoSuchTrace = errors.New("ipc: no such routing trace") + // ErrTaskResolved — a resolved task is not editable. var ErrTaskResolved = errors.New("ipc: task is resolved") diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 946ed37..5cf1fc9 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -247,6 +247,8 @@ func hydrate(e *RpcError) error { return fmt.Errorf("%w: %s", ErrReminderState, e.Message) case codeToolNotFound: return fmt.Errorf("%w: %s", ErrToolNotFound, e.Message) + case codeNoSuchTrace: + return fmt.Errorf("%w: %s", ErrNoSuchTrace, e.Message) case codeUnknownMethod: return fmt.Errorf("%w: %s", ErrUnknownMethod, e.Message) case codeBadParams: @@ -661,7 +663,11 @@ func (c *Client) Chat(ctx context.Context, conversation, text string) (ChatReply if err := c.call(ctx, MethodChat, chatReq{Text: text, Conversation: conversation}, &r); err != nil { return ChatReply{}, err } - return ChatReply{Reply: r.Reply, Source: r.Source}, nil + return ChatReply{Reply: r.Reply, Source: r.Source, TraceID: r.TraceID}, nil +} + +func (c *Client) CorrectTurn(ctx context.Context, traceID int64, shouldBe string) error { + return c.call(ctx, MethodCorrectTurn, correctTurnReq{TraceID: traceID, ShouldBe: shouldBe}, nil) } func (c *Client) TickTrace(ctx context.Context) (TickTrace, error) { diff --git a/internal/ipc/coreapi.go b/internal/ipc/coreapi.go index d3d952d..cf3d5dc 100644 --- a/internal/ipc/coreapi.go +++ b/internal/ipc/coreapi.go @@ -176,6 +176,14 @@ type SystemAPI interface { // has run since the daemon started. TurnDecisions(ctx context.Context, n int) ([]TurnDecision, error) + // CorrectTurn records that one persisted turn was routed wrongly, and what + // it should have been (V-630). shouldBe empty means "wrong, target + // unstated", which is a usable negative and must not cost more to give than + // the full answer. Unlike TurnDecisions this DOES reach a table, because a + // correction is the only supervised signal the box gets and it has to + // outlive the trace that carried it. + CorrectTurn(ctx context.Context, traceID int64, shouldBe string) error + // RecentEcosystemTraces reads the ecosystem call log, which lives in its // own table so machine-rate traces never crowd out human-rate facts. RecentEcosystemTraces(ctx context.Context, n int) ([]EcosystemTrace, error) diff --git a/internal/ipc/maperr_test.go b/internal/ipc/maperr_test.go index 7bdc4b3..6ff31fb 100644 --- a/internal/ipc/maperr_test.go +++ b/internal/ipc/maperr_test.go @@ -31,6 +31,7 @@ var mapErrPairs = []struct { {"ErrReminderNotFound", store.ErrReminderNotFound, ErrReminderNotFound}, {"ErrReminderState", store.ErrReminderState, ErrReminderState}, {"ErrToolNotFound", store.ErrToolNotFound, ErrToolNotFound}, + {"ErrNoSuchTrace", store.ErrNoSuchTrace, ErrNoSuchTrace}, {"ErrTaskNoDoneWhen", store.ErrTaskNoDoneWhen, ErrTaskNoDoneWhen}, {"ErrTaskDuplicate", store.ErrTaskDuplicate, ErrTaskDuplicate}, {"ErrTaskResolved", store.ErrTaskResolved, ErrTaskResolved}, diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 4d035a3..97ee006 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -514,7 +514,10 @@ var methodTable = map[Method]handlerFunc{ }), MethodChat: withParams(func(ctx context.Context, api CoreAPI, p chatReq) (chatResp, error) { reply, err := api.Chat(ctx, p.Conversation, p.Text) - return chatResp{Reply: reply.Reply, Source: reply.Source}, err + return chatResp{Reply: reply.Reply, Source: reply.Source, TraceID: reply.TraceID}, err + }), + MethodCorrectTurn: withParams(func(ctx context.Context, api CoreAPI, p correctTurnReq) (struct{}, error) { + return struct{}{}, api.CorrectTurn(ctx, p.TraceID, p.ShouldBe) }), MethodTickTrace: withoutParams(func(ctx context.Context, api CoreAPI) (TickTrace, error) { return api.TickTrace(ctx) diff --git a/internal/ipc/storeapi.go b/internal/ipc/storeapi.go index 21fd7da..a0fe3f2 100644 --- a/internal/ipc/storeapi.go +++ b/internal/ipc/storeapi.go @@ -213,6 +213,13 @@ func (a *storeAPI) TickTrace(ctx context.Context) (TickTrace, error) { return TickTrace{}, errors.New("store: tick trace not available via direct store API") } +// CorrectTurn — unlike TickTrace and TurnDecisions this one is a table, so the +// store adapter answers it for real (V-630). A correction has to land whether +// the caller reached the daemon or the store directly. +func (a *storeAPI) CorrectTurn(ctx context.Context, traceID int64, shouldBe string) error { + return mapErr(a.s.CorrectTurn(ctx, traceID, shouldBe, time.Now())) +} + // TurnDecisions — same story as TickTrace: the arbitration record is a daemon // ring, not a table, so there is nothing here to read it from (V-564). func (a *storeAPI) TurnDecisions(ctx context.Context, n int) ([]TurnDecision, error) { @@ -412,6 +419,8 @@ func mapErr(err error) error { return ErrReminderState case errors.Is(err, store.ErrToolNotFound): return ErrToolNotFound + case errors.Is(err, store.ErrNoSuchTrace): + return ErrNoSuchTrace case errors.Is(err, store.ErrTaskNoDoneWhen): return ErrTaskNoDoneWhen case errors.Is(err, store.ErrTaskDuplicate): diff --git a/internal/ipc/unimplemented.go b/internal/ipc/unimplemented.go index 69681cb..e5837bc 100644 --- a/internal/ipc/unimplemented.go +++ b/internal/ipc/unimplemented.go @@ -144,6 +144,10 @@ func (UnimplementedCoreAPI) RevertFact(ctx context.Context, key string) (int64, func (UnimplementedCoreAPI) TickTrace(ctx context.Context) (TickTrace, error) { return TickTrace{}, ErrNotImplemented } +func (UnimplementedCoreAPI) CorrectTurn(ctx context.Context, traceID int64, shouldBe string) error { + return ErrNotImplemented +} + func (UnimplementedCoreAPI) TurnDecisions(ctx context.Context, n int) ([]TurnDecision, error) { return nil, ErrNotImplemented } diff --git a/internal/ipc/wire.go b/internal/ipc/wire.go index 4b6ac78..a2058b0 100644 --- a/internal/ipc/wire.go +++ b/internal/ipc/wire.go @@ -49,6 +49,7 @@ const ( MethodRevertFact Method = "revert_fact" MethodTickTrace Method = "tick_trace" MethodTurnDecisions Method = "turn_decisions" + MethodCorrectTurn Method = "correct_turn" MethodMorningStatus Method = "morning_status" MethodMCPServers Method = "mcp_servers" MethodDayPlan Method = "day_plan" @@ -125,6 +126,7 @@ const ( codeReminderMissing = "reminder_not_found" codeReminderState = "reminder_state" codeToolNotFound = "tool_not_found" + codeNoSuchTrace = "no_such_trace" codeUnknownMethod = "unknown_method" codeBadParams = "bad_params" codeForbidden = "forbidden" @@ -160,6 +162,8 @@ func codeOf(err error) string { return codeReminderState case errors.Is(err, ErrToolNotFound): return codeToolNotFound + case errors.Is(err, ErrNoSuchTrace): + return codeNoSuchTrace case errors.Is(err, ErrUnknownMethod): return codeUnknownMethod case errors.Is(err, ErrBadParams): From 0d5bd0a9f0d438dd87ac397256f6220ecbb75109 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:45:50 +0400 Subject: [PATCH 3/7] one gesture beside the reply corrects a turn (V-630) Two buttons' worth of cost: wrong, or wrong and it should have been this. The second is worth much more and is not required to give the first, so a turn marked wrong with no target still lands as a usable negative. The target is one of the seven intents, never free text: an unroutable label would enter the one table V-632 fits prototypes from. /api/correct is not behind the step-up gate. It reaches no router, no model and no act path, and a correction that costs a passkey tap is one that does not get made. --- cmd/mavweb/chat.go | 103 +++++++++++++++++++++++++- cmd/mavweb/chat.html | 15 ++++ cmd/mavweb/correct_test.go | 146 +++++++++++++++++++++++++++++++++++++ cmd/mavweb/main.go | 7 ++ cmd/mavweb/static/ui.css | 5 ++ 5 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 cmd/mavweb/correct_test.go diff --git a/cmd/mavweb/chat.go b/cmd/mavweb/chat.go index 383245c..e78a9ff 100644 --- a/cmd/mavweb/chat.go +++ b/cmd/mavweb/chat.go @@ -2,12 +2,15 @@ package main import ( _ "embed" + "errors" "log" "net/http" "net/url" + "strconv" "strings" "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/router" "github.com/kami/maven/internal/webauthn" ) @@ -25,6 +28,21 @@ type chatMsg struct { // Source — the query source that claimed the turn, shown as a badge beside // the reply. Empty for a turn no source claimed (V-539). Source string + // TraceID anchors the correction gesture (V-630). Non-zero ⇒ the turn was + // persisted and can be corrected in one click. 0 ⇒ no correction is offered, + // which is honest: a box with no database has no turn to correct. + TraceID int64 + // Corrected — the owner already corrected this turn, so the page says thank + // you instead of offering the buttons again. + Corrected string +} + +// correctionTargets — the seven public intents, in the order the buttons are +// shown. Read from internal/router rather than typed out, so a new intent cannot +// exist without a way to correct a turn into it. +var correctionTargets = []router.Intent{ + router.IntentFact, router.IntentNote, router.IntentReminder, + router.IntentQuery, router.IntentAct, router.IntentChat, router.IntentSystem, } // handleChatPage renders the chat conversation page. @@ -38,12 +56,21 @@ func handleChatPage(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { msgs = append(msgs, chatMsg{Role: "user", Text: q}) } if reply := r.URL.Query().Get("r"); reply != "" { - msgs = append(msgs, chatMsg{Role: "assistant", Text: reply, Source: r.URL.Query().Get("s")}) + id, _ := strconv.ParseInt(r.URL.Query().Get("t"), 10, 64) + msgs = append(msgs, chatMsg{ + Role: "assistant", Text: reply, Source: r.URL.Query().Get("s"), + TraceID: id, Corrected: r.URL.Query().Get("c"), + }) } + // UserText rides beside the messages so the correction form can hand the + // conversation back on the redirect: this page has no session and no JS, so + // what is on screen is what the query params carry. renderPage(w, chatTmpl, struct { Error string Messages []chatMsg - }{Messages: msgs}) + Targets []router.Intent + UserText string + }{Messages: msgs, Targets: correctionTargets, UserText: r.URL.Query().Get("q")}) } // handleChatAPI processes a chat message POST and redirects back to /chat. @@ -88,5 +115,77 @@ func handleChatAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, ses if reply.Source != "" { dest += "&s=" + url.QueryEscape(reply.Source) } + // The trace id rides along so the reply can carry a correction gesture + // (V-630). Absent when nothing persisted, and the page then offers none. + if reply.TraceID != 0 { + dest += "&t=" + strconv.FormatInt(reply.TraceID, 10) + } http.Redirect(w, r, dest, http.StatusSeeOther) } + +// handleCorrectAPI records that the last turn was routed wrongly (V-630). +// +// A correction is the only supervised signal this box gets, and everything else +// in the trace accumulates on its own. So the gesture has to cost nothing: one +// POST from the reply he is already looking at, carrying the trace id and +// optionally the intent it should have been. An unstated target is accepted, +// because a turn marked wrong with no target is still a usable negative. +// +// Not step-up gated, unlike POST /api/chat. Writing a label reaches no router, +// no model and no act path; it writes one row nothing executes from. Gating it +// would price the gesture out of being used, which is the one thing that makes +// it worthless. +func handleCorrectAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { + if r.Method != http.MethodPost { + http.Error(w, "POST only", http.StatusMethodNotAllowed) + return + } + if !requireCore(w, core, "correct") { + return + } + id, err := strconv.ParseInt(strings.TrimSpace(r.FormValue("trace_id")), 10, 64) + if err != nil || id <= 0 { + http.Error(w, "trace_id required", http.StatusBadRequest) + return + } + shouldBe := strings.TrimSpace(r.FormValue("should_be")) + // Only one of the seven, or nothing. Free text here would put an unroutable + // label in the one table V-632 fits prototypes from. + if shouldBe != "" && !isCorrectionTarget(shouldBe) { + http.Error(w, "should_be must be one of the seven intents", http.StatusBadRequest) + return + } + if err := core.CorrectTurn(r.Context(), id, shouldBe); err != nil { + log.Printf("correct turn %d: %v", id, err) + // A turn past the retention bound is gone, and saying so is different + // from saying the write broke. + if errors.Is(err, ipc.ErrNoSuchTrace) { + http.Error(w, "that turn is no longer stored", http.StatusNotFound) + return + } + http.Error(w, "correction failed", http.StatusBadGateway) + return + } + stamp := shouldBe + if stamp == "" { + stamp = "wrong" + } + // Back to the conversation he was in, with the turn still on screen. The + // query params carry it, so the correction is preserved by re-sending them. + dest := "/chat?q=" + url.QueryEscape(r.FormValue("q")) + + "&r=" + url.QueryEscape(r.FormValue("rep")) + "&c=" + url.QueryEscape(stamp) + if s := r.FormValue("s"); s != "" { + dest += "&s=" + url.QueryEscape(s) + } + http.Redirect(w, r, dest, http.StatusSeeOther) +} + +// isCorrectionTarget — one of the seven, and nothing else. +func isCorrectionTarget(s string) bool { + for _, t := range correctionTargets { + if string(t) == s { + return true + } + } + return false +} diff --git a/cmd/mavweb/chat.html b/cmd/mavweb/chat.html index ac35c0e..88dccc1 100644 --- a/cmd/mavweb/chat.html +++ b/cmd/mavweb/chat.html @@ -5,6 +5,21 @@
{{range .Messages}}
{{if eq .Role "user"}}you{{else}}maven{{end}}: {{.Text}}{{if .Source}} {{.Source}}{{end}}
+ {{if and (eq .Role "assistant") .TraceID}} + {{if .Corrected}} +
corrected: {{.Corrected}}
+ {{else}} +
+ + + + + + should have been: + {{range $.Targets}}{{end}} +
+ {{end}} + {{end}} {{else}}
diff --git a/cmd/mavweb/correct_test.go b/cmd/mavweb/correct_test.go new file mode 100644 index 0000000..20ecd59 --- /dev/null +++ b/cmd/mavweb/correct_test.go @@ -0,0 +1,146 @@ +package main + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/kami/maven/internal/ipc" +) + +// correctCore records the correction the handler sends. +type correctCore struct { + ipc.UnimplementedCoreAPI + traceID int64 + shouldBe string + called bool + err error +} + +func (c *correctCore) CorrectTurn(_ context.Context, traceID int64, shouldBe string) error { + c.called, c.traceID, c.shouldBe = true, traceID, shouldBe + return c.err +} + +func postCorrect(form url.Values) *http.Request { + req := httptest.NewRequest(http.MethodPost, "/api/correct", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +// The full gesture: wrong, and it should have been a fact. +func TestCorrectAPIWithTarget(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{ + "trace_id": {"42"}, "should_be": {"fact"}, "q": {"поужинал"}, "rep": {"поняла"}, + }), core) + + if rr.Code != http.StatusSeeOther { + t.Fatalf("status %d, want 303; body=%s", rr.Code, rr.Body.String()) + } + if core.traceID != 42 || core.shouldBe != "fact" { + t.Errorf("corrected trace %d to %q", core.traceID, core.shouldBe) + } + // The turn stays on screen, and the page says it was corrected. + loc := rr.Header().Get("Location") + if !strings.Contains(loc, "c=fact") || !strings.Contains(loc, "q=") { + t.Errorf("redirect %q loses the turn or the correction", loc) + } +} + +// The cheap half. A turn marked wrong with no target is still a usable negative, +// and it must not cost more to give than the full answer. +func TestCorrectAPIWithNoTarget(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}}), core) + + if rr.Code != http.StatusSeeOther { + t.Fatalf("status %d, want 303", rr.Code) + } + if !core.called || core.shouldBe != "" { + t.Errorf("called=%v shouldBe=%q, want an untargeted negative recorded", core.called, core.shouldBe) + } + if !strings.Contains(rr.Header().Get("Location"), "c=wrong") { + t.Errorf("redirect %q does not say the turn was marked wrong", rr.Header().Get("Location")) + } +} + +// Free text here would put an unroutable label in the one table V-632 fits +// prototypes from. +func TestCorrectAPIRejectsUnknownTarget(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}, "should_be": {"погода"}}), core) + + if rr.Code != http.StatusBadRequest { + t.Fatalf("status %d, want 400", rr.Code) + } + if core.called { + t.Error("wrote a label for a target that is not one of the seven") + } +} + +func TestCorrectAPINeedsTraceID(t *testing.T) { + for _, form := range []url.Values{{}, {"trace_id": {"0"}}, {"trace_id": {"nope"}}} { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(form), core) + if rr.Code != http.StatusBadRequest { + t.Errorf("form %v: status %d, want 400", form, rr.Code) + } + if core.called { + t.Errorf("form %v: reached the core", form) + } + } +} + +// A write that broke is not a turn that expired, and the two must not read the +// same to the owner deciding whether to correct again. +func TestCorrectAPIReportsFailure(t *testing.T) { + core := &correctCore{err: errors.New("disk is full")} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core) + if rr.Code != http.StatusBadGateway { + t.Fatalf("status %d, want 502", rr.Code) + } +} + +// A trace past the retention bound is gone, and the surface says that. +func TestCorrectAPIExpiredTurn(t *testing.T) { + core := &correctCore{err: ipc.ErrNoSuchTrace} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core) + if rr.Code != http.StatusNotFound { + t.Fatalf("status %d, want 404", rr.Code) + } +} + +func TestCorrectAPIPostOnly(t *testing.T) { + rr := httptest.NewRecorder() + handleCorrectAPI(rr, httptest.NewRequest(http.MethodGet, "/api/correct", nil), &correctCore{}) + if rr.Code != http.StatusMethodNotAllowed { + t.Fatalf("status %d, want 405", rr.Code) + } +} + +// Every one of the seven intents has a button, so a new intent cannot exist with +// no way to correct a turn into it. +func TestCorrectionTargetsAreTheSeven(t *testing.T) { + if len(correctionTargets) != 7 { + t.Fatalf("%d targets, want the seven public intents", len(correctionTargets)) + } + for _, want := range []string{"fact", "note", "reminder", "query", "act", "chat", "system"} { + if !isCorrectionTarget(want) { + t.Errorf("%s is not offered", want) + } + } + if isCorrectionTarget("") { + t.Error("empty is not a target: it is the absence of one, handled separately") + } +} diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index ea913b9..15fa0d9 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -210,6 +210,13 @@ func main() { mux.HandleFunc("/routines", gatedPage(handleRoutines)) mux.HandleFunc("/api/chat", gatedPage(handleChatAPI)) mux.HandleFunc("/api/revert", gatedPage(handleRevert)) + // POST /api/correct is deliberately NOT on the step-up list (V-630). It + // reaches no router, no model and no act path: it writes one label row that + // nothing executes from. A correction that costs a passkey tap is a + // correction the owner does not make, and then the table stays empty. + mux.HandleFunc("/api/correct", func(w http.ResponseWriter, r *http.Request) { + handleCorrectAPI(w, r, core) + }) mux.HandleFunc("/models", func(w http.ResponseWriter, r *http.Request) { handleModels(w, r, core, swapConn, stepUpSession, *requireStepUp) }) diff --git a/cmd/mavweb/static/ui.css b/cmd/mavweb/static/ui.css index ee1eb0b..d7029c1 100644 --- a/cmd/mavweb/static/ui.css +++ b/cmd/mavweb/static/ui.css @@ -702,6 +702,11 @@ details[open] > summary { margin-bottom: var(--space-1); } .chat-form { display: flex; gap: var(--space-2); } .chat-form input { flex: 1; } .chat-scroll { max-height: 60vh; overflow-y: auto; margin-bottom: var(--space-4); } +/* The correction gesture (V-630). Wraps on a phone rather than scrolling: it is + one row of small buttons, and a gesture that has to be panned to is not one. */ +.chat-correct { display: flex; flex-wrap: wrap; align-items: center; gap: var(--space-1); + padding: 0 var(--space-3) var(--space-2); margin-top: calc(-1 * var(--space-1)); margin-bottom: var(--space-2); } +.chat-correct-label { font-size: var(--fs-xs); color: var(--text-machine); margin-left: var(--space-2); } /* ── Key-value grid ── */ .kv { display: grid; grid-template-columns: auto 1fr; gap: var(--space-1) var(--space-3); font-size: var(--fs-sm); } From eb129c2fada168bc0ecf54ae7ed3ab4f521cad4b Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:46:22 +0400 Subject: [PATCH 4/7] the correction, written down (V-630) --- docs/plans/22-correcting-a-turn.md | 60 ++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 docs/plans/22-correcting-a-turn.md diff --git a/docs/plans/22-correcting-a-turn.md b/docs/plans/22-correcting-a-turn.md new file mode 100644 index 0000000..6ac9eee --- /dev/null +++ b/docs/plans/22-correcting-a-turn.md @@ -0,0 +1,60 @@ +# Correcting a turn + +Last verified: 06-08-2026 @ 0d5bd0a + +V-630, under V-628. Reads with `21-persisting-the-routing-trace.md`. + +## Why a gesture and not a form + +The routing trace (V-629) stores every turn. Almost all of them routed correctly, so +almost all of them teach nothing. A correction is the only high-value supervised signal +the box produces. It is also the only one that costs the owner something to give. + +So the design constraint is the cost, not the schema. One gesture beside the reply. No +form, no separate page, no passkey tap. + +## Two things to capture, and only one of them is required + +A correction has two halves. + +- This turn was wrong. +- It should have been *this*. + +The second is worth much more. It names which boundary moved, and it is what a fitted +head trains against. But requiring it would price out the first, and a turn marked wrong +with no target is still a usable negative. So the target is optional. The trace carries +`wrong` when he did not say. + +The target is one of the seven intents and never free text. V-632 fits prototypes from +that table. An unroutable label would enter it, and a label nothing can score is worse +than no label. + +## Where the label lives + +`routing_labels`, migration #24, keyed unique on the utterance. A second correction of +the same sentence replaces the first, because his later answer is the one he meant. + +It is a separate table from `routing_traces` on purpose. The transcript expires after 14 +days. The label does not. A label is a sentence, an intent and an encoder id. That is not +a transcript, and the reversal in doc 21 rests on the distinction. + +`was` is stored beside `should_be`. The pair is what names the confusion. A label with no +`was` cannot say which boundary moved. + +## Reach + +`CorrectTurn(traceID, shouldBe)` takes no browser and no session. The trace id rides back +on `ipc.ChatReply` through the same context sink the query source badge uses. Nothing in +the seam assumes the web. + +Only `/chat` offers the gesture today. That is a gap, named rather than closed. If the web +is the only place to correct a turn, the sample skews to whatever the owner types at. Voice +is where the hard cases are. Telegram has the obvious shape, an inline keyboard on the +reply. Voice does not. Inventing a spoken correction grammar would put a recogniser in +front of the one signal that exists to fix recognisers. Both are follow-on work. + +## What is not decided + +Whether the owner ever wants to see the labels he gave. Nothing reads the table outward +yet. `/trace` shows the ring, which is 25 turns and in memory, and a labels view is a +different page with a different question. From 8f168ab8111eb2f4beb8e2e710e5a8e5bd61603c Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:46:39 +0400 Subject: [PATCH 5/7] the routing trace section names the correction gesture (V-630) --- CLAUDE.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 14cf634..f65026b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -296,11 +296,18 @@ beside it, writing `routing_traces` (migration #23). The utterance is stored in clear, because a 384-dimension vector of a short sentence is substantially recoverable and storing vectors instead would be a privacy claim we cannot support. What makes it safe is the same thing that makes the fact store safe. -Retention is 14 days, enforced on write. Nothing reads it outward, and the rule +Retention is 14 days, enforced on write and again on start, so a box that goes +quiet does not keep every row. Nothing reads it outward, and the rule that his notes and facts are never search input covers this table. `Store.Wipe` deletes it with everything else. A correction (V-630) is promoted out into a -seed-shaped row and kept, because a label is not a transcript. The transcript -still expires. Adding a rung to the ladder +seed-shaped row in `routing_labels` (migration #24) and kept, because a label is +not a transcript. The transcript still expires. The gesture that writes one is +two buttons beside the reply on `/chat`, reached over `ipc.CorrectTurn` and the +trace id that now rides back on `ipc.ChatReply`. A turn marked wrong with no +target is a usable negative, so naming the intent is never required. The target +is one of the seven intents and never free text. Only `/chat` offers it: the wire +op assumes no browser, but telegram and voice do not call it yet, and +`docs/plans/22-correcting-a-turn.md` says why voice is the hard one. Adding a rung to the ladder in `runTurn` means adding its name to `preRouteLadder` in `cmd/mavend/decisiontrace.go`, or that rung is silently missing from the record. From a4b47337679552fd04a612699a359e7632aa490f Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:48:29 +0400 Subject: [PATCH 6/7] the correction gesture is step-up gated after all (V-630) Trace ids are sequential integers and the label table is the one thing the routing heads will be fitted on, so an ungated POST let anyone past the transport gate mislabel turns the owner never touched. The cost argument for leaving it open does not hold: he tapped to send the turn he is correcting, so the session is already up when the buttons appear. --- cmd/mavweb/chat.go | 14 +++++++++----- cmd/mavweb/correct_test.go | 28 +++++++++++++++++++++------- cmd/mavweb/main.go | 8 +------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/cmd/mavweb/chat.go b/cmd/mavweb/chat.go index e78a9ff..6091f5a 100644 --- a/cmd/mavweb/chat.go +++ b/cmd/mavweb/chat.go @@ -131,11 +131,12 @@ func handleChatAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, ses // optionally the intent it should have been. An unstated target is accepted, // because a turn marked wrong with no target is still a usable negative. // -// Not step-up gated, unlike POST /api/chat. Writing a label reaches no router, -// no model and no act path; it writes one row nothing executes from. Gating it -// would price the gesture out of being used, which is the one thing that makes -// it worthless. -func handleCorrectAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { +// Step-up gated like POST /api/chat, and that costs the gesture nothing: he +// tapped to send the turn he is now correcting, so the session is already up. +// It is gated because trace ids are sequential integers and this writes the one +// table the routing heads (V-546) will be fitted on. A caller who can guess an +// id could otherwise mislabel turns he never corrected. +func handleCorrectAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, session *webauthn.PasskeySession, requireStepUp bool) { if r.Method != http.MethodPost { http.Error(w, "POST only", http.StatusMethodNotAllowed) return @@ -143,6 +144,9 @@ func handleCorrectAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) if !requireCore(w, core, "correct") { return } + if !stepUpGate(w, session, requireStepUp) { + return + } id, err := strconv.ParseInt(strings.TrimSpace(r.FormValue("trace_id")), 10, 64) if err != nil || id <= 0 { http.Error(w, "trace_id required", http.StatusBadRequest) diff --git a/cmd/mavweb/correct_test.go b/cmd/mavweb/correct_test.go index 20ecd59..ef33f2d 100644 --- a/cmd/mavweb/correct_test.go +++ b/cmd/mavweb/correct_test.go @@ -38,7 +38,7 @@ func TestCorrectAPIWithTarget(t *testing.T) { rr := httptest.NewRecorder() handleCorrectAPI(rr, postCorrect(url.Values{ "trace_id": {"42"}, "should_be": {"fact"}, "q": {"поужинал"}, "rep": {"поняла"}, - }), core) + }), core, stepUpSession(), false) if rr.Code != http.StatusSeeOther { t.Fatalf("status %d, want 303; body=%s", rr.Code, rr.Body.String()) @@ -58,7 +58,7 @@ func TestCorrectAPIWithTarget(t *testing.T) { func TestCorrectAPIWithNoTarget(t *testing.T) { core := &correctCore{} rr := httptest.NewRecorder() - handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}}), core) + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}}), core, stepUpSession(), false) if rr.Code != http.StatusSeeOther { t.Fatalf("status %d, want 303", rr.Code) @@ -76,7 +76,7 @@ func TestCorrectAPIWithNoTarget(t *testing.T) { func TestCorrectAPIRejectsUnknownTarget(t *testing.T) { core := &correctCore{} rr := httptest.NewRecorder() - handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}, "should_be": {"погода"}}), core) + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}, "should_be": {"погода"}}), core, stepUpSession(), false) if rr.Code != http.StatusBadRequest { t.Fatalf("status %d, want 400", rr.Code) @@ -90,7 +90,7 @@ func TestCorrectAPINeedsTraceID(t *testing.T) { for _, form := range []url.Values{{}, {"trace_id": {"0"}}, {"trace_id": {"nope"}}} { core := &correctCore{} rr := httptest.NewRecorder() - handleCorrectAPI(rr, postCorrect(form), core) + handleCorrectAPI(rr, postCorrect(form), core, stepUpSession(), false) if rr.Code != http.StatusBadRequest { t.Errorf("form %v: status %d, want 400", form, rr.Code) } @@ -105,7 +105,7 @@ func TestCorrectAPINeedsTraceID(t *testing.T) { func TestCorrectAPIReportsFailure(t *testing.T) { core := &correctCore{err: errors.New("disk is full")} rr := httptest.NewRecorder() - handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core) + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core, stepUpSession(), false) if rr.Code != http.StatusBadGateway { t.Fatalf("status %d, want 502", rr.Code) } @@ -115,7 +115,7 @@ func TestCorrectAPIReportsFailure(t *testing.T) { func TestCorrectAPIExpiredTurn(t *testing.T) { core := &correctCore{err: ipc.ErrNoSuchTrace} rr := httptest.NewRecorder() - handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core) + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core, stepUpSession(), false) if rr.Code != http.StatusNotFound { t.Fatalf("status %d, want 404", rr.Code) } @@ -123,7 +123,7 @@ func TestCorrectAPIExpiredTurn(t *testing.T) { func TestCorrectAPIPostOnly(t *testing.T) { rr := httptest.NewRecorder() - handleCorrectAPI(rr, httptest.NewRequest(http.MethodGet, "/api/correct", nil), &correctCore{}) + handleCorrectAPI(rr, httptest.NewRequest(http.MethodGet, "/api/correct", nil), &correctCore{}, stepUpSession(), false) if rr.Code != http.StatusMethodNotAllowed { t.Fatalf("status %d, want 405", rr.Code) } @@ -144,3 +144,17 @@ func TestCorrectionTargetsAreTheSeven(t *testing.T) { t.Error("empty is not a target: it is the absence of one, handled separately") } } + +// Trace ids are sequential, so a caller who cannot assert step-up must not be +// able to label a turn the owner never corrected. +func TestCorrectAPINeedsStepUp(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core, nil, true) + if rr.Code != http.StatusForbidden { + t.Fatalf("status %d, want 403", rr.Code) + } + if core.called { + t.Error("wrote a label with no step-up") + } +} diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index 15fa0d9..22578cd 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -210,13 +210,7 @@ func main() { mux.HandleFunc("/routines", gatedPage(handleRoutines)) mux.HandleFunc("/api/chat", gatedPage(handleChatAPI)) mux.HandleFunc("/api/revert", gatedPage(handleRevert)) - // POST /api/correct is deliberately NOT on the step-up list (V-630). It - // reaches no router, no model and no act path: it writes one label row that - // nothing executes from. A correction that costs a passkey tap is a - // correction the owner does not make, and then the table stays empty. - mux.HandleFunc("/api/correct", func(w http.ResponseWriter, r *http.Request) { - handleCorrectAPI(w, r, core) - }) + mux.HandleFunc("/api/correct", gatedPage(handleCorrectAPI)) mux.HandleFunc("/models", func(w http.ResponseWriter, r *http.Request) { handleModels(w, r, core, swapConn, stepUpSession, *requireStepUp) }) From 44b8793e2fbe134c48e6d9e4c83c1171724be0c0 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 19:48:40 +0400 Subject: [PATCH 7/7] the plan says the gesture is gated (V-630) --- docs/plans/22-correcting-a-turn.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/plans/22-correcting-a-turn.md b/docs/plans/22-correcting-a-turn.md index 6ac9eee..220dceb 100644 --- a/docs/plans/22-correcting-a-turn.md +++ b/docs/plans/22-correcting-a-turn.md @@ -11,7 +11,11 @@ almost all of them teach nothing. A correction is the only high-value supervised the box produces. It is also the only one that costs the owner something to give. So the design constraint is the cost, not the schema. One gesture beside the reply. No -form, no separate page, no passkey tap. +form and no separate page. + +It is step-up gated like the chat POST beside it, which costs nothing: he tapped to send +the turn he is correcting. It is gated because trace ids are sequential integers, and this +is the one table the routing heads will be fitted on. ## Two things to capture, and only one of them is required