From cd00277e8983039c7852362675c31698b08384e9 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:40:42 -0400 Subject: [PATCH 01/10] fix(oplock): harden reentry tokens and terminal detection (D-0609) --- internal/oplock/debt_rollup_test.go | 44 +++++++++++++++++++++++++++++ internal/oplock/guard.go | 7 +++-- internal/oplock/oplock.go | 10 ++++++- internal/oplock/stale_test.go | 6 ++-- 4 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 internal/oplock/debt_rollup_test.go diff --git a/internal/oplock/debt_rollup_test.go b/internal/oplock/debt_rollup_test.go new file mode 100644 index 000000000..40f71a650 --- /dev/null +++ b/internal/oplock/debt_rollup_test.go @@ -0,0 +1,44 @@ +//go:build !windows + +package oplock + +import ( + "context" + "encoding/json" + "os" + "testing" +) + +func TestReleaseInvalidatesHolderToken(t *testing.T) { + dir := t.TempDir() + l, err := Acquire(context.Background(), dir, Opts{}) + if err != nil { + t.Fatal(err) + } + l.Release() + raw, err := os.ReadFile(LockPath(dir)) + if err != nil { + t.Fatal(err) + } + var h Holder + if err := json.Unmarshal(raw, &h); err != nil { + t.Fatal(err) + } + if h.Token != "" { + t.Fatal("released holder retains its re-entry token") + } +} + +func TestStderrNullIsNotTerminal(t *testing.T) { + f, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatal(err) + } + defer f.Close() + old := Stderr + Stderr = f + t.Cleanup(func() { Stderr = old }) + if stderrIsTerminal() { + t.Fatal("null device is not a terminal") + } +} diff --git a/internal/oplock/guard.go b/internal/oplock/guard.go index 9f99d0b1d..3dcba6909 100644 --- a/internal/oplock/guard.go +++ b/internal/oplock/guard.go @@ -33,6 +33,8 @@ import ( "sync" "time" + "golang.org/x/term" + "github.com/nself-org/cli/internal/compat" "github.com/nself-org/cli/internal/errs" ) @@ -163,15 +165,14 @@ func noteUnsupported(quiet bool) { }) } -// stderrIsTerminal reports whether Stderr is a character device. A variable so +// stderrIsTerminal reports whether Stderr is a terminal. A variable so // tests can pretend to be a terminal. var stderrIsTerminal = func() bool { f, ok := Stderr.(*os.File) if !ok { return false } - fi, err := f.Stat() - return err == nil && fi.Mode()&os.ModeCharDevice != 0 + return term.IsTerminal(int(f.Fd())) } func isHeld(err error) bool { _, ok := err.(*HeldError); return ok } diff --git a/internal/oplock/oplock.go b/internal/oplock/oplock.go index 886dcb092..b65f7d0c2 100644 --- a/internal/oplock/oplock.go +++ b/internal/oplock/oplock.go @@ -146,6 +146,13 @@ func (l *Lock) Release() { return } l.once.Do(func() { + h := l.holder + h.Token = "" + if err := writeHolder(l.f, h); err != nil { + // An unreadable record also refuses re-entry; never leave a usable + // previous token behind while releasing the kernel lock. + _ = l.f.Truncate(0) + } unlock(l.f) _ = l.f.Close() }) @@ -185,7 +192,8 @@ func Acquire(ctx context.Context, projectDir string, opts Opts) (*Lock, error) { } _ = f.Close() h, known := readHolderSettled(ctx, projectDir, o) - if known && token != "" && h.Token == token { + host, hostErr := os.Hostname() + if known && token != "" && h.Token == token && hostErr == nil && host != "" && h.Host == host { return &Lock{holder: h, reentrant: true}, nil } waited := o.Clock.Now().Sub(start) diff --git a/internal/oplock/stale_test.go b/internal/oplock/stale_test.go index c9c7fc0d1..39fe79325 100644 --- a/internal/oplock/stale_test.go +++ b/internal/oplock/stale_test.go @@ -78,8 +78,8 @@ func TestOpLockStaleHolderIsUnknown(t *testing.T) { // Control: a live pid with the same token re-enters, so the refusal // above comes from the stale check alone. {name: "live pid same host", holder: Holder{PID: os.Getpid(), Command: "old", Token: stale, Host: host}, wantKnown: true, wantReent: true}, - // Another host cannot be checked: the record is taken as is. - {name: "dead pid other host", holder: Holder{PID: deadPID, Command: "old", Token: stale, Host: host + "-elsewhere"}, wantKnown: true, wantReent: true}, + // Another host cannot be checked, so its token cannot permit re-entry. + {name: "dead pid other host", holder: Holder{PID: deadPID, Command: "old", Token: stale, Host: host + "-elsewhere"}, wantKnown: true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -99,7 +99,7 @@ func TestOpLockStaleHolderIsUnknown(t *testing.T) { if he.Known != tc.wantKnown { t.Fatalf("Known = %v, want %v (holder %+v)", he.Known, tc.wantKnown, he.Holder) } - if strings.Contains(he.Error(), "999999") { + if !tc.wantKnown && strings.Contains(he.Error(), "999999") { t.Errorf("error names the dead pid: %q", he.Error()) } }) From 6ccff0744295e5a9dff9d4aec510c0144a34beab Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:40:47 -0400 Subject: [PATCH 02/10] fix(deploy): reject empty state and uncertain image snapshots (D-0609) --- internal/deploy/apply_remote_images.go | 16 +++++++- internal/deploy/apply_remote_images_test.go | 42 +++++++++++++++++++++ internal/deploy/state.go | 5 +++ internal/deploy/state_empty_test.go | 25 ++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 internal/deploy/apply_remote_images_test.go create mode 100644 internal/deploy/state_empty_test.go diff --git a/internal/deploy/apply_remote_images.go b/internal/deploy/apply_remote_images.go index e6b58d16c..ac277001b 100644 --- a/internal/deploy/apply_remote_images.go +++ b/internal/deploy/apply_remote_images.go @@ -49,7 +49,16 @@ func saveImageTags(ctx context.Context, r Remote, refs []string) ([]imageTag, er if len(refs) == 0 { return nil, nil } - script := fmt.Sprintf(`for img in %s; do id=$(docker image inspect --format '{{.Id}}' "$img" 2>/dev/null) && printf '%%s %%s\n' "$id" "$img"; done; exit 0`, strings.Join(quoteAll(refs), " ")) + script := fmt.Sprintf(`for img in %s; do +if id=$(docker image inspect --format '{{.Id}}' "$img" 2>&1); then + printf '%%s %%s\n' "$id" "$img" +else + case "$id" in + "Error response from daemon: No such image: $img"|"Error: No such image: $img") ;; + *) printf 'cannot inspect image %%s before pull\n' "$img" >&2; exit 1 ;; + esac +fi +done`, strings.Join(quoteAll(refs), " ")) out, err := r.Run(ctx, shScript(script)) if err != nil { return nil, fmt.Errorf("reading the image ids on %s before the pull: %w", r.Label(), err) @@ -60,9 +69,12 @@ func saveImageTags(ctx context.Context, r Remote, refs []string) ([]imageTag, er } var saved []imageTag for _, line := range strings.Split(string(out), "\n") { + if line == "" { + continue + } f := strings.Fields(line) if len(f) != 2 || !imageIDRe.MatchString(f[0]) || !want[f[1]] { - continue + return nil, fmt.Errorf("reading the image ids on %s before the pull: invalid inspect response", r.Label()) } saved = append(saved, imageTag{ID: f[0], Ref: f[1]}) } diff --git a/internal/deploy/apply_remote_images_test.go b/internal/deploy/apply_remote_images_test.go new file mode 100644 index 000000000..575ac3eef --- /dev/null +++ b/internal/deploy/apply_remote_images_test.go @@ -0,0 +1,42 @@ +package deploy + +import ( + "context" + "os/exec" + "testing" +) + +// imageInspectRemote executes only a shell function double, never Docker. +type imageInspectRemote struct { + *fakeRemote + answer string +} + +func (r imageInspectRemote) Run(ctx context.Context, command string) ([]byte, error) { + script := "docker() { " + r.answer + "; }; " + unshell(command) + return exec.CommandContext(ctx, "sh", "-c", script).Output() +} + +func TestSaveImageTagsInspectOutcomes(t *testing.T) { + for _, tc := range []struct { + name, answer string + wantErr bool + count int + }{ + {"present", "printf '%s\\n' " + q(fakeImageID("a")), false, 1}, + {"absent", "printf '%s\\n' 'Error response from daemon: No such image: web:1' >&2; return 1", false, 0}, + {"daemon unavailable", "printf '%s\\n' 'Cannot connect to the Docker daemon' >&2; return 1", true, 0}, + {"permission denied", "printf '%s\\n' 'permission denied' >&2; return 1", true, 0}, + {"empty success", "return 0", true, 0}, + {"malformed success", "printf '%s\\n' invalid", true, 0}, + {"successful warning", "printf '%s\\n' warning >&2; printf '%s\\n' " + q(fakeImageID("a")), true, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + r := imageInspectRemote{newFakeRemote(t), tc.answer} + saved, err := saveImageTags(context.Background(), r, []string{"web:1"}) + if (err != nil) != tc.wantErr || len(saved) != tc.count { + t.Fatalf("saved=%v err=%v", saved, err) + } + }) + } +} diff --git a/internal/deploy/state.go b/internal/deploy/state.go index 7c630f67b..5d3338698 100644 --- a/internal/deploy/state.go +++ b/internal/deploy/state.go @@ -115,6 +115,11 @@ func LoadDeployState(projectRoot string) (StrategyState, error) { if err := json.Unmarshal(data, &s); err != nil { return StrategyState{}, fmt.Errorf("parsing deploy state: %w", err) } + // Existing legacy records have a strategy, while v1.5 records carry a + // schema version. A structurally empty record must not select legacy rollback. + if s.SchemaVersion == 0 && s.Strategy == "" { + return StrategyState{}, fmt.Errorf("parsing deploy state: record has no schema version or strategy") + } return s, nil } diff --git a/internal/deploy/state_empty_test.go b/internal/deploy/state_empty_test.go new file mode 100644 index 000000000..71e6c3cf0 --- /dev/null +++ b/internal/deploy/state_empty_test.go @@ -0,0 +1,25 @@ +package deploy + +import ( + "os" + "path/filepath" + "testing" +) + +func TestLoadDeployStateRejectsEmptyRecords(t *testing.T) { + for _, body := range []string{"null", "{}", `{"unknown":true}`} { + t.Run(body, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, DeployStateFile) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0600); err != nil { + t.Fatal(err) + } + if _, err := LoadDeployState(dir); err == nil { + t.Fatal("empty state must not permit legacy rollback fallback") + } + }) + } +} From bba5ee7793a0a455cde99db32c8e5a96a565f6da Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:40:52 -0400 Subject: [PATCH 03/10] fix(reconcile): honor effective init container labels (D-0609) --- internal/reconcile/containers_init.go | 43 ++++++++++++++------------- internal/reconcile/containers_test.go | 8 +++++ 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/internal/reconcile/containers_init.go b/internal/reconcile/containers_init.go index 3a9f59fd7..76c6cc515 100644 --- a/internal/reconcile/containers_init.go +++ b/internal/reconcile/containers_init.go @@ -19,17 +19,14 @@ import ( "gopkg.in/yaml.v3" ) -// initContainerLabel is the label docker.cleanupInitContainers removes by. -const initContainerLabel = "nself.type=init-container" - type composeLabels struct { Services map[string]struct { Labels yaml.Node `yaml:"labels"` } `yaml:"services"` } -// initContainerServices returns the services carrying initContainerLabel in -// any of files. +// initContainerServices returns services whose effective nself.type label is +// init-container after applying label overrides in Compose file order. func initContainerServices(files []string) (map[string]bool, error) { out := map[string]bool{} for _, path := range files { @@ -45,31 +42,37 @@ func initContainerServices(files []string) (map[string]bool, error) { return nil, fmt.Errorf("parsing compose file %s: %w", path, err) } for name, svc := range doc.Services { - if hasLabel(&svc.Labels, initContainerLabel) { - out[name] = true + if value, present := labelValue(&svc.Labels, "nself.type"); present { + if value == "init-container" { + out[name] = true + } else { + delete(out, name) + } } } } return out, nil } -// hasLabel reports whether a compose labels node (map or list) holds kv. -func hasLabel(n *yaml.Node, kv string) bool { - key, val, _ := strings.Cut(kv, "=") +// labelValue reads one Compose label, preserving whether an override sets it. +func labelValue(n *yaml.Node, key string) (string, bool) { switch n.Kind { case yaml.MappingNode: - var m map[string]string - return n.Decode(&m) == nil && m[key] == val - case yaml.SequenceNode: - var l []string - if n.Decode(&l) != nil { - return false + var labels map[string]string + if n.Decode(&labels) == nil { + value, ok := labels[key] + return value, ok } - for _, e := range l { - if strings.TrimSpace(e) == kv { - return true + case yaml.SequenceNode: + var labels []string + if n.Decode(&labels) == nil { + for i := len(labels) - 1; i >= 0; i-- { + k, value, _ := strings.Cut(labels[i], "=") + if k == key { + return value, true + } } } } - return false + return "", false } diff --git a/internal/reconcile/containers_test.go b/internal/reconcile/containers_test.go index 29544d4bb..5f0397acd 100644 --- a/internal/reconcile/containers_test.go +++ b/internal/reconcile/containers_test.go @@ -135,6 +135,14 @@ func TestInitContainerServices(t *testing.T) { if want := map[string]bool{"meilisearch-init": true, "plugin-init": true}; !reflect.DeepEqual(got, want) { t.Errorf("initContainerServices = %v, want %v", got, want) } + override := write("override.yml", "services:\n meilisearch-init:\n labels: [\"nself.type=service\"]\n plugin-init:\n labels:\n extra: value\n") + got, err = initContainerServices([]string{base, frag, override}) + if err != nil { + t.Fatal(err) + } + if want := map[string]bool{"plugin-init": true}; !reflect.DeepEqual(got, want) { + t.Errorf("merged init services = %v, want %v", got, want) + } if _, err := initContainerServices([]string{write("bad.yml", "services: [")}); err == nil { t.Error("a broken compose file must be an error (container impact unknown), not an empty set") } From cc04f956ac715c2697c6a8f62261b72869db4fb2 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:40:58 -0400 Subject: [PATCH 04/10] fix(build): distinguish nginx variables from templates (D-0609) --- internal/build/plugin_configs_test.go | 3 +++ internal/build/plugin_nginx.go | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/internal/build/plugin_configs_test.go b/internal/build/plugin_configs_test.go index ee8586e77..02787f87a 100644 --- a/internal/build/plugin_configs_test.go +++ b/internal/build/plugin_configs_test.go @@ -191,6 +191,9 @@ func TestPluginSnippetSkip(t *testing.T) { hasSSL bool want string // substring of the reason; "" = injected }{ + {"comment placeholder", "# ${NSELF_DOMAIN}\nserver { listen 80; }\n", false, ""}, + {"native uppercase variable", "map $host $UPSTREAM { default localhost; }\nserver { location / { proxy_pass http://${UPSTREAM}; } }\n", false, ""}, + {"native route variable", "map $host $CUSTOM_ROUTE { default localhost; }\nserver { location / { proxy_pass http://${CUSTOM_ROUTE}; } }\n", false, ""}, {"valid server block", "server {\n listen 80;\n server_name a.example.com;\n location / { return 204; }\n}\n", false, ""}, {"nginx variable in braces", "server {\n listen 80;\n location / { return 301 https://${host}$request_uri; }\n}\n", false, ""}, {"upstream and map at top level", "upstream u { server a:1; }\nmap $http_upgrade $c { default upgrade; }\nserver { listen 80; }\n", false, ""}, diff --git a/internal/build/plugin_nginx.go b/internal/build/plugin_nginx.go index 2f4ccf194..08fdfccc1 100644 --- a/internal/build/plugin_nginx.go +++ b/internal/build/plugin_nginx.go @@ -109,8 +109,8 @@ func InjectPluginNginxRoutes(workdir, pluginDir string, cfg *config.Config) (int return injectPluginNginxRoutesVia(newDiskSink(workdir), workdir, pluginDir, cfg, true) } -// placeholderRE matches a ${NAME} template variable buildTemplateVars does -// not provide. nginx's own variables are lower case, so ${host} is left alone. +// placeholderRE finds template-shaped variables; the parsed directive and +// known template vocabulary distinguish them from comments and nginx variables. var placeholderRE = regexp.MustCompile(`\$\{[A-Z][A-Z0-9_]*\}`) // serverOnly lists directives nginx accepts only inside server (or deeper): @@ -124,7 +124,7 @@ var serverOnly = map[string]bool{"location": true, "if": true, "listen": true, " // nginx/sites, or "" to inject it. One bad file in nginx/sites stops nginx, // taking every route down, and installed plugin copies can be old, so the // build checks what it can before nginx does (D-0478 review): -// - a ${NAME} the CLI does not render (nginx reads it as an unknown variable); +// - a known template name that remains unrendered in directive arguments; // - text that does not parse, or a top-level directive that is valid only // inside a server block (a bare location block); // - a load of the base domain's certificate pair (the ${SSL_DIR} path) while @@ -132,13 +132,19 @@ var serverOnly = map[string]bool{"location": true, "if": true, "listen": true, " // custom before the pair exists), which would also block the ACME // challenge that issues it. func pluginSnippetSkip(rendered string, vars map[string]string, hasSSL bool) string { - if m := placeholderRE.FindString(rendered); m != "" { - return "it uses " + m + ", which nself does not provide" - } tree, err := routemodel.Parse(rendered) if err != nil { return "it does not parse: " + err.Error() } + for _, b := range tree { + for _, m := range placeholderRE.FindAllString(b.Render(), -1) { + name := strings.TrimSuffix(strings.TrimPrefix(m, "${"), "}") + _, known := vars[name] + if known || name == "DOMAIN" || name == "NSELF_DOMAIN" || name == "GRAFANA_ROUTE" { + return "it uses " + m + ", which nself does not provide" + } + } + } for _, b := range tree { if serverOnly[b.Name] { return "it has a top-level " + b.Name + " directive, which nginx accepts only inside a server block" From 908de3dcc33bf0ccaf9b04d2475b650d51cab9e7 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:41:00 -0400 Subject: [PATCH 05/10] fix(database): preserve Hasura metadata and directory identity (D-0609) --- internal/database/hasura_apply_layout.go | 13 +++++++++---- internal/database/hasura_apply_layout_test.go | 18 +++++++++++++++++- .../database/hasura_archive_layout_test.go | 18 ++++++++++++++++++ internal/database/hasura_export_layout.go | 12 +++++++++--- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/internal/database/hasura_apply_layout.go b/internal/database/hasura_apply_layout.go index 75c981471..a8ce75c40 100644 --- a/internal/database/hasura_apply_layout.go +++ b/internal/database/hasura_apply_layout.go @@ -4,7 +4,7 @@ package database // D-0564). nself writes its own layout (a config.yaml index of !include lines // at the metadata root, one .yaml per top-level key; hasura_export_layout.go), // which only the in-process include resolver reads. The Hasura CLI expects its -// own project (hasura/config.yaml naming metadata_directory, then version.yaml +// own project (hasura/config.yaml, optional metadata_directory, then version.yaml // and databases/), so it is used only for a real Hasura CLI project. // Inputs: the hasura/ project directory and its metadata/ directory. // Outputs: isHasuraCLIProject. @@ -27,7 +27,7 @@ type hasuraCLIProjectConfig struct { // isHasuraCLIProject reports whether hasuraDir is a Hasura CLI project whose // metadata the hasura binary should apply: hasuraDir/config.yaml names a -// metadata_directory that resolves to metadataDir, and metadataDir carries no +// metadata_directory (default metadata) identifying metadataDir, which carries no // nself config.yaml index (that index is nself's layout, which the Hasura CLI // cannot read). func isHasuraCLIProject(hasuraDir, metadataDir string) bool { @@ -44,10 +44,15 @@ func isHasuraCLIProject(hasuraDir, metadataDir string) bool { } dir := strings.TrimSpace(c.MetadataDirectory) if dir == "" { - return false + dir = "metadata" } if !filepath.IsAbs(dir) { dir = filepath.Join(hasuraDir, dir) } - return filepath.Clean(dir) == filepath.Clean(metadataDir) + configured, err := os.Stat(dir) + if err != nil || !configured.IsDir() { + return false + } + metadata, err := os.Stat(metadataDir) + return err == nil && metadata.IsDir() && os.SameFile(configured, metadata) } diff --git a/internal/database/hasura_apply_layout_test.go b/internal/database/hasura_apply_layout_test.go index 066b8b67a..6d7846159 100644 --- a/internal/database/hasura_apply_layout_test.go +++ b/internal/database/hasura_apply_layout_test.go @@ -128,7 +128,7 @@ func TestIsHasuraCLIProject(t *testing.T) { {"cli project", map[string]string{"hasura/config.yaml": "metadata_directory: metadata\n"}, true}, {"cli project ./metadata", map[string]string{"hasura/config.yaml": "metadata_directory: ./metadata\n"}, true}, {"no project config", map[string]string{"hasura/metadata/tables.yaml": "[]\n"}, false}, - {"no metadata_directory", map[string]string{"hasura/config.yaml": "version: 3\n"}, false}, + {"default metadata_directory", map[string]string{"hasura/config.yaml": "version: 3\n"}, true}, {"other metadata_directory", map[string]string{"hasura/config.yaml": "metadata_directory: md\n"}, false}, {"unparsable config", map[string]string{"hasura/config.yaml": "metadata_directory: [\n"}, false}, {"nself index wins", map[string]string{ @@ -150,3 +150,19 @@ func TestIsHasuraCLIProject(t *testing.T) { } } } + +func TestIsHasuraCLIProjectSymlinkDirectory(t *testing.T) { + project := t.TempDir() + metadata := filepath.Join(project, "metadata") + if err := os.Mkdir(metadata, 0o755); err != nil { + t.Fatal(err) + } + alias := filepath.Join(project, "linked-metadata") + if err := os.Symlink(metadata, alias); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + writeFixtureFile(t, project, "config.yaml", "metadata_directory: linked-metadata\n") + if !isHasuraCLIProject(project, metadata) { + t.Fatal("metadata directory aliases must identify the same CLI project") + } +} diff --git a/internal/database/hasura_archive_layout_test.go b/internal/database/hasura_archive_layout_test.go index 24f88605f..fe89da922 100644 --- a/internal/database/hasura_archive_layout_test.go +++ b/internal/database/hasura_archive_layout_test.go @@ -153,6 +153,24 @@ func TestHasuraLayoutExportEqualsArchive(t *testing.T) { } } +// Keep-chomp YAML strings must preserve every trailing newline through includes. +func TestHasuraLayoutPreservesTrailingNewlines(t *testing.T) { + data := []byte(`{"actions":[{"definition":{"handler":"https://example.test\n\n\n"}}]}`) + dir := t.TempDir() + if err := writeMetadataTree(dir, data); err != nil { + t.Fatal(err) + } + resolved, err := resolveIncludes(dir, filepath.Join(dir, metadataIndexFile)) + if err != nil { + t.Fatal(err) + } + var doc interface{} + if err := yaml.Unmarshal(resolved, &doc); err != nil { + t.Fatal(err) + } + sameJSON(t, convertYAMLToJSON(doc), data) +} + // An older monolithic archive (one config.yaml, everything inline) still resolves. func TestHasuraLayoutLegacyMonolithicStillResolves(t *testing.T) { data := layoutInput(t) diff --git a/internal/database/hasura_export_layout.go b/internal/database/hasura_export_layout.go index 6e65877c5..1d7a32b6b 100644 --- a/internal/database/hasura_export_layout.go +++ b/internal/database/hasura_export_layout.go @@ -12,7 +12,8 @@ package database // style) instead. The same JSON form is used when the body contains the text // "!include", with each "!include" written as "\u0021include", so a string value // is never mistaken for an include directive; YAML decodes it back to "!". -// Multi-line bodies without "!include" stay block YAML. +// Keep-chomp scalars also use JSON because the resolver trims trailing newlines. +// Other multi-line bodies without "!include" stay block YAML. // Pure and deterministic (sorted keys, no clock, no disk) except // writeMetadataTree, which only writes the files it is given. A key that is not // a safe file name is rejected. The layout is what applyViaIncludeResolver reads. @@ -22,6 +23,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "sort" "strings" @@ -30,6 +32,8 @@ import ( const metadataIndexFile = "config.yaml" +var metadataKeepChomp = regexp.MustCompile(`(?m)[|>](?:[1-9]?\+|\+[1-9])\n`) + // metadataFile is one file of the layout; Name is relative to the metadata dir. type metadataFile struct { Name string @@ -68,7 +72,7 @@ func metadataLayout(data []byte) ([]metadataFile, error) { } // marshalMetadataValue encodes one top-level value for its .yaml file: -// block YAML, except single-line bodies and bodies containing "!include", which +// block YAML, except single-line bodies, keep-chomp scalars, and "!include", which // become one line of JSON with "!include" escaped (see the header for why). func marshalMetadataValue(v interface{}) ([]byte, error) { body, err := yaml.Marshal(v) @@ -76,7 +80,9 @@ func marshalMetadataValue(v interface{}) ([]byte, error) { return nil, err } oneLine := strings.Count(strings.TrimRight(string(body), "\n"), "\n") == 0 - if !oneLine && !strings.Contains(string(body), "!include") { + // The textual include resolver trims terminal newlines, so keep-chomp + // scalars must use JSON escapes to preserve their exact string values. + if !oneLine && !strings.Contains(string(body), "!include") && !metadataKeepChomp.Match(body) { return body, nil } js, err := json.Marshal(v) From cbffaf3e559fa85f303d4e28b52e31671edee35e Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 09:41:03 -0400 Subject: [PATCH 06/10] fix(ci): cover data readers and spaced vendor paths (D-0609) --- scripts/ci/affected-pkgs.sh | 28 +++++++++++++++++++++++----- scripts/ci/affected-pkgs_test.sh | 16 +++++++++++----- scripts/ci/vendor-sync-check.sh | 6 +++--- scripts/ci/vendor-sync-check_test.sh | 14 +++++++++++--- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/scripts/ci/affected-pkgs.sh b/scripts/ci/affected-pkgs.sh index 637a64a9c..2d0383344 100644 --- a/scripts/ci/affected-pkgs.sh +++ b/scripts/ci/affected-pkgs.sh @@ -12,12 +12,12 @@ # - the package in its nearest enclosing package directory (its own # .go files, testdata/, data files read beside the code); a root # package only for root files and the root testdata/; -# - for a file outside every package directory (docs, scripts, -# .github/), every package whose .go files name its directory +# - for every non-Go file (including docs, scripts, .github/), +# every package whose .go files name its directory # path (`dir/` or `/dir"` for a top-level one), its top-level # directory as a quoted literal, or (for a root file) its file # name: tests that read the wiki or a CI script are found that way. -# Seeds from .go and embedded files are closed over reverse imports +# Seeds from .go, embedded files and deleted data owners are closed over reverse imports # (`go list -deps`, test imports included); the other seeds only # feed their own package's tests, so they are listed as they are. Prints `./...` alone when go.mod, go.sum, # vendor/ or anything under internal/errs/ changed (everything @@ -114,8 +114,9 @@ awk -F'|' -v modpath="$MODPATH" -v moddir="$MODDIR" ' sed -n 's/^P //p' "$TMP/mapped" > "$TMP/seeds" sed -n 's/^D //p' "$TMP/mapped" > "$TMP/direct" -# Files outside every package: find the module .go files that name them. -sed -n 's/^U //p' "$TMP/mapped" | while IFS= read -r f; do +# Data can be owned by one package and read by another. Scan every non-Go +# path, including owned data, for those readers. +sed '/\.go$/d' "$TMP/changed" | while IFS= read -r f; do case "$f" in */*/*) printf '%s\n' "${f%/*}" @@ -149,6 +150,23 @@ if [ -s "$TMP/patterns" ]; then fi fi +# A removed embedded file no longer appears in go list's EmbedFiles. Close +# deleted data owners over imports conservatively, so wildcard embeds still +# reach their consumers when another matching file keeps the pattern valid. +while IFS= read -r f; do + [ -e "$f" ] && continue + case "$f" in *.go) continue ;; esac + d="$f" + while [ "${d%/*}" != "$d" ]; do + d="${d%/*}" + awk -F'|' -v dir="$MODDIR/$d" '$2 == dir { print $1 }' "$TMP/graph" + done + case "$f" in + */*) ;; + *) awk -F'|' -v dir="$MODDIR" '$2 == dir { print $1 }' "$TMP/graph" ;; + esac +done < "$TMP/changed" >> "$TMP/seeds" + if [ ! -s "$TMP/seeds" ] && [ ! -s "$TMP/direct" ]; then echo "affected-pkgs: no Go package is affected by this change set" >&2 exit 0 diff --git a/scripts/ci/affected-pkgs_test.sh b/scripts/ci/affected-pkgs_test.sh index 4504521dd..c8bea94c3 100755 --- a/scripts/ci/affected-pkgs_test.sh +++ b/scripts/ci/affected-pkgs_test.sh @@ -38,19 +38,20 @@ printf 'module example.invalid/fx\n\ngo %s\n' "$GOV" > "$R/go.mod" cat > "$R/a/a.go" <<'EOF' package a -import _ "embed" +import "embed" -//go:embed assets/conf.yaml -var Conf string +//go:embed assets/*.yaml +var Conf embed.FS EOF printf 'key: one\n' > "$R/a/assets/conf.yaml" +printf 'key: two\n' > "$R/a/assets/other.yaml" printf 'package assets\n' > "$R/a/assets/assets.go" cat > "$R/b/b.go" <<'EOF' package b import "example.invalid/fx/a" -func Conf() string { return a.Conf } +func Conf() ([]byte, error) { return a.Conf.ReadFile("assets/other.yaml") } EOF printf 'package c\n' > "$R/c/c.go" printf 'input\n' > "$R/c/testdata/in.txt" @@ -64,6 +65,7 @@ import ( ) func TestGuide(t *testing.T) { + _, _ = os.ReadFile("../c/testdata/in.txt") if _, err := os.ReadFile("../docs/guide.md"); err != nil { t.Fatal(err) } @@ -77,6 +79,7 @@ import _ "example.invalid/fx/d" EOF printf '# guide\n' > "$R/docs/guide.md" printf 'todo\n' > "$R/notes/todo.txt" +(cd "$R" && go test ./...) git -C "$R" init -q git -C "$R" add -A git -C "$R" commit -qm base @@ -119,6 +122,7 @@ check() { case "$c" in edit:*) printf '// changed\n' >> "$R/${c#edit:}" ;; new:*) mkdir -p "$(dirname "$R/${c#new:}")"; printf 'new\n' > "$R/${c#new:}" ;; + delete:*) rm "$R/${c#delete:}" ;; stage) git -C "$R" add -A ;; commit) git -C "$R" add -A && git -C "$R" commit -qm change ;; esac @@ -144,7 +148,9 @@ cases() { # cases