add typed IPC command-schema framework (T-120)
test / unit + widget + golden + a11y (push) Failing after 28s
test / integration_test (xvfb) (push) Has been skipped
test / bundle smoke (xvfb 5s) (push) Has been skipped
test / daemon subprocess + web WASM smoke (push) Has been skipped
test / dart doc (lib API) (push) Failing after 23s
test / unit + widget + golden + a11y (push) Failing after 28s
test / integration_test (xvfb) (push) Has been skipped
test / bundle smoke (xvfb 5s) (push) Has been skipped
test / daemon subprocess + web WASM smoke (push) Has been skipped
test / dart doc (lib API) (push) Failing after 23s
Per D-74: commands register an argument schema beside their handler
instead of hand-validating args inline. DaemonDispatcher accumulates a
cmd->schema registry and, before invoking a handler, normalises the
argv-translator shape ({positional, flags}) into named args, coerces
types, and checks per-arg constraints (charset/pattern, leading-dash
rejection, numeric range, list caps). Violations return userError so no
handler sees malformed input. Schema adoption is opt-in per command —
unschema'd commands dispatch unchanged.
panel.resize adopts a schema (dropping the _ResizeArgs hand-lift from
T-119); git.checkout and git.push gain schemas that reject leading-dash
refs at the dispatcher and, via positional ordering, fix the C-client
CLI path — `clide git checkout <branch>` now reaches the handler, where
the positional token previously never mapped to `branch`. The T-104
validateGitRef + count/path caps stay in place as defense-in-depth
because the git client is reachable directly from the UI, not only
through the dispatcher.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import 'package:clide/clide.dart' show clideVersion;
|
||||
import 'package:clide/src/ipc/command_schema.dart';
|
||||
import 'package:clide/src/ipc/envelope.dart';
|
||||
import 'package:clide/src/ipc/schema_v1.dart';
|
||||
|
||||
@@ -12,13 +13,26 @@ class DaemonDispatcher {
|
||||
|
||||
final Map<String, CommandHandler> _handlers = {};
|
||||
|
||||
void register(String cmd, CommandHandler handler) {
|
||||
/// Per-command argument schemas (T-120 / D-74). Populated alongside
|
||||
/// handlers by registrants. A command with no entry dispatches
|
||||
/// unvalidated — schema adoption is opt-in per command.
|
||||
final Map<String, CommandSchema> _schemas = {};
|
||||
|
||||
/// Register [handler] for [cmd]. Pass [schema] to have the dispatcher
|
||||
/// normalise + validate `req.args` before the handler runs (D-74).
|
||||
void register(String cmd, CommandHandler handler, {CommandSchema? schema}) {
|
||||
_handlers[cmd] = handler;
|
||||
if (schema != null) {
|
||||
_schemas[cmd] = schema;
|
||||
} else {
|
||||
_schemas.remove(cmd);
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove all registered handlers except ping/version.
|
||||
void clear() {
|
||||
_handlers.removeWhere((k, _) => k != 'ping' && k != 'version');
|
||||
_schemas.removeWhere((k, _) => k != 'ping' && k != 'version');
|
||||
}
|
||||
|
||||
bool get isEmpty => _handlers.length <= 2; // only ping + version
|
||||
@@ -36,7 +50,13 @@ class DaemonDispatcher {
|
||||
),
|
||||
);
|
||||
}
|
||||
return h(req);
|
||||
final schema = _schemas[req.cmd];
|
||||
if (schema == null) return h(req);
|
||||
final result = schema.validate(schema.normalize(req.args));
|
||||
if (!result.isOk) {
|
||||
return schemaError(req.id, result.error!);
|
||||
}
|
||||
return h(IpcRequest(id: req.id, cmd: req.cmd, args: result.values!));
|
||||
}
|
||||
|
||||
Future<IpcResponse> _ping(IpcRequest req) async => IpcResponse.ok(
|
||||
|
||||
@@ -7,6 +7,7 @@ library;
|
||||
|
||||
import '../git/client.dart';
|
||||
import '../git/operations.dart' show GitException;
|
||||
import '../ipc/command_schema.dart';
|
||||
import '../ipc/envelope.dart';
|
||||
import '../ipc/schema_v1.dart';
|
||||
import '../panes/event_sink.dart';
|
||||
@@ -22,6 +23,30 @@ const int _gitLogMaxCount = 1000;
|
||||
/// arguments.
|
||||
const int _gitPathsMaxCount = 256;
|
||||
|
||||
/// Schema for `git.checkout` (D-74). `positional[0]` → `branch`, so
|
||||
/// `clide git checkout <branch>` from the C client now reaches the
|
||||
/// handler; the leading-dash guard stops argv injection at the
|
||||
/// dispatcher (mirrors [validateGitRef], kept below as
|
||||
/// defense-in-depth since the git client is also UI-reachable).
|
||||
const CommandSchema _checkoutSchema = CommandSchema(
|
||||
positional: ['branch'],
|
||||
args: {
|
||||
'branch': ArgSpec(required: true, rejectLeadingDash: true),
|
||||
},
|
||||
);
|
||||
|
||||
/// Schema for `git.push` (D-74). `clide git push <remote> <branch>`.
|
||||
/// Both refs optional (bare `git.push` is valid); leading-dash
|
||||
/// rejected on each.
|
||||
const CommandSchema _pushSchema = CommandSchema(
|
||||
positional: ['remote', 'branch'],
|
||||
args: {
|
||||
'remote': ArgSpec(rejectLeadingDash: true),
|
||||
'branch': ArgSpec(rejectLeadingDash: true),
|
||||
'setUpstream': ArgSpec(type: ArgType.boolean),
|
||||
},
|
||||
);
|
||||
|
||||
void registerGitCommands(
|
||||
DaemonDispatcher d,
|
||||
GitClient git,
|
||||
@@ -245,7 +270,7 @@ void registerGitCommands(
|
||||
} on GitException catch (e) {
|
||||
return _gitError(req.id, e);
|
||||
}
|
||||
});
|
||||
}, schema: _pushSchema);
|
||||
|
||||
d.register('git.branches', (req) async {
|
||||
try {
|
||||
@@ -279,7 +304,7 @@ void registerGitCommands(
|
||||
} on GitException catch (e) {
|
||||
return _gitError(req.id, e);
|
||||
}
|
||||
});
|
||||
}, schema: _checkoutSchema);
|
||||
}
|
||||
|
||||
List<String> _pathList(Object? raw) {
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
/// `lib/src/daemon/panel_resizer_kernel.dart`.
|
||||
library;
|
||||
|
||||
import '../ipc/command_schema.dart';
|
||||
import '../ipc/envelope.dart';
|
||||
import '../ipc/schema_v1.dart';
|
||||
import 'dispatcher.dart';
|
||||
@@ -47,29 +48,40 @@ abstract class PanelResizer {
|
||||
/// [PanelResizer.bumpEditorRatio] instead of [PanelResizer.setSlotSize].
|
||||
const String editorSplitSlot = 'editor';
|
||||
|
||||
/// Schema for `panel.resize` (D-74). The dispatcher normalises the
|
||||
/// argv shape (`positional[0]` → `slot`, `--to/--by` flags) and
|
||||
/// coerces `to`/`by` to numbers before the handler runs. The
|
||||
/// "exactly one of to/by" rule is cross-argument semantics, so it
|
||||
/// stays in the handler.
|
||||
const CommandSchema _resizeSchema = CommandSchema(
|
||||
positional: ['slot'],
|
||||
args: {
|
||||
'slot': ArgSpec(required: true),
|
||||
'to': ArgSpec(type: ArgType.number),
|
||||
'by': ArgSpec(type: ArgType.number),
|
||||
},
|
||||
);
|
||||
|
||||
void registerPanelCommands(DaemonDispatcher d, PanelResizer resizer) {
|
||||
d.register('panel.resize', (req) => _resize(req, resizer));
|
||||
d.register('panel.resize', (req) => _resize(req, resizer), schema: _resizeSchema);
|
||||
}
|
||||
|
||||
Future<IpcResponse> _resize(IpcRequest req, PanelResizer r) async {
|
||||
final view = _ResizeArgs.from(req.args);
|
||||
if (view.slot == null || view.slot!.isEmpty) {
|
||||
return _userErr(req.id, 'slot is required (e.g. "sidebar", "context", "$editorSplitSlot")');
|
||||
}
|
||||
if (!view.hasTo && !view.hasBy) {
|
||||
// Args are schema-normalised + coerced by the dispatcher: `slot` is a
|
||||
// non-empty string, `to`/`by` are num? when present.
|
||||
final slot = req.args['slot'] as String;
|
||||
final hasTo = req.args['to'] != null;
|
||||
final hasBy = req.args['by'] != null;
|
||||
if (!hasTo && !hasBy) {
|
||||
return _userErr(req.id, 'one of `to` (absolute) or `by` (delta) is required');
|
||||
}
|
||||
if (view.hasTo && view.hasBy) {
|
||||
if (hasTo && hasBy) {
|
||||
return _userErr(req.id, 'pass only one of `to` and `by`');
|
||||
}
|
||||
final value = view.value;
|
||||
if (value == null) {
|
||||
return _userErr(req.id, '${view.hasTo ? "to" : "by"} must be numeric');
|
||||
}
|
||||
final slot = view.slot!;
|
||||
final value = ((hasTo ? req.args['to'] : req.args['by']) as num).toDouble();
|
||||
|
||||
if (slot == editorSplitSlot) {
|
||||
if (view.hasTo) {
|
||||
if (hasTo) {
|
||||
r.setEditorRatio(value);
|
||||
} else {
|
||||
r.bumpEditorRatio(value);
|
||||
@@ -80,7 +92,7 @@ Future<IpcResponse> _resize(IpcRequest req, PanelResizer r) async {
|
||||
});
|
||||
}
|
||||
|
||||
final ok = view.hasTo ? r.setSlotSize(slot, value) : r.bumpSlotSize(slot, value);
|
||||
final ok = hasTo ? r.setSlotSize(slot, value) : r.bumpSlotSize(slot, value);
|
||||
if (!ok) {
|
||||
return _notFound(req.id, 'no such slot: $slot');
|
||||
}
|
||||
@@ -90,58 +102,6 @@ Future<IpcResponse> _resize(IpcRequest req, PanelResizer r) async {
|
||||
});
|
||||
}
|
||||
|
||||
/// Tiny adapter that lifts `panel.resize` arguments out of either
|
||||
/// the direct call shape (`{slot: ..., to: ...}`) or the argv-
|
||||
/// translator shape (`{positional: [slot], flags: {to: '...'}}`).
|
||||
/// Until T-120 formalises a shared schema, individual commands carry
|
||||
/// the lift themselves.
|
||||
class _ResizeArgs {
|
||||
_ResizeArgs._({
|
||||
required this.slot,
|
||||
required this.hasTo,
|
||||
required this.hasBy,
|
||||
required this.value,
|
||||
});
|
||||
|
||||
final String? slot;
|
||||
final bool hasTo;
|
||||
final bool hasBy;
|
||||
final double? value;
|
||||
|
||||
factory _ResizeArgs.from(Map<String, Object?> args) {
|
||||
String? slot;
|
||||
final rawSlot = args['slot'];
|
||||
if (rawSlot is String) slot = rawSlot;
|
||||
final positional = args['positional'];
|
||||
if (slot == null && positional is List && positional.isNotEmpty) {
|
||||
slot = positional.first.toString();
|
||||
}
|
||||
final flags = args['flags'];
|
||||
final flagsMap = flags is Map ? flags : const <Object?, Object?>{};
|
||||
final hasTo = args.containsKey('to') || flagsMap.containsKey('to');
|
||||
final hasBy = args.containsKey('by') || flagsMap.containsKey('by');
|
||||
final raw = args.containsKey('to')
|
||||
? args['to']
|
||||
: args.containsKey('by')
|
||||
? args['by']
|
||||
: flagsMap.containsKey('to')
|
||||
? flagsMap['to']
|
||||
: flagsMap['by'];
|
||||
return _ResizeArgs._(
|
||||
slot: slot,
|
||||
hasTo: hasTo,
|
||||
hasBy: hasBy,
|
||||
value: _coerceNum(raw),
|
||||
);
|
||||
}
|
||||
|
||||
static double? _coerceNum(Object? v) {
|
||||
if (v is num) return v.toDouble();
|
||||
if (v is String) return double.tryParse(v);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
IpcResponse _userErr(String id, String message, {String? hint}) => IpcResponse.err(
|
||||
id: id,
|
||||
error: IpcError(
|
||||
|
||||
@@ -0,0 +1,220 @@
|
||||
/// Typed argument schema for IPC commands (T-120, per D-74).
|
||||
///
|
||||
/// A [CommandSchema] declares the arguments a command accepts — their
|
||||
/// types, whether they're required, and per-argument constraints
|
||||
/// (charset/pattern, leading-dash rejection, numeric range, list
|
||||
/// caps). Schemas are registered alongside their handlers on the
|
||||
/// `DaemonDispatcher`; the dispatcher normalises + validates
|
||||
/// `req.args` against the registered schema *before* invoking the
|
||||
/// handler, so a command can never see malformed input and no handler
|
||||
/// has to hand-roll its own argument checks.
|
||||
///
|
||||
/// Two call shapes reach a command (see T-119): the direct shape
|
||||
/// (`{branch: "main"}`, from the in-process `DaemonClient` / UI) and
|
||||
/// the argv-translator shape (`{positional: [...], flags: {...}}`,
|
||||
/// from the C `clide` client). [CommandSchema.normalize] folds the
|
||||
/// argv shape into named arguments using the schema's positional
|
||||
/// ordering, so handlers only ever read flat named keys.
|
||||
///
|
||||
/// Flutter-free by construction — lives under `lib/src/ipc/` and
|
||||
/// imports nothing from the kernel. The constraint vocabulary is
|
||||
/// hand-rolled per the prefer-zero-deps guardrail.
|
||||
library;
|
||||
|
||||
import 'envelope.dart';
|
||||
import 'schema_v1.dart';
|
||||
|
||||
/// Coarse type of an argument value after coercion.
|
||||
enum ArgType { string, number, boolean, stringList }
|
||||
|
||||
/// Constraints on a single argument.
|
||||
class ArgSpec {
|
||||
const ArgSpec({
|
||||
this.type = ArgType.string,
|
||||
this.required = false,
|
||||
this.pattern,
|
||||
this.rejectLeadingDash = false,
|
||||
this.min,
|
||||
this.max,
|
||||
this.maxItems,
|
||||
this.allowed,
|
||||
});
|
||||
|
||||
final ArgType type;
|
||||
final bool required;
|
||||
|
||||
/// String only — the value must fully match this pattern.
|
||||
final RegExp? pattern;
|
||||
|
||||
/// String / stringList — reject any value (or element) starting with
|
||||
/// `-`. This is the argv-injection guard T-104 introduced for git
|
||||
/// refs; the schema makes it declarative.
|
||||
final bool rejectLeadingDash;
|
||||
|
||||
/// Number only — inclusive bounds.
|
||||
final num? min;
|
||||
final num? max;
|
||||
|
||||
/// stringList only — maximum element count.
|
||||
final int? maxItems;
|
||||
|
||||
/// String only — closed set of accepted values.
|
||||
final Set<String>? allowed;
|
||||
|
||||
/// Coerce [raw] to this spec's [type]. Returns the typed value, or a
|
||||
/// [_CoerceError] when the raw value can't be represented as the
|
||||
/// declared type. Flag values arrive as strings from the argv
|
||||
/// translator, so numeric/boolean args parse from text here.
|
||||
Object? _coerce(Object? raw, void Function(String) fail) {
|
||||
switch (type) {
|
||||
case ArgType.string:
|
||||
if (raw is String) return raw;
|
||||
fail('expected a string');
|
||||
return null;
|
||||
case ArgType.number:
|
||||
if (raw is num) return raw;
|
||||
if (raw is String) {
|
||||
final n = num.tryParse(raw);
|
||||
if (n != null) return n;
|
||||
}
|
||||
fail('expected a number');
|
||||
return null;
|
||||
case ArgType.boolean:
|
||||
if (raw is bool) return raw;
|
||||
if (raw == 'true') return true;
|
||||
if (raw == 'false') return false;
|
||||
fail('expected a boolean');
|
||||
return null;
|
||||
case ArgType.stringList:
|
||||
if (raw is List) {
|
||||
return raw.map((e) => '$e').toList();
|
||||
}
|
||||
if (raw is String) return [raw];
|
||||
fail('expected a list');
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/// Apply value constraints to an already-coerced [value]. Returns
|
||||
/// null when valid, else a human-readable reason.
|
||||
String? _check(Object? value) {
|
||||
switch (type) {
|
||||
case ArgType.string:
|
||||
final s = value as String;
|
||||
if (rejectLeadingDash && s.startsWith('-')) {
|
||||
return 'must not start with "-"';
|
||||
}
|
||||
if (allowed != null && !allowed!.contains(s)) {
|
||||
return 'must be one of ${allowed!.join(", ")}';
|
||||
}
|
||||
if (pattern != null && !pattern!.hasMatch(s)) {
|
||||
return 'does not match ${pattern!.pattern}';
|
||||
}
|
||||
return null;
|
||||
case ArgType.number:
|
||||
final n = value as num;
|
||||
if (min != null && n < min!) return 'must be >= $min';
|
||||
if (max != null && n > max!) return 'must be <= $max';
|
||||
return null;
|
||||
case ArgType.boolean:
|
||||
return null;
|
||||
case ArgType.stringList:
|
||||
final list = (value as List).cast<String>();
|
||||
if (maxItems != null && list.length > maxItems!) {
|
||||
return 'has ${list.length} items; cap is $maxItems';
|
||||
}
|
||||
if (rejectLeadingDash) {
|
||||
for (final e in list) {
|
||||
if (e.startsWith('-')) return 'element "$e" must not start with "-"';
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The argument contract for one command.
|
||||
class CommandSchema {
|
||||
const CommandSchema({this.positional = const [], this.args = const {}});
|
||||
|
||||
/// Names for positional argv tokens, in order. `positional[i]` in the
|
||||
/// argv-translator shape maps to the argument named `positional[i]`.
|
||||
final List<String> positional;
|
||||
|
||||
/// Argument name → spec. Includes the positional names.
|
||||
final Map<String, ArgSpec> args;
|
||||
|
||||
/// Reserved keys the argv translator emits — never argument names.
|
||||
static const _argvKeys = {'positional', 'flags', 'passthrough'};
|
||||
|
||||
/// Fold the argv-translator shape (`{positional, flags}`) into flat
|
||||
/// named arguments using [positional] ordering. The direct call
|
||||
/// shape (already flat named keys) is returned unchanged.
|
||||
Map<String, Object?> normalize(Map<String, Object?> raw) {
|
||||
final looksLikeArgv = raw.keys.any(_argvKeys.contains);
|
||||
if (!looksLikeArgv) return raw;
|
||||
final out = <String, Object?>{};
|
||||
final pos = raw['positional'];
|
||||
if (pos is List) {
|
||||
for (var i = 0; i < pos.length && i < positional.length; i++) {
|
||||
out[positional[i]] = pos[i];
|
||||
}
|
||||
}
|
||||
final flags = raw['flags'];
|
||||
if (flags is Map) {
|
||||
for (final e in flags.entries) {
|
||||
out['${e.key}'] = e.value;
|
||||
}
|
||||
}
|
||||
if (raw.containsKey('passthrough')) out['passthrough'] = raw['passthrough'];
|
||||
return out;
|
||||
}
|
||||
|
||||
/// Validate [normalized] (post-[normalize]) against every declared
|
||||
/// arg. Returns a [SchemaResult] carrying either the coerced argument
|
||||
/// map (declared args replaced with their typed values; undeclared
|
||||
/// keys preserved untouched) or the first violation message.
|
||||
SchemaResult validate(Map<String, Object?> normalized) {
|
||||
final out = Map<String, Object?>.from(normalized);
|
||||
for (final entry in args.entries) {
|
||||
final name = entry.key;
|
||||
final spec = entry.value;
|
||||
final present = normalized.containsKey(name) && normalized[name] != null;
|
||||
if (!present) {
|
||||
if (spec.required) return SchemaResult.err('$name is required');
|
||||
continue;
|
||||
}
|
||||
String? coerceFailure;
|
||||
final coerced = spec._coerce(normalized[name], (m) => coerceFailure = m);
|
||||
if (coerceFailure != null) return SchemaResult.err('$name: $coerceFailure');
|
||||
final reason = spec._check(coerced);
|
||||
if (reason != null) return SchemaResult.err('$name $reason');
|
||||
out[name] = coerced;
|
||||
}
|
||||
return SchemaResult.ok(out);
|
||||
}
|
||||
}
|
||||
|
||||
/// Outcome of [CommandSchema.validate].
|
||||
class SchemaResult {
|
||||
const SchemaResult.ok(this.values) : error = null;
|
||||
const SchemaResult.err(this.error) : values = null;
|
||||
|
||||
/// Coerced argument map on success; null on failure.
|
||||
final Map<String, Object?>? values;
|
||||
|
||||
/// Violation message on failure; null on success.
|
||||
final String? error;
|
||||
|
||||
bool get isOk => error == null;
|
||||
}
|
||||
|
||||
/// Build the standard `userError` response for a schema violation.
|
||||
IpcResponse schemaError(String id, String message) => IpcResponse.err(
|
||||
id: id,
|
||||
error: IpcError(
|
||||
code: IpcExitCode.userError,
|
||||
kind: IpcErrorKind.userError,
|
||||
message: message,
|
||||
),
|
||||
);
|
||||
Reference in New Issue
Block a user