# Migration Step 4 Implementation Plan ## Status Follow-up remediation is required. The public-facade adapter boundary is implemented, but the post-implementation review found an error-classification 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 [Step 4 public-facade adapter roadmap](step4.md) by correcting the review findings without changing the intended 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 [testing policy](../policy/testing.md) governs test value and ownership. The [documentation policy](../policy/documentation.md) governs completion bookkeeping and canonical ownership. ## Constraints - Keep this work in the Scriptorium repository. Do not modify the sibling Promptkit repository or infer that Migration Step 5 is complete merely from its local presence. - Preserve the public `ArtifactReader`, error sentinel, CLI, HTTP, containment, artifact-size, formatting, timeout, credential, and redaction contracts. - Do not add compatibility shims, begin framework extraction, change the module path, or add dependencies. - Keep tests deterministic, offline, independent of machine MIME databases, and focused at the narrowest stable owner. - Preserve unrelated working-tree changes. ## Completed Work The original five implementation stages 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 public request-error identities required by HTTP; - the restricted artifact reader and its containment and byte-limit policy now belong to the HTTP adapter and use public artifact types; - prepared-run formatting and CLI `run` and `render` consume public prepared, request, and result values; - the HTTP handler consumes a local interface expressed in public types, and CLI `serve` injects the restricted reader into the public engine; - the legacy restricted framework reader was removed, current-behavior documentation was reconciled, and an initial dependency guard was added; and - the initial validation pass covered the full suite, vet, build, race and repeated tests, maintained examples, configuration loading, links, and whitespace. The post-implementation review confirmed the production dependency direction but found that the completion gate needs the corrections below. ## Stage 1: Preserve The Active Public Error Category Correct `mapPublicError` so a public error returned by an injected collaborator does not suppress the public category added by the operation currently being performed. The current early return from `hasPublicError` is too broad: for example, an artifact reader that returns `ErrInvalidRequest` is wrapped by the runner's internal artifact-load category, but the facade returns before adding public `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.