fix(cmdcore): close review findings on safety tier inference
- schemaSafetyFromDecl: drop the now-unreachable nil return; the tier fill always produces a complete block for a declared Schema - validateDispatchDecl: panic when ConfirmFirst is set without Risk — it orders a confirmation that does not exist, and for declared-Schema writes an empty Risk would silently publish the read safety tier - RFC: document the boundary that write commands must declare Risk Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
1 parent
f805c966d6
commit
08c9c8a923
4 files changed
+28
-9
No files matched your search
@@ -1146,7 +1146,7 @@ cmd.RunE = func(cmd *cobra.Command, args []string) error {
|
||||
| 非 Shortcut 受管定义的 Cobra 命令 Hidden | 可执行 Contract | 挂载的命令可见性与声明匹配 |
|
||||
| Shortcut 列表成员资格与语义 disposition | 经评审的 Shortcut 可见性解析器 | public/all 列表成员资格与经评审决策匹配 |
|
||||
| Runtime Schema / Agent 暴露 | 经评审的 CommandRegistry 加精确排除 | 每个暴露叶子解析到活 Contract;排除显式且不重叠 |
|
||||
| 运行时 Risk 与确认 | 可执行 Contract(非空 `Risk`)或显式 annotate(如 `runtime_gate`);见 §5.0 | 不得靠推断;空 Risk 不嵌入 schema risk;写命令须 Risk 或 gate;Safety 组装优先级 **`Schema.Safety`(Final 显式字段)> `CommandSpec.Safety` 枚举 tier > `Risk.SafetyDefault()` > `runtime_gate`**,高优先命中即忽略低优先;tier 填充含 idempotency(读 idempotent / 写 unknown),故仅声明 Risk/Safety 枚举即自足 |
|
||||
| 运行时 Risk 与确认 | 可执行 Contract(非空 `Risk`)或显式 annotate(如 `runtime_gate`);见 §5.0 | 不得靠推断;空 Risk 不嵌入 schema risk;写命令须 Risk 或 gate;Safety 组装优先级 **`Schema.Safety`(Final 显式字段)> `CommandSpec.Safety` 枚举 tier > `Risk.SafetyDefault()` > `runtime_gate`**,高优先命中即忽略低优先;tier 填充含 idempotency(读 idempotent / 写 unknown),故仅声明 Risk/Safety 枚举即自足。**边界**:tier 恒可推导意味着写命令忘声明 `Risk` 会静默发布 read tier(且失去运行时确认)——写命令必须声明 `Risk`;`ConfirmFirst` 无 `Risk` 属 authoring 错误,构造期 panic |
|
||||
| 后端 product/tool/载荷绑定 | mcpbind + 后端元数据 | 每个绑定引用真实的 flag/属性 |
|
||||
| Agent 选择文案(`use_when`、`avoid_when`、摘要) | 经评审的 hints/catalog | 身份解析到活契约 |
|
||||
|
||||
|
||||
@@ -469,6 +469,15 @@ func validateDispatchDecl(spec CommandSpec) {
|
||||
"command %q must declare exactly one of RunE/Invoke/Orchestrate, got %d",
|
||||
spec.Use, declared))
|
||||
}
|
||||
// ConfirmFirst only makes sense on a Risk-driven confirmation flow. A
|
||||
// command with ConfirmFirst but no Risk is always an authoring mistake —
|
||||
// and for a declared-Schema write it would also silently publish the read
|
||||
// safety tier (empty Risk infers SafetyRead).
|
||||
if spec.ConfirmFirst && strings.TrimSpace(string(spec.Risk)) == "" {
|
||||
panic(fmt.Sprintf(
|
||||
"command %q sets ConfirmFirst but declares no Risk: ConfirmFirst orders the Risk-driven confirmation and is meaningless without one",
|
||||
spec.Use))
|
||||
}
|
||||
}
|
||||
|
||||
// RegisterFlags registers every flag (plus hidden aliases and MarkFlagRequired)
|
||||
@@ -1073,13 +1082,10 @@ func schemaSafetyFromDecl(risk Risk, safety Safety, decl SafetyDecl) *cli.Safety
|
||||
}
|
||||
// effect_source is assembly-derived for every Contract-declared safety,
|
||||
// whether it came from the Risk shorthand or an explicit SchemaDecl; the
|
||||
// declaration in code is the source of truth.
|
||||
if out.Effect != "" && out.EffectSource == "" {
|
||||
out.EffectSource = "cmdcore.contract"
|
||||
}
|
||||
if out.Effect == "" && out.Risk == "" && out.Confirmation == "" && out.Idempotency == "" {
|
||||
return nil
|
||||
}
|
||||
// declaration in code is the source of truth. The tier fill above always
|
||||
// produces a complete block (read tier by default), so the payload is
|
||||
// never nil for a declared Schema.
|
||||
out.EffectSource = "cmdcore.contract"
|
||||
return &out
|
||||
}
|
||||
|
||||
|
||||
@@ -899,6 +899,15 @@ func TestCrossPlatformCoverageNewCommandRequiresExactlyOneDispatcher(t *testing.
|
||||
RunE: func(*cobra.Command, []string) error { return nil },
|
||||
Invoke: func(*Ctx, map[string]any) error { return nil },
|
||||
}, "got 2")
|
||||
|
||||
// ConfirmFirst without Risk orders a confirmation that does not exist — and
|
||||
// for declared-Schema writes it would also publish the read safety tier.
|
||||
mustPanic("confirmFirst without risk", CommandSpec{
|
||||
Use: "guarded",
|
||||
Flags: flags,
|
||||
ConfirmFirst: true,
|
||||
Invoke: func(*Ctx, map[string]any) error { return nil },
|
||||
}, "ConfirmFirst but declares no Risk")
|
||||
}
|
||||
|
||||
func TestCrossPlatformCoverageNewCommandOrchestrateDispatch(t *testing.T) {
|
||||
|
||||
@@ -131,7 +131,11 @@ func validateSchemaDecl(spec CommandSpec) {
|
||||
// (no hints fallback). Safety needs no completeness check: the enum tier
|
||||
// (CommandSpec.Safety, falling back to Risk.SafetyDefault()) fills every
|
||||
// Safety field including Idempotency, so any declared Schema projects a
|
||||
// complete safety block by construction.
|
||||
// complete safety block by construction. The flip side of always-on
|
||||
// inference: a WRITE command must declare Risk (which also drives runtime
|
||||
// confirmation) — an empty Risk publishes the read tier. The detectable
|
||||
// half of that mistake (ConfirmFirst without Risk) is caught by
|
||||
// validateDispatchDecl.
|
||||
if iface := spec.Schema.Interface; iface == nil ||
|
||||
strings.TrimSpace(iface.Mode) == "" || strings.TrimSpace(iface.Availability) == "" {
|
||||
missing = append(missing, "Schema.Interface (mode/availability)")
|
||||
|
||||
Reference in new issue
Block a user