Implement ADR-0007
This commit is contained in:
@@ -296,8 +296,9 @@ func TestRunResumeSelectsConfiguredOrPerUserCheckpointRoot(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("without resume avoids checkpoint root resolution", func(t *testing.T) {
|
||||
t.Run("disabled avoids checkpoint root resolution", func(t *testing.T) {
|
||||
roots := newStateTestRoots(t)
|
||||
replaceStateTestConfigLine(t, roots.config, " enabled: true\n", " enabled: false\n")
|
||||
removeStateTestConfigLine(t, roots.config, fmt.Sprintf(" directory: %q\n", roots.checkpoints))
|
||||
opts := newStateTestHarness().options()
|
||||
opts.UserCacheDir = func() (string, error) { return "", errors.New("checkpoint cache must not be resolved") }
|
||||
@@ -308,6 +309,16 @@ func TestRunResumeSelectsConfiguredOrPerUserCheckpointRoot(t *testing.T) {
|
||||
assertStateTestOutput(t, roots.output)
|
||||
assertAbsent(t, roots.checkpoints)
|
||||
})
|
||||
|
||||
t.Run("resume requires enabled checkpoint recording", func(t *testing.T) {
|
||||
roots := newStateTestRoots(t)
|
||||
replaceStateTestConfigLine(t, roots.config, " enabled: true\n", " enabled: false\n")
|
||||
result := runStateTest(t, roots, newStateTestHarness().options(), true, true, "bypass")
|
||||
if result.code != 1 || !strings.Contains(result.stderr, "--resume requires cache.checkpoints.enabled: true") {
|
||||
t.Fatalf("code=%d stdout=%q stderr=%q", result.code, result.stdout, result.stderr)
|
||||
}
|
||||
assertNoRunState(t, roots)
|
||||
})
|
||||
}
|
||||
|
||||
func TestConfigCommandsDoNotResolveRunState(t *testing.T) {
|
||||
|
||||
@@ -135,7 +135,7 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i
|
||||
debug := fs.Bool("debug", false, "write a debug bundle")
|
||||
debugDir := fs.String("debug-dir", "", "debug bundle directory")
|
||||
llmProfile := fs.String("llm-profile", "", "LLM profile override")
|
||||
resume := fs.Bool("resume", false, "reuse and record compatible checkpoints")
|
||||
resume := fs.Bool("resume", false, "reuse compatible recorded checkpoints")
|
||||
chunkCache := chunkCacheFlag{}
|
||||
sessionID := sessionIDFlag{}
|
||||
referenceFlags := stringListFlag{}
|
||||
@@ -219,6 +219,10 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i
|
||||
fmt.Fprintf(stderr, "notarius: %v\n", err)
|
||||
return 1
|
||||
}
|
||||
if *resume && !cfg.Cache.Checkpoints.Enabled {
|
||||
fmt.Fprintln(stderr, "notarius: --resume requires cache.checkpoints.enabled: true")
|
||||
return 1
|
||||
}
|
||||
|
||||
startedAt := opts.Now().UTC()
|
||||
runID, err := opts.RunIDGenerator(startedAt)
|
||||
@@ -437,7 +441,10 @@ func checkpointHandlersForRun(
|
||||
sessionID string,
|
||||
resume bool,
|
||||
) (pipeline.CheckpointRecorder, pipeline.CheckpointLoader, error) {
|
||||
if !resume {
|
||||
if !settings.Enabled {
|
||||
if resume {
|
||||
return nil, nil, fmt.Errorf("--resume requires cache.checkpoints.enabled: true")
|
||||
}
|
||||
return pipeline.NoopCheckpointRecorder(), pipeline.NoopCheckpointLoader(), nil
|
||||
}
|
||||
identity, err := checkpoint.NewIdentity(checkpoint.IdentityInput{
|
||||
@@ -463,9 +470,12 @@ func checkpointHandlersForRun(
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("create checkpoint recorder: %w", err)
|
||||
}
|
||||
loader, err := checkpoint.NewFilesystemLoader(checkpointRoot, identity)
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("create checkpoint loader: %w", err)
|
||||
loader := pipeline.NoopCheckpointLoader()
|
||||
if resume {
|
||||
loader, err = checkpoint.NewFilesystemLoader(checkpointRoot, identity)
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("create checkpoint loader: %w", err)
|
||||
}
|
||||
}
|
||||
return recorder, loader, nil
|
||||
}
|
||||
|
||||
@@ -56,12 +56,8 @@ func TestRunStateSurfaceMatrix(t *testing.T) {
|
||||
t.Fatalf("chunk plan store roots = %v, want [%q]", storeRoots, roots.plans)
|
||||
}
|
||||
}
|
||||
if resume {
|
||||
assertAnyFile(t, roots.checkpoints)
|
||||
assertRestrictedTree(t, roots.checkpoints)
|
||||
} else {
|
||||
assertAbsent(t, roots.checkpoints)
|
||||
}
|
||||
assertAnyFile(t, roots.checkpoints)
|
||||
assertRestrictedTree(t, roots.checkpoints)
|
||||
if debug {
|
||||
bundle := onlyChildDir(t, roots.debug)
|
||||
assertFile(t, filepath.Join(bundle, "summary", "invocation.json"))
|
||||
@@ -97,6 +93,9 @@ func TestRunKeepsStateRootsIndependentAndReusesSelectedCheckpointRoot(t *testing
|
||||
if harness.chunkCalls != 1 {
|
||||
t.Fatalf("chunk calls after debug toggle = %d, want 1", harness.chunkCalls)
|
||||
}
|
||||
if harness.extractCalls != 2 {
|
||||
t.Fatalf("extract calls after two recording-only runs = %d, want 2", harness.extractCalls)
|
||||
}
|
||||
if got, err := os.ReadFile(planPath); err != nil || !bytes.Equal(got, initialPlan) {
|
||||
t.Fatalf("chunk plan changed after debug toggle: %v", err)
|
||||
}
|
||||
@@ -105,10 +104,14 @@ func TestRunKeepsStateRootsIndependentAndReusesSelectedCheckpointRoot(t *testing
|
||||
}
|
||||
|
||||
checkpointRoot := roots.checkpoints
|
||||
extractCallsBeforeResume := harness.extractCalls
|
||||
seed := runStateTest(t, roots, harness.options(), false, true, "auto")
|
||||
if seed.code != 0 {
|
||||
t.Fatalf("checkpoint seed code=%d stderr=%q", seed.code, seed.stderr)
|
||||
}
|
||||
if harness.extractCalls != extractCallsBeforeResume {
|
||||
t.Fatalf("extract calls after reusing recording-only checkpoint = %d, want %d", harness.extractCalls, extractCallsBeforeResume)
|
||||
}
|
||||
extractCalls := harness.extractCalls
|
||||
checkpointFiles := readTree(t, checkpointRoot)
|
||||
reused := runStateTest(t, roots, harness.options(), false, true, "auto")
|
||||
@@ -632,7 +635,7 @@ func newStateTestRoots(t *testing.T) stateTestRoots {
|
||||
t.Fatal(err)
|
||||
}
|
||||
roots.config = filepath.Join(base, "config.yml")
|
||||
config := fmt.Sprintf("version: 3\noutput:\n directory: %q\ncache:\n chunk_plans:\n directory: %q\n mode: auto\n checkpoints:\n directory: %q\ndebug:\n directory: %q\npipelines:\n sample:\n input: test/input\n chunk: test/chunk\n artifacts:\n items:\n extract: test/extract\n merge: test/merge\n normalize: test/normalize\n output: test/output\n", roots.output, roots.plans, roots.checkpoints, roots.debug)
|
||||
config := fmt.Sprintf("version: 3\noutput:\n directory: %q\ncache:\n chunk_plans:\n directory: %q\n mode: auto\n checkpoints:\n enabled: true\n directory: %q\ndebug:\n directory: %q\npipelines:\n sample:\n input: test/input\n chunk: test/chunk\n artifacts:\n items:\n extract: test/extract\n merge: test/merge\n normalize: test/normalize\n output: test/output\n", roots.output, roots.plans, roots.checkpoints, roots.debug)
|
||||
if err := os.WriteFile(roots.config, []byte(config), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
@@ -43,6 +43,7 @@ type ChunkPlanCacheConfig struct {
|
||||
}
|
||||
|
||||
type CheckpointCacheConfig struct {
|
||||
Enabled bool `json:"enabled"`
|
||||
Directory string `json:"directory,omitempty"`
|
||||
}
|
||||
type DebugConfig struct {
|
||||
|
||||
@@ -60,6 +60,7 @@ type FileChunkPlanCacheConfig struct {
|
||||
Mode *string `yaml:"mode,omitempty"`
|
||||
}
|
||||
type FileCheckpointCacheConfig struct {
|
||||
Enabled *bool `yaml:"enabled,omitempty"`
|
||||
Directory *string `yaml:"directory,omitempty"`
|
||||
}
|
||||
type FileDebugConfig struct {
|
||||
@@ -358,10 +359,15 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin
|
||||
}
|
||||
}
|
||||
}
|
||||
if fileCfg.Cache.Checkpoints != nil && fileCfg.Cache.Checkpoints.Directory != nil {
|
||||
c.Cache.Checkpoints.Directory = cleanOptionalPath(*fileCfg.Cache.Checkpoints.Directory)
|
||||
if strings.ContainsRune(c.Cache.Checkpoints.Directory, '\x00') {
|
||||
return fmt.Errorf("cache.checkpoints.directory must not contain NUL")
|
||||
if fileCfg.Cache.Checkpoints != nil {
|
||||
if fileCfg.Cache.Checkpoints.Enabled != nil {
|
||||
c.Cache.Checkpoints.Enabled = *fileCfg.Cache.Checkpoints.Enabled
|
||||
}
|
||||
if fileCfg.Cache.Checkpoints.Directory != nil {
|
||||
c.Cache.Checkpoints.Directory = cleanOptionalPath(*fileCfg.Cache.Checkpoints.Directory)
|
||||
if strings.ContainsRune(c.Cache.Checkpoints.Directory, '\x00') {
|
||||
return fmt.Errorf("cache.checkpoints.directory must not contain NUL")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ func TestDefaultReturnsDocumentedValuesAndIndependentMaps(t *testing.T) {
|
||||
if first.Output.Directory != "./notarius-output" || first.Debug.Directory != "./notarius-debug" {
|
||||
t.Fatalf("output/debug defaults = %#v, %#v", first.Output, first.Debug)
|
||||
}
|
||||
if first.Cache.ChunkPlans.Mode != pipeline.ChunkCacheAuto || first.Cache.ChunkPlans.Directory != "" || first.Cache.Checkpoints.Directory != "" {
|
||||
if first.Cache.ChunkPlans.Mode != pipeline.ChunkCacheAuto || first.Cache.ChunkPlans.Directory != "" || first.Cache.Checkpoints.Enabled || first.Cache.Checkpoints.Directory != "" {
|
||||
t.Fatalf("cache defaults = %#v", first.Cache)
|
||||
}
|
||||
if len(first.Pipelines) != 0 {
|
||||
@@ -91,6 +91,16 @@ func TestFileConfigRejectsUnknownCurrentAndRemovedFields(t *testing.T) {
|
||||
yaml: "version: 3\npipelines:\n main:\n input:\n module: seriatim\n unknown: true\n",
|
||||
want: "field unknown not found in module binding",
|
||||
},
|
||||
{
|
||||
name: "checkpoint field",
|
||||
yaml: "version: 3\ncache:\n checkpoints:\n unknown: true\n",
|
||||
want: "field unknown not found",
|
||||
},
|
||||
{
|
||||
name: "checkpoint enabled type",
|
||||
yaml: "version: 3\ncache:\n checkpoints:\n enabled: definitely\n",
|
||||
want: "cannot unmarshal",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
@@ -279,6 +289,7 @@ cache:
|
||||
directory: ./plans
|
||||
mode: bypass
|
||||
checkpoints:
|
||||
enabled: true
|
||||
directory: ./checkpoints
|
||||
debug:
|
||||
directory: ./debug
|
||||
@@ -290,7 +301,7 @@ debug:
|
||||
t.Fatalf("concurrency = %#v", cfg.Concurrency)
|
||||
}
|
||||
if cfg.Output.Directory != "./output" || cfg.Cache.ChunkPlans.Directory != "plans" || cfg.Cache.ChunkPlans.Mode != pipeline.ChunkCacheBypass ||
|
||||
cfg.Cache.Checkpoints.Directory != "checkpoints" || cfg.Debug.Directory != "./debug" {
|
||||
!cfg.Cache.Checkpoints.Enabled || cfg.Cache.Checkpoints.Directory != "checkpoints" || cfg.Debug.Directory != "./debug" {
|
||||
t.Fatalf("state sections = %#v, %#v, %#v, %#v", cfg.Output, cfg.Cache, cfg.Debug, cfg.Scriptorium)
|
||||
}
|
||||
if cfg.Output.Directory == cfg.Cache.ChunkPlans.Directory || cfg.Cache.ChunkPlans.Directory == cfg.Cache.Checkpoints.Directory || cfg.Cache.Checkpoints.Directory == cfg.Debug.Directory {
|
||||
@@ -298,6 +309,20 @@ debug:
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileConfigCheckpointEnabledCanBeExplicitlyDisabled(t *testing.T) {
|
||||
cfg := applyFileConfig(t, "version: 3\ncache:\n checkpoints:\n enabled: true\n")
|
||||
if !cfg.Cache.Checkpoints.Enabled || !cloneConfig(cfg).Cache.Checkpoints.Enabled {
|
||||
t.Fatalf("enabled checkpoint config was not retained: %#v", cfg.Cache.Checkpoints)
|
||||
}
|
||||
file := parseFileConfig(t, "version: 3\ncache:\n checkpoints:\n enabled: false\n")
|
||||
if err := cfg.ApplyFileConfig(file); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if cfg.Cache.Checkpoints.Enabled {
|
||||
t.Fatalf("explicit false checkpoint config was not applied: %#v", cfg.Cache.Checkpoints)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileConfigRejectsTrimmedKeyCollisions(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Reference in New Issue
Block a user