Record public facade boundary validation

docs/roadmap/implementation.md

@@ -2,16 +2,14 @@
 
 ## Status
 
-Follow-up remediation is required. The public-facade adapter boundary is
+Complete as of 2026-07-28. The public-facade adapter boundary and its
-implemented, but the post-implementation review found an error-classification
+post-implementation remediation are fully revalidated.
-gap, a machine-dependent HTTP artifact test, and an incomplete architecture
-guard. Migration Step 4 is not fully revalidated until the stages below pass.
 
 ## Objective
 
-Complete the target state in the
+This plan completed the target state in the
-[Step 4 public-facade adapter roadmap](step4.md) by correcting the review
+[Step 4 public-facade adapter roadmap](step4.md) without changing the intended
-findings without changing the intended CLI, HTTP, or public Go contracts.
+CLI, HTTP, or public Go contracts.
 
 The [accepted split decision](../adr/0002-split-promptkit-from-scriptorium.md)
 owns the long-term Promptkit/Scriptorium boundary. The
@@ -34,7 +32,7 @@ bookkeeping and canonical ownership.
 
 ## Completed Work
 
-The original five implementation stages established the Step 4 boundary:
+The original implementation established the Step 4 boundary:
 
 - the root facade now exposes `ArtifactReader` and `WithArtifactReader`, copies
   mutable artifact bodies, handles nil responses, and exposes the two specific
@@ -51,226 +49,14 @@ The original five implementation stages established the Step 4 boundary:
   repeated tests, maintained examples, configuration loading, links, and
   whitespace.
 
-The post-implementation review confirmed the production dependency direction
+The post-implementation remediation is also complete:
-but found that the completion gate needs the corrections below.
 
