Merge branch 'fix/g08' into fix/integrated
# Conflicts: # internal/store/migrations.go
This commit is contained in:
kami
@@ -200,6 +200,14 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2
|
||||
);
|
||||
CREATE INDEX IF NOT EXISTS idx_eco_traces_ts ON ecosystem_traces (ts DESC);
|
||||
CREATE INDEX IF NOT EXISTS idx_eco_traces_correlation ON ecosystem_traces (correlation_id);`,
|
||||
`ALTER TABLE tools ADD COLUMN fingerprint TEXT NOT NULL DEFAULT '';`,
|
||||
// #17 — what a discovered tool WAS when it was approved (Vikunja #251).
|
||||
// An MCP row's cmd is ["mcp", server, tool], which is a late-bound
|
||||
// reference: it names a tool on a server the remote end owns and it pins
|
||||
// no behaviour at all. A server upgraded, or taken over, can redefine
|
||||
// list_tasks into something that writes without the row changing by one
|
||||
// byte. The fingerprint is the declared shape at approval time, so a
|
||||
// redefinition is a re-approval instead of a silent upgrade.
|
||||
}
|
||||
|
||||
// migrate applies every migration with a number greater than the DB's current
|
||||
|
||||
+118
-5
@@ -65,7 +65,16 @@ func (s *Store) ProposeTool(ctx context.Context, name, utterance, scope string,
|
||||
// Like ProposeTool it never touches an existing row, so re-discovery on every
|
||||
// restart is idempotent and cannot silently re-arm a tool that was disabled or
|
||||
// change the cmd of one already enabled.
|
||||
func (s *Store) ProposeMCPTool(ctx context.Context, name, scope string, cmd []string, destructive bool, utterance string, ts time.Time) (bool, error) {
|
||||
//
|
||||
// The row is NOT what protects him, and it is worth being exact about that.
|
||||
// cmd is ["mcp", server, tool]: a late-bound reference to a name the remote
|
||||
// server owns. The tool it points at can be redefined on the far end without
|
||||
// the row changing at all, so "the cmd cannot change" is true and beside the
|
||||
// point. fingerprint is what closes that: it records the declared shape (name,
|
||||
// description, input schema, readOnlyHint) at the time the proposal was
|
||||
// written, and ReconcileMCPTool compares against it on every later discovery.
|
||||
// Pass "" for a row with nothing to fingerprint (a Home Assistant device).
|
||||
func (s *Store) ProposeMCPTool(ctx context.Context, name, scope string, cmd []string, destructive bool, utterance, fingerprint string, ts time.Time) (bool, error) {
|
||||
if len(cmd) == 0 {
|
||||
return false, ErrToolCmd
|
||||
}
|
||||
@@ -81,10 +90,10 @@ func (s *Store) ProposeMCPTool(ctx context.Context, name, scope string, cmd []st
|
||||
d = 1
|
||||
}
|
||||
res, err := s.db.ExecContext(ctx, `
|
||||
INSERT INTO tools (name, scope, cmd, destructive, status, utterance, created_ts, updated_ts)
|
||||
VALUES (?, ?, ?, ?, 'proposed', ?, ?, ?)
|
||||
INSERT INTO tools (name, scope, cmd, destructive, status, utterance, fingerprint, created_ts, updated_ts)
|
||||
VALUES (?, ?, ?, ?, 'proposed', ?, ?, ?, ?)
|
||||
ON CONFLICT(name) DO NOTHING`,
|
||||
name, scope, string(raw), d, utterance, ts.UnixMilli(), ts.UnixMilli())
|
||||
name, scope, string(raw), d, utterance, fingerprint, ts.UnixMilli(), ts.UnixMilli())
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("propose mcp tool: %w", err)
|
||||
}
|
||||
@@ -105,7 +114,111 @@ func (s *Store) ProposeMCPTool(ctx context.Context, name, scope string, cmd []st
|
||||
// turn. Re-discovery on every refresh is idempotent — an existing row is never
|
||||
// touched, so a device he disabled stays disabled.
|
||||
func (s *Store) ProposeSmartHomeTool(ctx context.Context, name, scope string, cmd []string, utterance string, ts time.Time) (bool, error) {
|
||||
return s.ProposeMCPTool(ctx, name, scope, cmd, true, utterance, ts)
|
||||
return s.ProposeMCPTool(ctx, name, scope, cmd, true, utterance, "", ts)
|
||||
}
|
||||
|
||||
// ToolChange — what ReconcileMCPTool did to an existing row.
|
||||
type ToolChange struct {
|
||||
// Changed — the discovered shape differs from the approved one.
|
||||
Changed bool
|
||||
// Demoted — the row was enabled and is now 'proposed' again, so the
|
||||
// capability is off until a human looks at it a second time.
|
||||
Demoted bool
|
||||
// Escalated — destructive went from 0 to 1. It never goes the other way.
|
||||
Escalated bool
|
||||
}
|
||||
|
||||
// ReconcileMCPTool compares a freshly discovered tool against the row that was
|
||||
// approved, and escalates when they disagree.
|
||||
//
|
||||
// The failure this exists for: day 1 the server offers list_tasks with
|
||||
// readOnlyHint true, so the row is proposed non-destructive and Kami enables
|
||||
// it. Day 30 the server is upgraded, or taken over, and list_tasks now writes.
|
||||
// Insert-or-skip does nothing on that discovery — the row is still enabled,
|
||||
// still destructive=0 — and the confirm turn never fires, because the flag was
|
||||
// frozen against a claim the server has since withdrawn.
|
||||
//
|
||||
// So: a differing fingerprint drops the row back to 'proposed' and rewrites the
|
||||
// provenance, and a tool that stopped claiming read-only gets destructive=1.
|
||||
// destructive is only ever raised, never lowered: relaxing it on the say-so of
|
||||
// the same server that changed underneath us would undo the point.
|
||||
//
|
||||
// A row with an empty stored fingerprint predates this and simply adopts the
|
||||
// discovered one — an upgrade is not a redefinition.
|
||||
func (s *Store) ReconcileMCPTool(ctx context.Context, name, fingerprint string, destructive bool, utterance string, ts time.Time) (ToolChange, error) {
|
||||
var (
|
||||
stored string
|
||||
status string
|
||||
wasDest int
|
||||
)
|
||||
err := s.db.QueryRowContext(ctx,
|
||||
`SELECT fingerprint, status, destructive FROM tools WHERE name = ?`, name).
|
||||
Scan(&stored, &status, &wasDest)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return ToolChange{}, ErrToolNotFound
|
||||
}
|
||||
if err != nil {
|
||||
return ToolChange{}, fmt.Errorf("reconcile mcp tool: %w", err)
|
||||
}
|
||||
var ch ToolChange
|
||||
if stored == "" {
|
||||
if _, err := s.db.ExecContext(ctx,
|
||||
`UPDATE tools SET fingerprint = ?, updated_ts = ? WHERE name = ?`,
|
||||
fingerprint, ts.UnixMilli(), name); err != nil {
|
||||
return ToolChange{}, fmt.Errorf("reconcile mcp tool: %w", err)
|
||||
}
|
||||
return ch, nil
|
||||
}
|
||||
if stored == fingerprint {
|
||||
return ch, nil
|
||||
}
|
||||
ch.Changed = true
|
||||
ch.Demoted = status == "enabled"
|
||||
d := wasDest
|
||||
if destructive && wasDest == 0 {
|
||||
d, ch.Escalated = 1, true
|
||||
}
|
||||
if _, err := s.db.ExecContext(ctx, `
|
||||
UPDATE tools
|
||||
SET fingerprint = ?, destructive = ?, status = 'proposed', utterance = ?, updated_ts = ?
|
||||
WHERE name = ?`,
|
||||
fingerprint, d, utterance, ts.UnixMilli(), name); err != nil {
|
||||
return ToolChange{}, fmt.Errorf("reconcile mcp tool: %w", err)
|
||||
}
|
||||
return ch, nil
|
||||
}
|
||||
|
||||
// WithdrawTool disarms a row whose remote tool no longer exists: it drops back
|
||||
// to 'proposed' and its provenance says why.
|
||||
//
|
||||
// Nothing else retracted a proposal, so a tool a server stopped offering kept
|
||||
// its row forever, and an ENABLED one stayed enabled and failed at call time
|
||||
// with an internal string the act path does not match. /tools is where he would
|
||||
// go to find out and it was the one place that did not say. Returns whether the
|
||||
// row was still enabled.
|
||||
func (s *Store) WithdrawTool(ctx context.Context, name, utterance string, ts time.Time) (bool, error) {
|
||||
res, err := s.db.ExecContext(ctx, `
|
||||
UPDATE tools SET status = 'proposed', utterance = ?, updated_ts = ?
|
||||
WHERE name = ? AND status = 'enabled'`,
|
||||
utterance, ts.UnixMilli(), name)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("withdraw tool: %w", err)
|
||||
}
|
||||
n, err := res.RowsAffected()
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("withdraw tool: rows affected: %w", err)
|
||||
}
|
||||
if n > 0 {
|
||||
return true, nil
|
||||
}
|
||||
// Not enabled: still refresh the provenance so the proposed row says it.
|
||||
_, err = s.db.ExecContext(ctx,
|
||||
`UPDATE tools SET utterance = ?, updated_ts = ? WHERE name = ?`,
|
||||
utterance, ts.UnixMilli(), name)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("withdraw tool: %w", err)
|
||||
}
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// EnableTool fills cmd + destructive and flips status to 'enabled'. This is the
|
||||
|
||||
@@ -70,7 +70,7 @@ func TestProposeMCPTool(t *testing.T) {
|
||||
now := time.Now()
|
||||
cmd := []string{"mcp", "vikunja", "list_tasks"}
|
||||
|
||||
fresh, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "mcp vikunja/list_tasks: List tasks", now)
|
||||
fresh, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "mcp vikunja/list_tasks: List tasks", "fp1", now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -92,7 +92,7 @@ func TestProposeMCPTool(t *testing.T) {
|
||||
}
|
||||
|
||||
// Re-discovery on the next boot is idempotent.
|
||||
fresh, err = s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, true, "changed", now)
|
||||
fresh, err = s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, true, "changed", "fp1", now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -104,7 +104,7 @@ func TestProposeMCPTool(t *testing.T) {
|
||||
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", []string{"mcp", "vikunja", "delete_task"}, true, "x", now); err != nil {
|
||||
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", []string{"mcp", "vikunja", "delete_task"}, true, "x", "fp2", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got, err = s.LookupTool(ctx, "vikunja_list_tasks")
|
||||
@@ -118,7 +118,142 @@ func TestProposeMCPTool(t *testing.T) {
|
||||
|
||||
func TestProposeMCPToolNeedsCmd(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
if _, err := s.ProposeMCPTool(context.Background(), "x", "mcp:y", nil, false, "", time.Now()); !errors.Is(err, ErrToolCmd) {
|
||||
if _, err := s.ProposeMCPTool(context.Background(), "x", "mcp:y", nil, false, "", "", time.Now()); !errors.Is(err, ErrToolCmd) {
|
||||
t.Fatalf("err = %v, want ErrToolCmd", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A server that redefines a tool Kami already approved must have to ask again.
|
||||
// The row stores cmd ["mcp", server, tool], a late-bound reference to a name
|
||||
// the far end owns, so before the fingerprint a server could turn an enabled
|
||||
// read-only list_tasks into something that writes and Maven would keep running
|
||||
// it without a confirm turn.
|
||||
func TestReconcileMCPToolDemotesARedefinedTool(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now()
|
||||
cmd := []string{"mcp", "vikunja", "list_tasks"}
|
||||
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "read only", "fp1", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// Same shape ⇒ nothing happens. Discovery runs every minute and must be
|
||||
// idempotent.
|
||||
ch, err := s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp1", false, "read only", now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if ch.Changed {
|
||||
t.Fatalf("an unchanged tool must not be touched: %+v", ch)
|
||||
}
|
||||
if got, _ := s.LookupTool(ctx, "vikunja_list_tasks"); got.Status != "enabled" {
|
||||
t.Fatalf("status = %q, want it left enabled", got.Status)
|
||||
}
|
||||
|
||||
// It stopped claiming read-only and its schema moved.
|
||||
ch, err = s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp2", true, "now writes", now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !ch.Changed || !ch.Demoted || !ch.Escalated {
|
||||
t.Fatalf("change = %+v, want changed+demoted+escalated", ch)
|
||||
}
|
||||
got, err := s.LookupTool(ctx, "vikunja_list_tasks")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.Status != "proposed" {
|
||||
t.Errorf("status = %q, want a redefined tool back in the queue", got.Status)
|
||||
}
|
||||
if !got.Destructive {
|
||||
t.Error("a tool that stopped claiming read-only must gain the confirm turn")
|
||||
}
|
||||
if got.Utterance != "now writes" {
|
||||
t.Errorf("utterance = %q, want what the server says today", got.Utterance)
|
||||
}
|
||||
|
||||
// destructive is only ever raised. The server that changed underneath us
|
||||
// does not get to relax it by claiming read-only next time.
|
||||
if _, err := s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp3", false, "read only again", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got, _ = s.LookupTool(ctx, "vikunja_list_tasks"); !got.Destructive {
|
||||
t.Error("destructive was relaxed by the server")
|
||||
}
|
||||
}
|
||||
|
||||
// A row written before fingerprints exist simply adopts one. An upgrade is not
|
||||
// a redefinition and must not disable everything Kami approved.
|
||||
func TestReconcileMCPToolAdoptsAnEmptyFingerprint(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now()
|
||||
cmd := []string{"mcp", "vikunja", "list_tasks"}
|
||||
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "x", "", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ch, err := s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp1", false, "x", now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if ch.Changed {
|
||||
t.Fatalf("adopting must be silent: %+v", ch)
|
||||
}
|
||||
if got, _ := s.LookupTool(ctx, "vikunja_list_tasks"); got.Status != "enabled" {
|
||||
t.Fatalf("status = %q, want still enabled after the upgrade", got.Status)
|
||||
}
|
||||
// And now it is pinned.
|
||||
if ch, _ = s.ReconcileMCPTool(ctx, "vikunja_list_tasks", "fp2", false, "y", now); !ch.Changed {
|
||||
t.Fatal("the adopted fingerprint must be enforced on the next pass")
|
||||
}
|
||||
}
|
||||
|
||||
func TestReconcileMCPToolUnknownRow(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
if _, err := s.ReconcileMCPTool(context.Background(), "nope", "fp", false, "", time.Now()); !errors.Is(err, ErrToolNotFound) {
|
||||
t.Fatalf("err = %v, want ErrToolNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A tool the server stopped offering must be disarmed and must say why. It used
|
||||
// to stay enabled and fail at call time with an internal string, and /tools —
|
||||
// the one place he would look — did not mention it.
|
||||
func TestWithdrawTool(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now()
|
||||
cmd := []string{"mcp", "vikunja", "list_tasks"}
|
||||
if _, err := s.ProposeMCPTool(ctx, "vikunja_list_tasks", "mcp:vikunja", cmd, false, "x", "fp1", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := s.EnableTool(ctx, "vikunja_list_tasks", cmd, false, "mcp:vikunja", now); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
was, err := s.WithdrawTool(ctx, "vikunja_list_tasks", "gone", now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !was {
|
||||
t.Error("withdrawing an enabled tool must report that it was enabled")
|
||||
}
|
||||
got, err := s.LookupTool(ctx, "vikunja_list_tasks")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.Status != "proposed" || got.Utterance != "gone" {
|
||||
t.Fatalf("row = %+v, want proposed and saying why", got)
|
||||
}
|
||||
// Withdrawing again is not an error and does not claim it was enabled.
|
||||
if was, err = s.WithdrawTool(ctx, "vikunja_list_tasks", "still gone", now); err != nil || was {
|
||||
t.Fatalf("second withdraw = %v, %v", was, err)
|
||||
}
|
||||
if got, _ = s.LookupTool(ctx, "vikunja_list_tasks"); got.Utterance != "still gone" {
|
||||
t.Errorf("utterance = %q, want the provenance refreshed anyway", got.Utterance)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user