Skip to content

Commit f156c87

Browse files
ale-rinaldiclaude
andauthored
fix(configio,codec): record export locale and refuse cross-locale silent label remap (#20)
Two changes that compose to close issue #4's silent-data-loss path: 1. enumNum no longer returns 0 when a compound sub-field's enum label can't be resolved. It returns an error and encodeVar / EncodeCompound propagate up to the user. This matches the loud behaviour resolveEnumWriteValue already had for top-level ENUMs and removes the case where a YAML label translated to a different locale silently writes the zero entry (SOGLIA Tipo flipping MINIMA↔MASSIMA was the worked example). 2. Export records `device.locale` in the YAML header. Import / diff / validate read it back through a small reconcileLocale helper: * file has no locale (legacy) → CLI default unchanged; * file has locale, user didn't pass --locale → adopt file's; * file has locale, user passed matching --locale → use it; * file has locale, user passed different --locale → return LocaleMismatchError unless --force-locale is set. This keeps YAML files readable in the locale they were produced under while making cross-locale round-trips a loud, opt-in event rather than a silent corruption. The error names both locales and points at --force-locale so the operator can either drop --locale (adopting the file's) or keep theirs explicitly. Closes #4 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 27bb37e commit f156c87

12 files changed

Lines changed: 276 additions & 24 deletions

File tree

cmd/mythy/diff.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func newDiffCmd(cf *catalogFlags) *cobra.Command {
2626
var format string
2727
var noProgress bool
2828
var includeAll bool
29+
var forceLocale bool
2930

3031
cmd := &cobra.Command{
3132
Use: "diff <file>",
@@ -41,6 +42,9 @@ func newDiffCmd(cf *catalogFlags) *cobra.Command {
4142
if err != nil {
4243
return err
4344
}
45+
if err := reconcileLocale(cmd, parsed.Device.Locale, cf, forceLocale); err != nil {
46+
return err
47+
}
4448
s, err := conn.build(ctx, cf)
4549
if err != nil {
4650
return err
@@ -72,6 +76,7 @@ func newDiffCmd(cf *catalogFlags) *cobra.Command {
7276
cmd.Flags().BoolVar(&noProgress, "no-progress", false, "suppress the progress indicator (auto-suppressed when stderr isn't a TTY)")
7377
cmd.Flags().BoolVar(&includeAll, "all", false,
7478
"include runtime/state items (READONLY=YES, paths under Read/) — defaults to filtered to surface only configuration drift")
79+
cmd.Flags().BoolVar(&forceLocale, "force-locale", false, "proceed even if --locale differs from the YAML's device.locale")
7580
return cmd
7681
}
7782

cmd/mythy/export.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ func newExportCmd(cf *catalogFlags) *cobra.Command {
3838
includeSkip = true
3939
}
4040
b, err := configio.Export(ctx, s, configio.ExportOptions{
41-
Scope: scope,
41+
Scope: scope,
42+
Locale: cf.locale,
4243
Filter: session.ExportFilter{
4344
IncludeHidden: includeHidden,
4445
IncludeReadOnly: includeReadOnly,

cmd/mythy/import.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
func newImportCmd(cf *catalogFlags) *cobra.Command {
1515
var conn connFlags
16-
var dryRun, force, noProgress bool
16+
var dryRun, force, noProgress, forceLocale bool
1717
var includeSkip, includeHidden, includeAll, yesIUnderstand bool
1818
var format string
1919

@@ -31,6 +31,9 @@ func newImportCmd(cf *catalogFlags) *cobra.Command {
3131
if err != nil {
3232
return err
3333
}
34+
if err := reconcileLocale(cmd, parsed.Device.Locale, cf, forceLocale); err != nil {
35+
return err
36+
}
3437
s, err := conn.build(ctx, cf)
3538
if err != nil {
3639
return err
@@ -105,6 +108,7 @@ func newImportCmd(cf *catalogFlags) *cobra.Command {
105108
cmd.Flags().BoolVar(&includeHidden, "include-hidden", false, "allow writes to VISIBILITY=3 keys (Administrator subtree)")
106109
cmd.Flags().BoolVar(&includeAll, "all", false, "shorthand for --include-skip --include-hidden")
107110
cmd.Flags().BoolVar(&yesIUnderstand, "yes-i-understand", false, "confirm intent when an --include-* flag is set")
111+
cmd.Flags().BoolVar(&forceLocale, "force-locale", false, "proceed even if --locale differs from the YAML's device.locale")
108112
cmd.Flags().StringVar(&format, "format", "", "human|json|yaml (default: from MYTHY_FORMAT)")
109113
return cmd
110114
}

cmd/mythy/locale_compat.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package main
2+
3+
import (
4+
"github.com/gridsociety/mythy/pkg/configio"
5+
"github.com/spf13/cobra"
6+
)
7+
8+
// reconcileLocale picks the effective --locale for a command that
9+
// consumes a YAML file (import / diff / validate). The decision is:
10+
//
11+
// - no locale in YAML (legacy export): keep cf.locale unchanged.
12+
// - locale in YAML, user didn't pass --locale explicitly: adopt
13+
// the YAML's locale into cf.locale.
14+
// - locale in YAML, user passed --locale matching: keep cf.locale.
15+
// - locale in YAML, user passed --locale differing: return a typed
16+
// LocaleMismatchError unless --force-locale is set.
17+
//
18+
// The flag-was-passed check uses cobra's Changed(): the locale flag
19+
// defaults to "en" (or MYTHY_LOCALE), and we want to distinguish "user
20+
// typed --locale=en" from "user typed nothing and en is the default".
21+
// "Changed" reports true only for explicit invocation.
22+
//
23+
// Reconciliation mutates cf.locale in place. Callers can rely on
24+
// cf.load() / connFlags.build() using the resolved value transparently.
25+
func reconcileLocale(cmd *cobra.Command, fileLocale string, cf *catalogFlags, forceLocale bool) error {
26+
if fileLocale == "" {
27+
return nil
28+
}
29+
explicit := cmd.Flags().Changed("locale")
30+
if !explicit {
31+
cf.locale = fileLocale
32+
return nil
33+
}
34+
if cf.locale == fileLocale {
35+
return nil
36+
}
37+
if forceLocale {
38+
return nil
39+
}
40+
return &configio.LocaleMismatchError{FromFile: fileLocale, FromCLI: cf.locale}
41+
}

cmd/mythy/locale_compat_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package main
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/gridsociety/mythy/pkg/configio"
8+
"github.com/spf13/cobra"
9+
)
10+
11+
// mkLocaleCmd sets up a minimal cobra.Command + catalogFlags with a
12+
// --locale flag wired exactly as the real subcommands do (default
13+
// "en", env MYTHY_LOCALE). args provides the CLI tokens after the
14+
// command name; pass nothing for "user did not specify --locale".
15+
func mkLocaleCmd(t *testing.T, args ...string) (*cobra.Command, *catalogFlags) {
16+
t.Helper()
17+
cf := &catalogFlags{}
18+
cmd := &cobra.Command{Use: "x", RunE: func(*cobra.Command, []string) error { return nil }}
19+
cmd.PersistentFlags().StringVar(&cf.locale, "locale", "en", "")
20+
cmd.SetArgs(args)
21+
if err := cmd.ParseFlags(args); err != nil {
22+
t.Fatalf("ParseFlags: %v", err)
23+
}
24+
return cmd, cf
25+
}
26+
27+
func TestReconcileLocaleLegacyFile(t *testing.T) {
28+
cmd, cf := mkLocaleCmd(t)
29+
if err := reconcileLocale(cmd, "", cf, false); err != nil {
30+
t.Errorf("legacy export (no locale in file): err = %v, want nil", err)
31+
}
32+
if cf.locale != "en" {
33+
t.Errorf("legacy export: cf.locale = %q, want unchanged 'en'", cf.locale)
34+
}
35+
}
36+
37+
func TestReconcileLocaleAdoptsFileLocaleWhenCLIImplicit(t *testing.T) {
38+
cmd, cf := mkLocaleCmd(t)
39+
if err := reconcileLocale(cmd, "it", cf, false); err != nil {
40+
t.Errorf("err = %v, want nil", err)
41+
}
42+
if cf.locale != "it" {
43+
t.Errorf("cf.locale = %q, want 'it' (adopted from file)", cf.locale)
44+
}
45+
}
46+
47+
func TestReconcileLocaleKeepsCLIWhenMatching(t *testing.T) {
48+
cmd, cf := mkLocaleCmd(t, "--locale", "it")
49+
if err := reconcileLocale(cmd, "it", cf, false); err != nil {
50+
t.Errorf("err = %v, want nil", err)
51+
}
52+
if cf.locale != "it" {
53+
t.Errorf("cf.locale = %q, want 'it'", cf.locale)
54+
}
55+
}
56+
57+
func TestReconcileLocaleErrorsOnExplicitMismatch(t *testing.T) {
58+
cmd, cf := mkLocaleCmd(t, "--locale", "us")
59+
err := reconcileLocale(cmd, "it", cf, false)
60+
var lm *configio.LocaleMismatchError
61+
if !errors.As(err, &lm) {
62+
t.Fatalf("err = %v, want LocaleMismatchError", err)
63+
}
64+
if lm.FromFile != "it" || lm.FromCLI != "us" {
65+
t.Errorf("error fields = %+v, want FromFile=it FromCLI=us", lm)
66+
}
67+
}
68+
69+
func TestReconcileLocaleForceLocaleBypassesMismatch(t *testing.T) {
70+
cmd, cf := mkLocaleCmd(t, "--locale", "us")
71+
if err := reconcileLocale(cmd, "it", cf, true); err != nil {
72+
t.Errorf("--force-locale should bypass mismatch: err = %v", err)
73+
}
74+
if cf.locale != "us" {
75+
t.Errorf("--force-locale must keep CLI value: cf.locale = %q, want 'us'", cf.locale)
76+
}
77+
}

cmd/mythy/validate.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,32 @@ type configFile struct {
2121

2222
type configDevice struct {
2323
Product string `yaml:"product"`
24+
Locale string `yaml:"locale"`
2425
}
2526

2627
func newValidateCmd(cf *catalogFlags) *cobra.Command {
27-
return &cobra.Command{
28+
var forceLocale bool
29+
cmd := &cobra.Command{
2830
Use: "validate <file>",
2931
Short: "Validate a config YAML against the device's catalog (no connection)",
3032
Args: cobra.ExactArgs(1),
3133
RunE: func(cmd *cobra.Command, args []string) error {
32-
tpl, entry, err := cf.load()
33-
if err != nil {
34-
return err
35-
}
3634
data, err := os.ReadFile(args[0])
3735
if err != nil {
3836
return fmt.Errorf("read %s: %w", args[0], err)
3937
}
40-
var cfg configFile
41-
if err := yaml.Unmarshal(data, &cfg); err != nil {
38+
var cfgPeek configFile
39+
if err := yaml.Unmarshal(data, &cfgPeek); err != nil {
4240
return fmt.Errorf("parse YAML: %w", err)
4341
}
42+
if err := reconcileLocale(cmd, cfgPeek.Device.Locale, cf, forceLocale); err != nil {
43+
return err
44+
}
45+
tpl, entry, err := cf.load()
46+
if err != nil {
47+
return err
48+
}
49+
cfg := cfgPeek
4450
if cfg.MythyVersion != 1 {
4551
return fmt.Errorf("unsupported mythy_version %d", cfg.MythyVersion)
4652
}
@@ -72,4 +78,6 @@ func newValidateCmd(cf *catalogFlags) *cobra.Command {
7278
return nil
7379
},
7480
}
81+
cmd.Flags().BoolVar(&forceLocale, "force-locale", false, "proceed even if --locale differs from the YAML's device.locale")
82+
return cmd
7583
}

pkg/codec/compound.go

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,17 @@ func encodeVar(v catalog.ClassVar, val any, width int) ([]uint16, error) {
157157
s, _ := val.(string)
158158
return encodeStringChars(s, width), nil
159159
case "ENUM", "ENUM_BYTE", "ENUM_WORD":
160-
return []uint16{uint16(enumNum(val, v) & 0xFFFF)}, nil
160+
n, err := enumNum(val, v)
161+
if err != nil {
162+
return nil, err
163+
}
164+
return []uint16{uint16(n & 0xFFFF)}, nil
161165
case "ENUM_LONG":
162-
u := uint64(enumNum(val, v))
166+
n, err := enumNum(val, v)
167+
if err != nil {
168+
return nil, err
169+
}
170+
u := uint64(n)
163171
return []uint16{uint16(u & 0xFFFF), uint16((u >> 16) & 0xFFFF)}, nil
164172
}
165173
return nil, fmt.Errorf("encodeVar: unknown TIPO %q", v.Tipo)
@@ -189,26 +197,32 @@ func toUint64(v any) uint64 {
189197
return 0
190198
}
191199

192-
func enumNum(val any, v catalog.ClassVar) int {
200+
// enumNum resolves a sub-field value to the integer wire encoding for
201+
// an ENUM family VAR. Strings are looked up against v.InlineEnum (the
202+
// per-VAR <RANGE> declarations); integers pass through. An
203+
// unresolvable string label returns an error rather than silently
204+
// defaulting to 0 — quietly zeroing on a typo, a locale-translated
205+
// label, or any other miss was the silent-data-loss path tracked in
206+
// issue #4.
207+
func enumNum(val any, v catalog.ClassVar) (int, error) {
193208
switch x := val.(type) {
194209
case string:
195-
if v.InlineEnum != nil {
196-
if n, err := v.InlineEnum.ValueFor(x); err == nil {
197-
return n
198-
}
210+
if v.InlineEnum == nil {
211+
return 0, fmt.Errorf("VAR %s has no inline enum to resolve label %q", v.Name, x)
199212
}
200-
return 0
213+
n, err := v.InlineEnum.ValueFor(x)
214+
if err != nil {
215+
return 0, fmt.Errorf("VAR %s: %w", v.Name, err)
216+
}
217+
return n, nil
201218
case int:
202-
// YAML decoders typically produce `int` for small integers; the
203-
// previous version of this switch only knew int64/uint64, so an
204-
// integer enum value silently became 0.
205-
return x
219+
return x, nil
206220
case int64:
207-
return int(x)
221+
return int(x), nil
208222
case uint64:
209-
return int(x)
223+
return int(x), nil
210224
}
211-
return 0
225+
return 0, fmt.Errorf("VAR %s: unsupported enum value type %T", v.Name, val)
212226
}
213227

214228
// encodeStringChars encodes a string as `width` registers of NUL-padded

pkg/codec/compound_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package codec_test
33
import (
44
"path/filepath"
55
"reflect"
6+
"strings"
67
"testing"
78

89
"github.com/gridsociety/mythy/pkg/catalog"
@@ -177,6 +178,42 @@ func TestEncodeCompoundHandlesEnumWord(t *testing.T) {
177178
}
178179
}
179180

181+
func TestEncodeCompoundErrorsOnUnresolvableEnumLabel(t *testing.T) {
182+
// Issue #4: silently-zeroed enum labels (e.g. an Italian label
183+
// landing in a US-locale template) corrupted compound writes
184+
// without any error. enumNum now surfaces the failure; encodeVar
185+
// propagates it; EncodeCompound returns it to the caller.
186+
cls := &catalog.Class{
187+
Name: "SOGLIA_T",
188+
Dim: 3,
189+
Vars: []catalog.ClassVar{
190+
{
191+
Name: "Tipo",
192+
Tipo: "ENUM",
193+
InlineEnum: &catalog.Enum{
194+
Name: "TipoInline",
195+
Entries: []catalog.EnumEntry{
196+
{Value: 0, Label: "Minimum"},
197+
{Value: 1, Label: "Maximum"},
198+
},
199+
},
200+
},
201+
{Name: "Pad", Tipo: "UWORD"},
202+
},
203+
}
204+
in := map[string]any{"Tipo": "SOGLIA MASSIMA", "Pad": uint64(0)}
205+
_, err := codec.EncodeCompound(in, cls, nil, nil)
206+
if err == nil {
207+
t.Fatal("expected error for unresolvable enum label, got nil")
208+
}
209+
if !strings.Contains(err.Error(), "Tipo") {
210+
t.Errorf("error should name the offending VAR: %v", err)
211+
}
212+
if !strings.Contains(err.Error(), "SOGLIA MASSIMA") {
213+
t.Errorf("error should quote the unresolved label: %v", err)
214+
}
215+
}
216+
180217
func TestEncodeCompoundHandlesINT(t *testing.T) {
181218
// Issue #10: legacy SIF/SVF templates use TIPO="INT" for 16-bit
182219
// signed integers. SIF5600.IdentificationResponse declares dim=5

pkg/configio/export.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ type ExportOptions struct {
1616
Scope string // empty = whole device
1717
Filter session.ExportFilter // READONLY/hidden defaults; SKIP always excluded
1818

19+
// Locale is recorded into device.locale in the YAML so import /
20+
// diff / validate can detect cross-locale round-trips that would
21+
// otherwise silently remap enum labels to zero. Empty means "don't
22+
// record" (legacy behaviour); the CLI always passes a value.
23+
Locale string
24+
1925
// Progress, when non-nil, is invoked once before each catalog leaf
2026
// is read, and once more at the end with done == total and name == "".
2127
// Use the final call to clear any in-progress UI. The full-device
@@ -106,6 +112,7 @@ func Export(ctx context.Context, s *session.Session, opts ExportOptions) ([]byte
106112
Product: s.Entry().Product,
107113
Identification: s.Entry().Identification,
108114
ExportedAt: time.Now().Format(time.RFC3339),
115+
Locale: opts.Locale,
109116
}
110117
if id := s.Ident(); id != nil {
111118
dev.Identification = int(id.Identification)

0 commit comments

Comments
 (0)