-## Stage 1: Preserve The Active Public Error Category
+- public error mapping retains collaborator identities while adding the active
-
+  operation's public category;
-Correct `mapPublicError` so a public error returned by an injected collaborator
+- HTTP artifact MIME coverage uses a built-in media type while retaining the
-does not suppress the public category added by the operation currently being
+  unknown-extension fallback check;
-performed.
+- the dependency guard recursively scans production adapter and formatter
-
+  sources and rejects descendants of forbidden framework package families; and
-The current early return from `hasPublicError` is too broad: for example, an
+- the final validation passed full and repeated tests, vet, a temporary-output
-artifact reader that returns `ErrInvalidRequest` is wrapped by the runner's
+  build, race checks, maintained examples, both maintained configuration files,
-internal artifact-load category, but the facade returns before adding public
+  import and link inspection, and whitespace validation.
-`ErrArtifactLoad`. The corrected mapping must:
-
-- first classify the internal operation or source category present in the
-  complete chain;
-- add the corresponding public category even when another public sentinel is
-  already nested in the chain;
-- retain the original collaborator error and every existing internal cause for
-  `errors.Is`;
-- preserve the dual `ErrInvalidRequest` plus specific-sentinel behavior for
-  `ErrProfileRequired` and `ErrAPIKeyEnvMissing`; and
-- return an already-public or otherwise unclassified error unchanged when no
-  internal category requires translation.
-
-Remove the blanket public-error short circuit rather than adding an
-artifact-specific exception. Keep `publicErrorFor` responsible for selecting
-the outer public category from internal identities, with specific causes
-checked before broad internal categories. Do not parse error text or change
-public method signatures.
-
-### Regression Tests
-
-Extend the public engine contract tests at the real `Engine.Prepare` and
-`Engine.Run` boundaries:
-
-- make an injected artifact reader return `ErrInvalidRequest` and assert that
-  the result matches both `ErrArtifactLoad` and the original
-  `ErrInvalidRequest`;
-- make an injected LLM client return a different public sentinel, such as
-  `ErrArtifactLoad`, and assert that the result matches both
-  `ErrLLMGenerate` and the original sentinel; and
-- retain the existing generic reader-error, nil-response, cancellation,
-  profile-required, and missing-credential cases without duplicating them.
-
-The second case protects the shared translation rule rather than only the
-reported artifact example. Do not add a cross-product of every public sentinel
-and failure category.
-
-Run:
-
-```bash
-go test .
-go test -count=20 -run 'Test.*(ArtifactReader|PublicError|LLM)' .
-```
-
-### Stage 1 Gate
-
-- Every categorized runner failure gains the public category for its active
-  operation even when a nested collaborator error is already public.
-- Nested custom error identities remain observable through `errors.Is`.
-- Existing broad and specific public error behavior remains unchanged.
-
-## Stage 2: Make HTTP Artifact MIME Coverage Deterministic
-
-Repair `TestRestrictedArtifactReaderReadsContainedFiles` without changing the
-implemented MIME policy. The contract remains:
-
-- call `mime.TypeByExtension` for a file extension; and
-- use `text/plain` only when that lookup returns an empty string.
-
-Replace the `.md` fixture with an extension in Go's built-in MIME table, such
-as `.html`, and assert the corresponding non-empty built-in result. Retain the
-separate unknown-extension case that asserts the `text/plain` fallback. Keep
-the contained relative and absolute path cases and all metadata assertions.
-
-Do not hard-code a platform-specific Markdown media type, consult system MIME
-files directly, mutate the process-global MIME registry, or weaken the test to
-accept either value. Change production code only if the deterministic test
-demonstrates that it does not implement the stated lookup-and-fallback rule.
-
-Run:
-
-```bash
-go test ./internal/adapter/http
-go test -count=20 ./internal/adapter/http
-```
-
-### Stage 2 Gate
-
-- The HTTP package passes repeatedly on a machine whose MIME database has no
-  Markdown registration.
-- The test independently protects successful MIME inference and the unknown
-  extension fallback.
-- Artifact metadata, containment, and byte-limit behavior are unchanged.
-
-## Stage 3: Strengthen The Public-Boundary Dependency Guard
-
-Update `internal/adapter/dependency_test.go` so the durable guard proves the
-entire intended production boundary rather than only today's flat directory
-layout.
-
-### Recursive Source Inspection
-
-Walk these roots recursively with `filepath.WalkDir`:
-
-- `internal/adapter/cli`;
-- `internal/adapter/http`; and
-- `internal/format`.
-
-Inspect every non-test `.go` file under those roots. Skip directories only when
-they are not Go source owned by the repository; do not silently skip nested
-adapter or formatter packages. Continue parsing imports with the standard
-library and `parser.ImportsOnly`.
-
-### Forbidden Package Families
-
-Represent each Promptkit-destined internal dependency as a package-family root:
-
-- `internal/domain`;
-- `internal/usecase`;
-- `internal/promptdef`;
-- `internal/prompt`;
-- `internal/profile`;
-- `internal/validate`;
-- `internal/llm`; and
-- `internal/artifact`.
-
-Reject an import when it equals a forbidden root or begins with that root plus
-`/`. The `internal/profile` rule therefore covers `internal/profile/builtin`
-and any future descendant. Do not reject the public root package or
-Scriptorium-owned adapter, configuration, defaults, and formatter packages.
-
-Diagnostics must identify the importing file and complete forbidden import
-path. Continue excluding `_test.go` files from the durable production
-architecture rule; test-boundary review remains a separate source inspection.
-
-### Guard Regression Test
-
-Extract only the small scanning or classification helper needed to exercise
-the guard. Using `t.TempDir`, create a nested Go source fixture that imports a
-descendant of a forbidden package family and assert that the guard reports it.
-Also include an allowed public-facade import so the test proves it is not
-overbroad. Keep this fixture test in the existing dependency-test file rather
-than creating another architecture-test owner.
-
-Run:
-
-```bash
-go test ./internal/adapter -run TestScriptoriumAdaptersUseOnlyPublicFrameworkBoundary
-go test -count=20 ./internal/adapter
-```
-
-### Stage 3 Gate
-
-- Nested production packages cannot bypass the scan.
-- Current or future descendants of forbidden framework packages cannot bypass
-  package-family matching.
-- Allowed public and Scriptorium-owned dependencies remain accepted.
-- Failures remain precise and actionable.
-
-## Stage 4: Revalidate And Reconcile Completion Records
-
-After Stages 1 through 3 pass, run the complete Step 4 validation from the
-Scriptorium repository:
-
-```bash
-go test ./...
-go vet ./...
-build_output="$(mktemp -d)"
-go build -o "$build_output/scriptorium" ./cmd/scriptorium
-go test -race . ./internal/adapter/http
-go test -count=20 . ./internal/adapter ./internal/adapter/cli ./internal/adapter/http ./internal/format
-go test ./internal/adapter/http -run TestMaintainedHTTPRunExampleMatchesRequestContract
-bash ./examples/render-markdown-summary.sh
-go run ./cmd/scriptorium render \
-  --config ./examples/config.full.yml \
-  --prompt generic.markdown_summary \
-  --input transcript=./examples/fixtures/transcript.md \
-  --input glossary=./examples/fixtures/glossary.yml \
-  --format text
-go run ./examples/go-library/prepare
-git diff --check
-```
-
-Also:
-
-- pass both maintained configuration files through the real configuration
-  loader;
-- inspect production and test imports under the adapter and formatter roots;
-- validate every local Markdown link changed by the remediation;
-- confirm no credential, private data, dependency, module, workspace, or local
-  `replace` change was introduced; and
-- inspect the final diff for unrelated work.
-
-Once every check passes:
-
-1. update the Step 4 completion summary in `step4.md` to mention the
-   post-implementation error and guard corrections without turning the feature
-   roadmap into an implementation log;
-2. update the Step 4 gate in `migration.md` so its validation claim accurately
-   reflects the successful follow-up pass;
-3. mark this implementation plan complete and move the remediation stages into
-   the completed-work summary; and
-4. leave the Step 5 gate unchanged unless its out-of-band repository,
-   governance, policy, and CI requirements have been independently confirmed.
-
-Do not begin extraction or modify Promptkit as part of this stage.
-
-### Stage 4 Gate
-
-- Every Step 4 completion criterion is satisfied.
-- The public error contract is protected against nested public-sentinel
-  collisions.
-- The default suite is deterministic across machine MIME configurations.
-- The dependency guard enforces recursive package-family boundaries.
-- Full tests, vet, build, race checks, repeated tests, maintained examples,
-  configuration loading, links, and whitespace validation pass.
-- Scriptorium's completion records accurately describe the validated state.
-
-## Open Questions
-
-None.

