From 1ed718cadcdb6264aff62ba75115f17f42899df1 Mon Sep 17 00:00:00 2001 From: toki Date: Sun, 9 Aug 2026 10:35:53 +0900 Subject: [PATCH] =?UTF-8?q?fix(single=5Frequest):=20=EB=AA=85=EC=8B=9C?= =?UTF-8?q?=EB=90=9C=20null=20=ED=85=9C=ED=94=8C=EB=A6=BF=20=EA=B2=BD?= =?UTF-8?q?=EB=A1=9C=EB=A5=BC=20=EC=83=9D=EB=9E=B5=EA=B3=BC=20=EA=B5=AC?= =?UTF-8?q?=EB=B6=84=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plan_file: null`과 `review_file: ~`은 YAML에 실제로 존재하는 키인데 mapstructure가 present nil을 생략과 같은 nil pointer로 접어버려, 승인 경계가 빈 문자열과 공백만 거부하고 null 형태는 조용히 built-in default로 떨어뜨렸다. Viper가 돌려주는 raw preset 구조에는 두 키가 nil 값으로 남아 있으므로, strict decode 전에 그 presence를 검사해 필드 경로와 함께 거부하고 실제로 없는 키만 fallback 신호로 남긴다. 두 필드와 두 가지 YAML null 표기를 모두 회귀로 고정하고, 기존 빈/공백, 생략, 상대 경로, clone 독립성, refresh redaction 근거는 그대로 유지한다. Refs: agent-task/single_request_plan_review_templates/PLAN-cloud-G08.md Co-Authored-By: Claude Opus 5 --- packages/go/config/load.go | 39 +++++++++++++++++++ .../model_execution_preset_config_test.go | 12 ++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/go/config/load.go b/packages/go/config/load.go index 3d71edf5..aec20054 100644 --- a/packages/go/config/load.go +++ b/packages/go/config/load.go @@ -71,6 +71,9 @@ func LoadEdge(cfgFile string) (*EdgeConfig, error) { } if v.InConfig("execution_presets") { raw := v.Get("execution_presets") + if err := rejectNullSingleRequestTemplatePaths(raw); err != nil { + return nil, err + } var presets []ExecutionPreset var metadata mapstructure.Metadata decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ @@ -681,6 +684,42 @@ func validateWorkspaceNumericLimits(ws WorkspaceDefinition, nodeIdx, wsIdx int) return nil } +// rejectNullSingleRequestTemplatePaths fails closed on a present-but-null +// plan_file/review_file before the strict preset decode runs. Mapstructure +// collapses a present raw nil into the same nil pointer as an absent key, so +// `plan_file: null` and `review_file: ~` would otherwise be indistinguishable +// from omission and silently select the built-in default. Only a truly absent +// key may reach the nil-pointer fallback in resolveSingleRequestTemplates; +// non-nil values stay with the strict decoder and loadTemplateFile, and any +// other raw shape is left to the strict decoder's own type error. +func rejectNullSingleRequestTemplatePaths(raw any) error { + presets, ok := raw.([]any) + if !ok { + return nil + } + for i, entry := range presets { + preset, ok := entry.(map[string]any) + if !ok { + continue + } + singleRequest, ok := preset["single_request"].(map[string]any) + if !ok { + continue + } + templates, ok := singleRequest["templates"].(map[string]any) + if !ok { + continue + } + for _, field := range []string{"plan_file", "review_file"} { + value, present := templates[field] + if present && value == nil { + return fmt.Errorf("execution_presets[%d] single_request.templates.%s: template path must not be empty", i, field) + } + } + } + return nil +} + // resolveSingleRequestTemplates freezes the effective Plan/Review templates for // every single-request preset. Only an omitted plan_file/review_file selects the // built-in default; every configured value is routed through loadTemplateFile so diff --git a/packages/go/config/model_execution_preset_config_test.go b/packages/go/config/model_execution_preset_config_test.go index ab5d8991..60669e76 100644 --- a/packages/go/config/model_execution_preset_config_test.go +++ b/packages/go/config/model_execution_preset_config_test.go @@ -965,9 +965,11 @@ nodes: }) // Only an omitted field selects the built-in default. An explicitly - // configured empty or whitespace-only path is a configuration error and - // must be rejected with its own field context before the loader touches - // the filesystem. + // configured empty, whitespace-only, or null path is a configuration error + // and must be rejected with its own field context before the loader touches + // the filesystem. Both YAML null spellings are covered because a present + // null decodes into the same nil pointer as an absent key, so the rejection + // has to happen on the raw preset structure before that presence is erased. t.Run("explicitly configured empty template path fails closed", func(t *testing.T) { cases := []struct { name string @@ -977,8 +979,12 @@ nodes: }{ {"empty plan_file", `""`, "", "single_request.templates.plan_file"}, {"whitespace plan_file", `" "`, "", "single_request.templates.plan_file"}, + {"null plan_file", "null", "", "single_request.templates.plan_file"}, + {"tilde plan_file", "~", "", "single_request.templates.plan_file"}, {"empty review_file", "", `""`, "single_request.templates.review_file"}, {"whitespace review_file", "", `" "`, "single_request.templates.review_file"}, + {"null review_file", "", "null", "single_request.templates.review_file"}, + {"tilde review_file", "", "~", "single_request.templates.review_file"}, } for _, tc := range cases {