keybinding semantic validation

docs/reference/config/keys-srf.md

@@ -37,6 +37,12 @@ action::ACTION_NAME,key::KEY_STRING[,scope::SCOPE]
 A tab-local binding may not reuse a globally-bound key; zfin refuses
 to start if you create that conflict.
 
+A binding whose `scope` names no tab, or whose `action` isn't one of
+that tab's actions, never fires. It also doesn't fall back to the
+tab's default binding, because any override for a tab replaces all of
+its defaults. zfin prints a warning for each one when the TUI starts,
+and `zfin doctor` reports them with line numbers.
+
 ## Default global bindings
 
 ```srf

src/tui.zig

@@ -106,6 +106,109 @@ pub const Tab = blk: {
     break :blk @Enum(u8, .exhaustive, &names, &values);
 };
 
+// ── keys.srf tab-scoped bindings ──────────────────────────────
+
+/// Why a `keys.srf` tab-scoped binding can never fire.
+pub const ScopedBindingCheck = enum {
+    ok,
+    /// `scope::` names no tab, so the binding lands in a bucket
+    /// nothing ever consults.
+    unknown_scope,
+    /// The tab exists, but `action::` is not one of its `Action` enum
+    /// tags. The dispatcher finds no match and the key does nothing -
+    /// and because any override for a tab replaces its defaults
+    /// wholesale, it does not fall back to a default binding either.
+    unknown_action,
+};
+
+/// The one definition of "is this tab-scoped binding real", derived
+/// from `tab_modules` at comptime. `keybinds.zig` deliberately doesn't
+/// know about tabs, so it can't check this while loading; the TUI
+/// startup path (`warnScopedBindingProblems`) and `zfin doctor` (via
+/// `keybinds.srf_schema.semanticCheck`) both ask here instead.
+///
+/// `scope::global` is not a tab scope; callers handle it before this.
+pub fn checkScopedBinding(scope: []const u8, action: []const u8) ScopedBindingCheck {
+    inline for (std.meta.fields(@TypeOf(tab_modules))) |field| {
+        if (std.mem.eql(u8, field.name, scope)) {
+            const ActionT = @field(tab_modules, field.name).Action;
+            return if (std.meta.stringToEnum(ActionT, action) != null) .ok else .unknown_action;
+        }
+    }
+    return .unknown_scope;
+}
+
+/// "global, portfolio, analysis, ..." - every valid `scope::` value.
+const valid_scope_list: []const u8 = blk: {
+    var s: []const u8 = "global";
+    for (std.meta.fields(@TypeOf(tab_modules))) |f| s = s ++ ", " ++ f.name;
+    break :blk s;
+};
+
+/// Comma-separated `Action` tags for tab `scope`, or null for a scope
+/// that is not a tab. Empty for a tab with no local actions.
+fn tabActionList(scope: []const u8) ?[]const u8 {
+    inline for (std.meta.fields(@TypeOf(tab_modules))) |field| {
+        if (std.mem.eql(u8, field.name, scope)) {
+            const list = comptime blk: {
+                var s: []const u8 = "";
+                for (std.meta.fields(@field(tab_modules, field.name).Action), 0..) |af, i| {
+                    s = s ++ (if (i == 0) "" else ", ") ++ af.name;
+                }
+                break :blk s;
+            };
+            return list;
+        }
+    }
+    return null;
+}
+
+/// The wording for a binding that `checkScopedBinding` rejected.
+/// Shared by the TUI startup warning and `zfin doctor` so the two
+/// cannot describe the same mistake differently.
+pub fn writeScopedBindingProblem(
+    w: *std.Io.Writer,
+    scope: []const u8,
+    action: []const u8,
+    problem: ScopedBindingCheck,
+) std.Io.Writer.Error!void {
+    switch (problem) {
+        .ok => {},
+        .unknown_scope => try w.print(
+            "scope '{s}' is not a tab, so the binding for action '{s}' never fires; valid scopes: {s}",
+            .{ scope, action, valid_scope_list },
+        ),
+        .unknown_action => {
+            const actions = tabActionList(scope) orelse "";
+            if (actions.len == 0) {
+                try w.print("action '{s}' does nothing: the {s} tab has no local actions", .{ action, scope });
+            } else {
+                try w.print(
+                    "action '{s}' is not a {s} tab action, so that key does nothing there; valid: {s}",
+                    .{ action, scope, actions },
+                );
+            }
+        },
+    }
+}
+
+/// Write one `zfin: keys.srf: ...` line per tab-scoped binding in
+/// `keymap` that can never fire. Returns how many were written.
+fn warnScopedBindingProblems(w: *std.Io.Writer, keymap: keybinds.KeyMap) std.Io.Writer.Error!usize {
+    var n: usize = 0;
+    for (keymap.tab_overrides) |to| {
+        for (to.bindings) |b| {
+            const problem = checkScopedBinding(to.scope, b.action_name);
+            if (problem == .ok) continue;
+            try w.writeAll("zfin: keys.srf: ");
+            try writeScopedBindingProblem(w, to.scope, b.action_name, problem);
+            try w.writeAll("\n");
+            n += 1;
+        }
+    }
+    return n;
+}
+
 /// Comptime lookup table of tab-bar display labels, indexed by
 /// `@intFromEnum(tab)`. Each entry is `" {N}:{label} "` composed
 /// from the 1-indexed registry position + the tab module's
@@ -946,9 +1049,11 @@ pub const App = struct {
                                 }
                             }
                             // Action name didn't resolve; treat as
-                            // unbound (silent - keys.srf parsing
-                            // already validated the action exists,
-                            // future work).
+                            // unbound. The loader can't validate
+                            // tab-scoped action names, so this is
+                            // reported at startup by
+                            // `warnScopedBindingProblems` and by
+                            // `zfin doctor`, not here per keypress.
                             return false;
                         }
                     }
@@ -2574,14 +2679,18 @@ pub fn run(
     defer keymap.deinit();
 
     // Surface per-record parse warnings (unknown action, malformed
-    // key, etc.) to stderr. Non-fatal - the keymap is otherwise
-    // usable; user just sees that some lines didn't take effect.
-    if (keymap.warnings.len > 0) {
+    // key, etc.) to stderr, plus tab-scoped bindings that name no tab
+    // or no action of that tab - the loader can't check those because
+    // it doesn't know about tab modules. Non-fatal - the keymap is
+    // otherwise usable; user just sees that some lines didn't take
+    // effect.
+    {
         var stderr_buf: [4096]u8 = undefined;
         var stderr_writer = std.Io.File.stderr().writer(io, &stderr_buf);
         for (keymap.warnings) |w| {
             try stderr_writer.interface.print("zfin: {s}\n", .{w});
         }
+        _ = try warnScopedBindingProblems(&stderr_writer.interface, keymap);
         try stderr_writer.interface.flush();
     }
 
@@ -3381,3 +3490,116 @@ test "toggleOverlay: same-sym match is case-insensitive" {
     app.toggleOverlay("vti");
     try testing.expectEqualStrings("", app.overlay_symbol);
 }
+
+// ── checkScopedBinding / warnScopedBindingProblems ──
+
+test "checkScopedBinding: real tab and action is ok" {
+    try testing.expectEqual(ScopedBindingCheck.ok, checkScopedBinding("options", "expand_collapse"));
+    try testing.expectEqual(ScopedBindingCheck.ok, checkScopedBinding("portfolio", "sort_reverse"));
+}
+
+test "checkScopedBinding: misspelled scope and misspelled action" {
+    try testing.expectEqual(ScopedBindingCheck.unknown_scope, checkScopedBinding("optons", "expand_collapse"));
+    try testing.expectEqual(ScopedBindingCheck.unknown_action, checkScopedBinding("options", "expand_colapse"));
+    // An action that exists, but on a different tab.
+    try testing.expectEqual(ScopedBindingCheck.unknown_action, checkScopedBinding("options", "sort_reverse"));
+}
+
+test "checkScopedBinding: every registered tab is a valid scope" {
+    // Derived from `tab_modules`, so a new tab can't be forgotten.
+    inline for (std.meta.fields(@TypeOf(tab_modules))) |f| {
+        try testing.expect(checkScopedBinding(f.name, "__no_such_action__") != .unknown_scope);
+    }
+}
+
+test "writeScopedBindingProblem: wording lists the valid choices" {
+    var buf: [512]u8 = undefined;
+    {
+        var w = std.Io.Writer.fixed(&buf);
+        try writeScopedBindingProblem(&w, "optons", "expand_collapse", .unknown_scope);
+        const msg = w.buffered();
+        try testing.expect(std.mem.indexOf(u8, msg, "scope 'optons' is not a tab") != null);
+        try testing.expect(std.mem.indexOf(u8, msg, "valid scopes: global, portfolio,") != null);
+        try testing.expect(std.mem.indexOf(u8, msg, "options") != null);
+    }
+    {
+        var w = std.Io.Writer.fixed(&buf);
+        try writeScopedBindingProblem(&w, "options", "expand_colapse", .unknown_action);
+        const msg = w.buffered();
+        try testing.expect(std.mem.startsWith(u8, msg, "action 'expand_colapse' is not a options tab action, so that key does nothing there; valid: "));
+        // Every one of the tab's actions is offered - derived from the
+        // enum here too, not a hand-copied list.
+        inline for (std.meta.fields(@field(tab_modules, "options").Action)) |af| {
+            try testing.expect(std.mem.indexOf(u8, msg, af.name) != null);
+        }
+    }
+    // A tab with no local actions at all gets a message that says so,
+    // rather than an empty "valid:" list. Found from the registry rather
+    // than named, so the test can't go stale when a tab gains actions.
+    var checked_empty = false;
+    inline for (std.meta.fields(@TypeOf(tab_modules))) |f| {
+        if (comptime std.meta.fields(@field(tab_modules, f.name).Action).len == 0) {
+            var w = std.Io.Writer.fixed(&buf);
+            try writeScopedBindingProblem(&w, f.name, "anything", .unknown_action);
+            try testing.expectEqualStrings(
+                "action 'anything' does nothing: the " ++ f.name ++ " tab has no local actions",
+                w.buffered(),
+            );
+            checked_empty = true;
+        }
+    }
+    try testing.expect(checked_empty);
+}
+
+test "warnScopedBindingProblems: reports only the bindings that can't fire" {
+    const km: keybinds.KeyMap = .{
+        .bindings = &.{},
+        .tab_overrides = &.{
+            .{ .scope = "options", .bindings = &.{
+                .{ .action_name = "expand_collapse", .key = .{ .codepoint = 'x' } },
+                .{ .action_name = "expand_colapse", .key = .{ .codepoint = 'y' } },
+            } },
+            .{ .scope = "optons", .bindings = &.{
+                .{ .action_name = "expand_collapse", .key = .{ .codepoint = 'z' } },
+            } },
+        },
+    };
+    var buf: [2048]u8 = undefined;
+    var w = std.Io.Writer.fixed(&buf);
+    try testing.expectEqual(@as(usize, 2), try warnScopedBindingProblems(&w, km));
+    const out = w.buffered();
+    try testing.expect(std.mem.indexOf(u8, out, "zfin: keys.srf: action 'expand_colapse'") != null);
+    try testing.expect(std.mem.indexOf(u8, out, "zfin: keys.srf: scope 'optons'") != null);
+    // The valid binding is not mentioned.
+    try testing.expect(std.mem.indexOf(u8, out, "'expand_collapse' is not") == null);
+}
+
+test "warnScopedBindingProblems: the built-in defaults are clean" {
+    var buf: [256]u8 = undefined;
+    var w = std.Io.Writer.fixed(&buf);
+    try testing.expectEqual(@as(usize, 0), try warnScopedBindingProblems(&w, keybinds.defaults()));
+}
+
+test "writeScopedBindingProblem: every message fits in a lint finding" {
+    // `srf_lint.Finding.describe` truncates at `describe_max`, and these
+    // messages list the valid choices. A tab that grows enough actions
+    // would silently clip its list in `zfin doctor` - this fails first.
+    const srf_lint = @import("srf_lint.zig");
+    // Generous stand-ins for a user's typo.
+    const long_scope = "a_long_misspelled_scope_name_xx";
+    const long_action = "a_long_misspelled_action_name_xxxxxxxx";
+    var buf: [4096]u8 = undefined;
+    {
+        var w = std.Io.Writer.fixed(&buf);
+        try writeScopedBindingProblem(&w, long_scope, long_action, .unknown_scope);
+        try testing.expect(w.buffered().len <= srf_lint.Finding.describe_max);
+    }
+    inline for (std.meta.fields(@TypeOf(tab_modules))) |f| {
+        var w = std.Io.Writer.fixed(&buf);
+        try writeScopedBindingProblem(&w, f.name, long_action, .unknown_action);
+        if (w.buffered().len > srf_lint.Finding.describe_max) {
+            std.debug.print("{s} tab message is {d} bytes, describe_max is {d}\n", .{ f.name, w.buffered().len, srf_lint.Finding.describe_max });
+            return error.MessageTooLong;
+        }
+    }
+}

src/tui/keybinds.zig

@@ -2,6 +2,11 @@ const std = @import("std");
 const vaxis = @import("vaxis");
 const srf = @import("srf");
 const srf_opts = @import("../srf_opts.zig");
+const srf_lint = @import("../srf_lint.zig");
+// Cycle-safe: `tui.zig` imports this file too, and Zig resolves the
+// loop lazily. Needed only by `srf_schema.semanticCheck`, which asks
+// `tui.checkScopedBinding` because only `tui.zig` knows the tab set.
+const tui = @import("../tui.zig");
 
 pub const Action = enum {
     quit,
@@ -432,6 +437,34 @@ pub const srf_schema = struct {
     pub const Record = RawRecord;
     pub const file_label: []const u8 = "keys.srf";
     pub const doc_path: []const u8 = "reference/config/keys-srf.md";
+
+    /// Bindings that load but can never fire. The TUI prints the same
+    /// problems at startup; this puts them in `zfin doctor` with a line
+    /// number. The tab-scoped rule and its wording live in `tui.zig`
+    /// (`checkScopedBinding`, `writeScopedBindingProblem`), so there is
+    /// one definition for both.
+    pub fn semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void {
+        _ = ctx;
+        const is_global = rec.scope == null or std.mem.eql(u8, rec.scope.?, "global");
+        if (is_global) {
+            // Same test the loader uses to skip the record.
+            if (parseAction(rec.action) == null) {
+                try sink.addOwned(.semantic, "action", try std.fmt.allocPrint(
+                    sink.allocator,
+                    "unknown global action '{s}'; binding skipped (`zfin interactive --default-keys` lists them)",
+                    .{rec.action},
+                ));
+            }
+            return;
+        }
+        const scope = rec.scope.?;
+        const problem = tui.checkScopedBinding(scope, rec.action);
+        if (problem == .ok) return;
+        var aw: std.Io.Writer.Allocating = .init(sink.allocator);
+        errdefer aw.deinit();
+        try tui.writeScopedBindingProblem(&aw.writer, scope, rec.action, problem);
+        try sink.addOwned(.semantic, if (problem == .unknown_scope) "scope" else "action", try aw.toOwnedSlice());
+    }
 };
 
 /// Load keybindings from an SRF file. Returns null if the file doesn't exist
@@ -792,3 +825,44 @@ test "printSectionHeader writes a comment header line" {
     try printSectionHeader(&aw.writer, "Tab: portfolio");
     try std.testing.expectEqualStrings("\n# ── Tab: portfolio ──\n", aw.written());
 }
+
+// ── srf_schema.semanticCheck ──
+
+fn lintKeys(data: []const u8) ![]u8 {
+    const allocator = std.testing.allocator;
+    var r = try srf_lint.check(allocator, data, srf_lint.shapeOfSchema(srf_schema));
+    defer r.deinit();
+    try srf_lint.checkSemantic(srf_schema, &r, data, .{ .today = @import("../Date.zig").fromYmd(2026, 1, 1) });
+    r.sort();
+    var aw: std.Io.Writer.Allocating = .init(allocator);
+    errdefer aw.deinit();
+    var buf: [srf_lint.Finding.describe_max]u8 = undefined;
+    for (r.findings) |f| try aw.writer.print("{d}: {s}\n", .{ f.first_line, f.describe(&buf) });
+    return aw.toOwnedSlice();
+}
+
+test "srf_schema.semanticCheck: valid global and scoped bindings are clean" {
+    const got = try lintKeys(
+        \\#!srfv1
+        \\action::quit,key::q
+        \\action::expand_collapse,key::x,scope::options
+        \\action::refresh,key::F5,scope::global
+        \\
+    );
+    defer std.testing.allocator.free(got);
+    try std.testing.expectEqualStrings("", got);
+}
+
+test "srf_schema.semanticCheck: misspelled scope, scoped action, and global action" {
+    const got = try lintKeys(
+        \\#!srfv1
+        \\action::expand_collapse,key::x,scope::optons
+        \\action::expand_colapse,key::y,scope::options
+        \\action::quitt,key::Q
+        \\
+    );
+    defer std.testing.allocator.free(got);
+    try std.testing.expect(std.mem.indexOf(u8, got, "2: scope 'optons' is not a tab") != null);
+    try std.testing.expect(std.mem.indexOf(u8, got, "3: action 'expand_colapse' is not a options tab action") != null);
+    try std.testing.expect(std.mem.indexOf(u8, got, "4: unknown global action 'quitt'") != null);
+}