docs/roadmap/migration.md

@@ -175,8 +175,10 @@ framework behavior, and all tests and documented smoke commands pass.
 **Gate status:** Complete as of 2026-07-28. CLI `run`, `render`, and `serve`,
 the HTTP handler, and prepared-run formatting use the public facade; the
 restricted HTTP reader is injected through the public extension point. The
-dependency guard, full tests, vet, build, race checks, maintained examples, and
+post-implementation public-error, deterministic MIME, and recursive
-configuration smoke checks passed. Step 5 repository creation is next.
+dependency-guard corrections passed full tests, vet, build, race checks,
+maintained examples, and configuration smoke checks. Step 5 repository
+creation is next.
 
 ### Step 5: Create The Promptkit Repository
 

docs/roadmap/step4.md

@@ -2,10 +2,11 @@
 
 ## Status
 
-Complete as of 2026-07-28. The public-facade adapter boundary is established;
+Complete as of 2026-07-28. The public-facade adapter boundary and its
-the [implementation plan](implementation.md) records the completed work, and
+post-implementation error-classification, MIME-test, and dependency-guard
-the [main migration roadmap](migration.md) identifies repository creation as
+corrections are validated. The [implementation plan](implementation.md)
-the next gate.
+records the completed work, and the [main migration roadmap](migration.md)
+identifies repository creation as the next gate.
 
 ## Purpose
 
