amadan
appconfig: pin the two properties the re-review proved unpinned
Both were correct by construction and caught by nothing. A transient bucket blip printing the stale-secrets notice — whose remedy is "delete this file by hand" — would have operators removing live production files mid-incident; dropping !c.known from the guard left the suite green until the negative assertion here. And moving the header prepend after the up-to-date check would rewrite a secrets-bearing root-owned file every tick forever; the socket shape had its idempotence pin, the addr shape now has its mtime one. Both mutations verified caught.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 file changed,
+35
−0
internal/appconfig/syncexec_test.go+35 −0
diff --git a/internal/appconfig/syncexec_test.go b/internal/appconfig/syncexec_test.go| index 8c65f7371..a5f12f875 100644 |
| --- a/internal/appconfig/syncexec_test.go |
| +++ b/internal/appconfig/syncexec_test.go |
| @@ -10,6 +10,7 @@ import ( |
| "slices" |
| "strings" |
| "testing" |
| + "time" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -650,6 +651,30 @@ func TestSyncExecOptedInAddrRouteMaterializesFile(t *testing.T) { |
| if string(got) != want { |
| t.Fatalf("config env = %q, want %q (issue #18's required header, then the vars)", got, want) |
| } |
| + |
| + // Idempotence must survive the header (2026-08-03 re-review, IMPORTANT |
| + // 7): the up-to-date comparison has to include the header bytes, or an |
| + // unchanged pass rewrites this secrets-bearing root-owned file every |
| + // tick forever. The socket shape has this pin; before this assertion, |
| + // moving the header prepend after the comparison left the whole suite |
| + // green. mtime is the observable: a second pass over unchanged config |
| + // must not touch the file. |
| + path := filepath.Join(dd, "demo.go.tito.io.config.env") |
| + before, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + time.Sleep(10 * time.Millisecond) // ensure a rewrite would move mtime |
| + if err := SyncExec(ctx, reg, s, pk, dd, func(string) error { return nil }); err != nil { |
| + t.Fatalf("SyncExec (second pass): %v", err) |
| + } |
| + after, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !after.ModTime().Equal(before.ModTime()) { |
| + t.Fatalf("an unchanged second pass rewrote the config file (mtime %v -> %v)", before.ModTime(), after.ModTime()) |
| + } |
| } |
| // TestSyncExecUnoptedAddrRouteIsSkipped pins the un-opted case: the exact |
| @@ -805,6 +830,16 @@ func TestSyncExecOptedInAddrRouteFetchFailureLogsAndLeavesFileAsIs(t *testing.T) |
| if !strings.Contains(out, "SlowDown") { |
| t.Fatalf("stdout = %q, want the underlying fetch error surfaced", out) |
| } |
| + // The mutual exclusion the ruling depends on (2026-08-03 re-review, |
| + // IMPORTANT 6): a transient blip must print ONLY the unresolved line — |
| + // never the stale-secrets notice, whose remedy is "delete this file by |
| + // hand". Dropping !c.known from that notice's guard would turn every |
| + // bucket blip into an instruction to rm a live production file |
| + // mid-incident, and before this assertion the whole suite stayed green |
| + // under exactly that mutation. |
| + if strings.Contains(out, "delete it by hand") { |
| + t.Fatalf("stdout = %q — a transient fetch failure produced the stale-file removal advice", out) |
| + } |
| } |
| // TestSyncExecOptedInAddrRouteIsNeverBounced is the structural pin the |