@@ -26,7 +27,8 @@ The CLI, HTTP handler, and prepared-run formatter now consume public engine
 values and errors. `serve` injects Scriptorium's HTTP-owned restricted artifact
 reader through the public extension point, while the root facade continues to
 compose the framework implementation inside this repository. A repository-level
-dependency test protects the direct-import boundary.
+dependency test recursively protects production adapter and formatter packages
+from framework-owned imports.
 
 ## Target State
 

engine_test.go

@@ -915,6 +915,12 @@ func TestArtifactReaderFailuresPreserveArtifactLoadErrors(t *testing.T) {
 			}},
 			wantNested: context.Canceled,
 		},
+		{
+			name:       "public reader error",
+			ctx:        context.Background(),
+			reader:     &recordingArtifactReader{err: scriptorium.ErrInvalidRequest},
+			wantNested: scriptorium.ErrInvalidRequest,
+		},
 	}
 
 	for _, tc := range tests {
@@ -936,6 +942,26 @@ func TestArtifactReaderFailuresPreserveArtifactLoadErrors(t *testing.T) {
 	}
 }
 
+func TestRunAddsLLMGenerateToCollaboratorPublicError(t *testing.T) {
+	engine := newContractEngineWithOptions(t, frameworkSchemaDir,
+		scriptorium.WithLLMClient(&fakeLLMClient{err: scriptorium.ErrArtifactLoad}),
+	)
+
+	_, err := engine.Run(context.Background(), scriptorium.RunRequest{
+		PromptID: frameworkMarkdownSummaryPromptID,
+		Inputs: map[string]scriptorium.ArtifactRef{
+			"transcript": scriptorium.Inline("Rin opens the gate."),
+			"glossary":   scriptorium.Inline("gate: A guarded passage."),
+		},
+	})
+	if !errors.Is(err, scriptorium.ErrLLMGenerate) {
+		t.Fatalf("expected ErrLLMGenerate, got %v", err)
+	}
+	if !errors.Is(err, scriptorium.ErrArtifactLoad) {
+		t.Fatalf("expected preserved ErrArtifactLoad, got %v", err)
+	}
+}
+
 func TestPrepareWithoutProfileMatchesSpecificPublicError(t *testing.T) {
 	promptDir := t.TempDir()
 	writePublicPromptFile(t, promptDir, "profile-required", "")

errors.go

@@ -13,9 +13,6 @@ func mapPublicError(err error) error {
 	if err == nil {
 		return nil
 	}
-	if hasPublicError(err) {
-		return err
-	}
 	publicErr := publicErrorFor(err)
 	if publicErr == nil {
 		return err
@@ -23,28 +20,6 @@ func mapPublicError(err error) error {
 	return fmt.Errorf("%w: %w", publicErr, err)
 }
 
-func hasPublicError(err error) bool {
-	for _, publicErr := range []error{
-		ErrInvalidConfig,
-		ErrInvalidRequest,
-		ErrPromptNotFound,
-		ErrProfileNotFound,
-		ErrProfileRequired,
-		ErrPromptLoad,
-		ErrProfileLoad,
-		ErrAPIKeyEnvMissing,
-		ErrArtifactLoad,
-		ErrPromptRender,
-		ErrLLMGenerate,
-		ErrValidation,
-	} {
-		if errors.Is(err, publicErr) {
-			return true
-		}
-	}
-	return false
-}
-
 func publicErrorFor(err error) error {
 	switch {
 	case errors.Is(err, promptdef.ErrPromptDefinitionNotFound):

internal/adapter/dependency_test.go

@@ -1,8 +1,10 @@
 package adapter_test
 
 import (
+	"fmt"
 	"go/parser"
 	"go/token"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -11,6 +13,24 @@ import (
 	"testing"
 )
 
+const scriptoriumModulePath = "gitea.maximumdirect.net/eric/scriptorium"
+
+var forbiddenFrameworkPackageRoots = []string{
+	scriptoriumModulePath + "/internal/domain",
+	scriptoriumModulePath + "/internal/usecase",
+	scriptoriumModulePath + "/internal/promptdef",
+	scriptoriumModulePath + "/internal/prompt",
+	scriptoriumModulePath + "/internal/profile",
+	scriptoriumModulePath + "/internal/validate",
+	scriptoriumModulePath + "/internal/llm",
+	scriptoriumModulePath + "/internal/artifact",
+}
+
+type forbiddenFrameworkImport struct {
+	filePath   string
+	importPath string
+}
+
 func TestScriptoriumAdaptersUseOnlyPublicFrameworkBoundary(t *testing.T) {
 	_, testFile, _, ok := runtime.Caller(0)
 	if !ok {
@@ -18,47 +38,95 @@ func TestScriptoriumAdaptersUseOnlyPublicFrameworkBoundary(t *testing.T) {
 	}
 
 	adapterDir := filepath.Dir(testFile)
-	directories := []string{
+	violations, err := findForbiddenFrameworkImports([]string{
 		filepath.Join(adapterDir, "cli"),
 		filepath.Join(adapterDir, "http"),
 		filepath.Join(adapterDir, "..", "format"),
+	})
+	if err != nil {
+		t.Fatalf("scan framework imports: %v", err)
 	}
-	forbidden := map[string]struct{}{
+	for _, violation := range violations {
-		"gitea.maximumdirect.net/eric/scriptorium/internal/domain":          {},
+		t.Errorf("%s imports forbidden framework package %s", violation.filePath, violation.importPath)
-		"gitea.maximumdirect.net/eric/scriptorium/internal/usecase":         {},
+	}
-		"gitea.maximumdirect.net/eric/scriptorium/internal/promptdef":       {},
+}
-		"gitea.maximumdirect.net/eric/scriptorium/internal/prompt":          {},
+
-		"gitea.maximumdirect.net/eric/scriptorium/internal/profile":         {},
+func TestForbiddenFrameworkImportScannerDetectsNestedPackageFamilies(t *testing.T) {
-		"gitea.maximumdirect.net/eric/scriptorium/internal/profile/builtin": {},
+	root := t.TempDir()
-		"gitea.maximumdirect.net/eric/scriptorium/internal/validate":        {},
+	nestedDir := filepath.Join(root, "nested", "adapter")
-		"gitea.maximumdirect.net/eric/scriptorium/internal/llm":             {},
+	if err := os.MkdirAll(nestedDir, 0o755); err != nil {
-		"gitea.maximumdirect.net/eric/scriptorium/internal/artifact":        {},
+		t.Fatalf("create nested source directory: %v", err)
 	}
 
-	for _, directory := range directories {
+	sourcePath := filepath.Join(nestedDir, "imports.go")
-		entries, err := os.ReadDir(directory)
+	source := `package nested
-		if err != nil {
+
-			t.Fatalf("read source directory %s: %v", directory, err)
+import (
-		}
+	_ "gitea.maximumdirect.net/eric/scriptorium"
-		for _, entry := range entries {
+	_ "gitea.maximumdirect.net/eric/scriptorium/internal/profile/builtin"
+)
+`
+	if err := os.WriteFile(sourcePath, []byte(source), 0o644); err != nil {
+		t.Fatalf("write nested source fixture: %v", err)
+	}
+
+	violations, err := findForbiddenFrameworkImports([]string{root})
+	if err != nil {
+		t.Fatalf("scan nested source fixture: %v", err)
+	}
+	if len(violations) != 1 {
+		t.Fatalf("expected one forbidden import, got %#v", violations)
+	}
+	if violations[0].filePath != sourcePath {
+		t.Fatalf("unexpected importing file: %q", violations[0].filePath)
+	}
+	wantImport := scriptoriumModulePath + "/internal/profile/builtin"
+	if violations[0].importPath != wantImport {
+		t.Fatalf("unexpected forbidden import: %q", violations[0].importPath)
+	}
+}
+
+func findForbiddenFrameworkImports(roots []string) ([]forbiddenFrameworkImport, error) {
+	var violations []forbiddenFrameworkImport
+	for _, root := range roots {
+		err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error {
+			if err != nil {
+				return err
+			}
 			if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".go") || strings.HasSuffix(entry.Name(), "_test.go") {
-				continue
+				return nil
 			}
 
-			path := filepath.Join(directory, entry.Name())
 			file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.ImportsOnly)
 			if err != nil {
-				t.Fatalf("parse imports in %s: %v", path, err)
+				return fmt.Errorf("parse imports in %s: %w", path, err)
 			}
 			for _, imported := range file.Imports {
 				importPath, err := strconv.Unquote(imported.Path.Value)
 				if err != nil {
-					t.Fatalf("parse import path in %s: %v", path, err)
+					return fmt.Errorf("parse import path in %s: %w", path, err)
 				}
-				if _, found := forbidden[importPath]; found {
+				if isForbiddenFrameworkImport(importPath) {
-					t.Errorf("%s directly imports forbidden framework package %s", path, importPath)
+					violations = append(violations, forbiddenFrameworkImport{
+						filePath:   path,
+						importPath: importPath,
+					})
 				}
 			}
+			return nil
+		})
+		if err != nil {
+			return nil, fmt.Errorf("walk source root %s: %w", root, err)
 		}
 	}
+	return violations, nil
+}
+
+func isForbiddenFrameworkImport(importPath string) bool {
+	for _, root := range forbiddenFrameworkPackageRoots {
+		if importPath == root || strings.HasPrefix(importPath, root+"/") {
+			return true
+		}
+	}
+	return false
 }

internal/adapter/http/artifact_reader_test.go

@@ -3,7 +3,6 @@ package httpadapter
 import (
 	"context"
 	"errors"
-	"mime"
 	"os"
 	"path/filepath"
 	"testing"
@@ -14,7 +13,7 @@ import (
 func TestRestrictedArtifactReaderReadsContainedFiles(t *testing.T) {
 	root := t.TempDir()
 	outside := t.TempDir()
-	inputPath := filepath.Join(root, "input.md")
+	inputPath := filepath.Join(root, "input.html")
 	if err := os.WriteFile(inputPath, []byte("allowed"), 0o644); err != nil {
 		t.Fatal(err)
 	}
@@ -34,17 +33,17 @@ func TestRestrictedArtifactReaderReadsContainedFiles(t *testing.T) {
 	}
 
 	for _, ref := range []scriptorium.ArtifactRef{
-		{Type: scriptorium.ArtifactRefFile, URI: "nested/../input.md"},
+		{Type: scriptorium.ArtifactRefFile, URI: "nested/../input.html"},
 		{Type: scriptorium.ArtifactRefFile, URI: inputPath},
 	} {
 		artifact, err := reader.Read(context.Background(), ref)
 		if err != nil {
 			t.Fatalf("read contained path %q: %v", ref.URI, err)
 		}
-		if artifact.Name != "input.md" || artifact.URI != inputPath || artifact.Size != int64(len("allowed")) || string(artifact.Body) != "allowed" {
+		if artifact.Name != "input.html" || artifact.URI != inputPath || artifact.Size != int64(len("allowed")) || string(artifact.Body) != "allowed" {
 			t.Fatalf("unexpected artifact metadata: %#v", artifact)
 		}
-		if artifact.ContentType != mime.TypeByExtension(".md") {
+		if artifact.ContentType != "text/html; charset=utf-8" {
 			t.Fatalf("unexpected artifact content type: %q", artifact.ContentType)
 		}
 		if artifact.Hash != artifactHash([]byte("allowed")) {