From ad115a225995245f0f2df465894f467c139390ad Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 27 Jul 2026 19:30:36 -0500 Subject: [PATCH] Add an implementation plan and roadmap for Step 4 of the migration plan --- docs/roadmap/implementation.md | 554 +++++++++++++++++++++++++++++++++ docs/roadmap/step4.md | 396 +++++++++++++++++++++++ 2 files changed, 950 insertions(+) create mode 100644 docs/roadmap/implementation.md create mode 100644 docs/roadmap/step4.md diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..12beb3e --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,554 @@ +# Migration Step 4 Implementation Plan + +## Status + +Proposed. Implement each stage in order and satisfy its gate before continuing. + +## Objective + +Implement the target state in the +[Step 4 public-facade adapter roadmap](step4.md): make Scriptorium's CLI, HTTP +handler, prepared-run formatter, and HTTP artifact policy consume only the +public framework facade, while preserving the current executable 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 current-behavior +updates and canonical ownership. + +## Constraints + +- Execute the stages in order and keep the repository buildable and testable at + every stage gate. +- Do not create the Promptkit repository, change the module path, rename the + root package, or move framework packages out of this repository. +- Do not export internal repositories, domain values, renderers, validators, + the runner, or adapter DTOs. +- Add only the public artifact-reader extension and public error identities + required by the Step 4 roadmap. +- Preserve CLI, application configuration, HTTP, containment, validation, + timeout, credential, and redaction behavior. +- Keep tests deterministic, offline, independent of real credentials, and + focused at the narrowest stable owner. +- Prefer rewriting or moving an existing test over retaining parallel + assertions for the same behavior. +- Preserve unrelated working-tree changes. +- Do not begin Migration Step 5 in this change. + +## Stage 1: Complete The Public Framework Boundary + +Add the public artifact-reader extension and error identities before changing +any adapter consumer. + +### Public Artifact Reader + +In `types.go`, add: + +```go +type ArtifactReader interface { + Read(context.Context, ArtifactRef) (*Artifact, error) +} +``` + +In `engine.go`, add `WithArtifactReader(ArtifactReader) Option`. Extend +`engineOptions` with an internal artifact-reader slot and source-presence flag, +parallel to the existing LLM and source options. A supplied reader replaces +`artifact.NewCompositeReader()` when the runner is constructed; when the option +is omitted, current default inline and unrestricted file behavior remains +unchanged. + +Reject a nil public reader as `ErrInvalidConfig`. Keep option construction +errors wrapped by `NewEngine` consistently with the existing option behavior. + +Create `artifact_reader.go` in the root facade with a private adapter that +implements `internal/artifact.Reader` by: + +1. converting the internal reference to a public `ArtifactRef`; +2. calling the supplied public reader with the original context; +3. returning the reader error unchanged so the runner and facade can add their + normal artifact-load categories while preserving `errors.Is`; +4. rejecting `(nil, nil)` with a descriptive non-sentinel error so the runner + maps it to `ErrArtifactLoad`; and +5. converting a non-nil public artifact to an internal artifact while copying + `Body`. + +Do not teach the public adapter to infer missing names, hashes, sizes, or +content types. Those values are the reader's responsibility, except that the + existing runner continues assigning the input-map name when `Artifact.Name` + is empty. + +### Public Error Detail + +Add public `ErrProfileRequired` and `ErrAPIKeyEnvMissing` sentinels alongside +the existing facade errors. + +Update `mapPublicError` so: + +- a missing explicit/default profile matches both `ErrInvalidRequest` and + `ErrProfileRequired`; +- an unset selected credential environment variable matches both + `ErrInvalidRequest` and `ErrAPIKeyEnvMissing`; +- the original internal error remains wrapped for context during the + single-repository phase; and +- all existing broad public mappings retain their current behavior. + +Use error wrapping, not message inspection. Check the specific internal causes +before the general internal `ErrInvalidRequest`. Construct the returned chain +so `errors.Is` can independently match the broad public sentinel, the specific +public sentinel, and the original cause. + +### Public Tests + +Extend `engine_test.go` rather than creating a mock framework. Add a small +recording public `ArtifactReader` fake and cover: + +- `WithArtifactReader(nil)` returns `ErrInvalidConfig`; +- the reader receives the exact public type, URI, and body from a request; +- a returned artifact participates in a successful preparation through the + real engine collaborators; +- a reader error produces an error matching both the reader's custom sentinel + and `ErrArtifactLoad`; +- `(nil, nil)` produces `ErrArtifactLoad` without panicking; and +- reader cancellation or a canceled caller context remains visible through the + artifact-load error chain. + +Add `artifact_reader_internal_test.go` in package `scriptorium` with a focused +test of the private adapter's body-copy guarantee. + +Extend the existing missing-credential public test to assert +`ErrAPIKeyEnvMissing` and `ErrInvalidRequest`. Add a purpose-built prompt with +no default profile and assert that preparation without an explicit profile +matches `ErrProfileRequired` and `ErrInvalidRequest`. Do not assert complete +error strings. + +Run: + +```bash +go test . +go test -count=20 -run 'Test.*(ArtifactReader|ProfileRequired|MissingCredentials)' . +``` + +### Stage 1 Gate + +- The public artifact-reader option has the exact contract defined in + `step4.md`. +- Public error chains retain the two HTTP-required distinctions and their broad + invalid-request category. +- Default engine artifact behavior is unchanged when the option is absent. +- No adapter has been refactored yet. + +## Stage 2: Establish The Scriptorium-Owned HTTP Artifact Reader + +Create `internal/adapter/http/artifact_reader.go`. It must implement the new +public `scriptorium.ArtifactReader` interface directly and must not import +`internal/domain`, the Promptkit-destined `internal/artifact`, or framework +defaults. + +Expose an internal-repository constructor for the CLI adapter: + +```go +func NewRestrictedArtifactReader(root string, maxBytes int64) (scriptorium.ArtifactReader, error) +``` + +Keep these sentinels in the HTTP adapter package: + +- `ErrFileNotAllowed`; +- `ErrFileOutsideRoot`; and +- `ErrFileTooLarge`. + +The implementation should move the HTTP-owned mechanisms from +`internal/artifact` into public types: + +- route public inline and file reference types; +- reject unsupported reference types, missing inline bodies, and missing file + paths with descriptive local errors; +- hash inline and file bodies with SHA-256; +- use `text/plain` as the inline and unknown-extension fallback content type; +- infer file content type with `mime.TypeByExtension`; +- populate file name, URI, size, hash, body, and content type; +- deny file references when the trimmed root is empty; +- reject a negative byte limit during construction; +- treat zero as an unlimited byte limit; +- clean and absolutize a configured root; +- accept cleaned relative and absolute paths lexically contained by that root; +- reject paths lexically outside the root; +- retain the documented behavior of following symlinks after lexical checking; +- check file size before reading and enforce the limit again with + `io.LimitReader(maxBytes+1)`; and +- honor caller cancellation before resolving either inline or file input. + +Move the restricted-reader unit cases from +`internal/artifact/reader_test.go` into +`internal/adapter/http/artifact_reader_test.go`, rewritten with public +`ArtifactRef` and `Artifact` values. Preserve coverage for contained relative +and absolute paths, traversal, outside absolute paths, symlinks, no-root +denial, exact and excessive byte limits, zero limits, metadata, and negative +limits. Add focused cancellation and unsupported/malformed-reference cases. + +Keep the old restricted-reader implementation in `internal/artifact` only +temporarily because CLI serving and some handler tests still use its internal +interface. Remove its now-moved focused unit cases to avoid maintaining two +authoritative test sets. Mark no compatibility promise for the old internal +constructor; it must be deleted in Stage 4 when the HTTP handler and CLI +`serve` path move together. + +Run: + +```bash +go test ./internal/artifact ./internal/adapter/http +``` + +### Stage 2 Gate + +- The new HTTP reader independently satisfies the public reader contract and + preserves every documented HTTP containment outcome. +- Its focused tests use no framework-internal type. +- The old restricted implementation remains only as a temporary build bridge + with a mandatory deletion point in Stage 4. + +## Stage 3: Rewrite Formatting And Move CLI Run And Render + +Update `internal/format/prepared_run.go` so every formatter interface and +function accepts `*scriptorium.PreparedRun` rather than +`*domain.PreparedRun`. Replace internal rendered-message and target references +with their corresponding public types. Do not add a translation DTO: the +formatter is a Scriptorium consumer of the public value. + +Update `internal/format/prepared_run_test.go` to construct public values. Keep +the existing behavioral assertions for: + +- deterministic text formatting; +- effective settings and sorted extra parameters; +- session IDs and cache-control fields; +- message ordering and content; +- valid JSON output; +- omission of absent optional values; and +- absence of resolved or direct secret values. + +Do not change text or JSON output solely because the source Go type changed. +Leave the Scriptorium-owned format enum and parser used by the configuration +package unchanged. + +Refactor the `run` and `render` paths in `internal/adapter/cli/run.go` to import +and consume the root public package. Keep the existing internal construction +used by `serve` temporarily because the HTTP handler still accepts internal +types until Stage 4. Do not add a conversion shim between a public engine and +the old handler. + +### Construction + +Add a public engine-construction helper that: + +- maps prompt, profile, and schema directories into public `Config`; +- accepts public engine options so Stage 4 can supply `WithArtifactReader`; +- leaves framework timeout and provider defaults unset unless Scriptorium has + an actual application-owned override; and +- returns the `NewEngine` error to the command. + +`run` and `render` use this helper with the engine's default artifact reader and +default OpenAI-compatible client. Stop constructing an internal LLM client or +runner on those paths. + +Report engine-construction failures as ordinary command runtime failures +without leaking secrets. + +Leave `serve`, its internal LLM construction, its legacy restricted reader, and +the internal runner helper buildable but otherwise unchanged in this stage. +They are a temporary boundary exception with an explicit removal in Stage 4. + +### Request And Result Mapping + +Change `buildRunRequestFromConfig` to return public `RunRequest` values: + +- use public `File` references for CLI input paths; +- copy variables into the public request; +- map prompt and profile IDs unchanged; +- create a public `ExecutionTargetOverride` only when at least one override flag + was explicitly supplied; and +- preserve pointer presence and explicit zero for temperature, max tokens, + top-p, and timeout seconds. + +Change `determineExitCode`, `printSummary`, output handling, and render +formatting to consume public result and prepared values. Preserve current CLI +output, validation exit classification, cache-usage summary behavior, and file +permissions. + +### CLI Tests + +Rewrite CLI tests and helpers to use public request and result values. Keep +application configuration and Scriptorium default imports where they remain +owned locally. Preserve the existing parser, precedence, output, built-in +profile, selected-profile, explicit-zero, provider-wire, and end-to-end command +tests. + +Add or consolidate a focused mapping assertion showing that omitted numeric +flags remain nil while explicitly supplied zero values remain non-nil public +overrides. Do not duplicate framework precedence tests already owned by the +root suite. + +Run: + +```bash +go test ./internal/format ./internal/config +go test ./internal/adapter/cli +go test -count=20 ./internal/adapter/cli +bash ./examples/render-markdown-summary.sh +``` + +### Stage 3 Gate + +- `internal/format` has no `internal/domain` import and formatter output remains + unchanged. +- Application configuration still parses and applies render-format settings. +- CLI `run` and `render` construct and call only the public engine for framework + behavior. +- CLI behavior and maintained rendering remain unchanged. +- The remaining internal framework imports are confined to `serve` construction + and its legacy helpers, with deletion required in the next stage. + +## Stage 4: Move The HTTP Handler And CLI Serve Path Together + +Rewrite `internal/adapter/http/handler.go` against the root public facade. + +### Handler Boundary And DTO Conversion + +Keep the consumer interface in the HTTP package and narrow it to: + +```go +type Runner interface { + Run(context.Context, scriptorium.RunRequest) (*scriptorium.RunResult, error) +} +``` + +The public `*Engine` must satisfy it directly. Do not add this interface to the +root facade. + +Map request DTOs directly to public values: + +- convert each input to public `ArtifactRef`; +- convert model overrides to public `ExecutionTargetOverride`; +- retain pointer-valued numeric fields without dereferencing them; +- map variables, prompt version, and profile selection; and +- continue omitting direct API keys and framework-only request fields from the + HTTP DTO. + +Map public artifacts, validation, execution targets, usage, hashes, timestamps, +and duration into the existing response DTO without changing serialized names +or inclusion rules. + +### Error Mapping + +Replace every internal prompt, profile, artifact, domain, and use-case error +check with public framework or Scriptorium HTTP-reader identities. Preserve +the existing mapping order: + +1. public `ErrPromptNotFound` and `ErrProfileNotFound`; +2. public `ErrProfileRequired` and `ErrAPIKeyEnvMissing`; +3. public prompt/profile load errors and general `ErrInvalidRequest`; +4. local `ErrFileNotAllowed` or `ErrFileOutsideRoot`, then local + `ErrFileTooLarge`; +5. public `ErrArtifactLoad`; +6. public render, generation, and validation errors; and +7. the sanitized internal fallback. + +Keep current status codes, public error codes, and sanitized messages exactly as +defined by `docs/api.md`. Check identity with `errors.Is`, never error text. + +### HTTP Tests + +Rewrite the handler fake to use public request and result values. Convert +request/response mapping tests and the error table to public sentinels and +local HTTP artifact sentinels. + +For tests that need assembled behavior, construct a real public engine from +small temporary prompt/profile fixtures and inject only a public LLM fake. Add +the new restricted artifact reader in the containment cases. In particular: + +- retain one public-engine test proving reserved provider parameters map to + `invalid_request`; +- retain end-to-end handler coverage for inline input, file denial without a + root, contained file access, outside-root denial, and artifact-size + enforcement; and +- keep the maintained `examples/http-run.json` request-contract check. + +Do not recreate internal prompt repositories, profile repositories, renderers, +or runners in handler tests. Remove redundant assembled assertions already +owned by the public engine or focused artifact-reader tests. + +### Complete The CLI Boundary + +After the handler accepts the public run interface, update `serveCommand` to: + +1. construct `httpadapter.NewRestrictedArtifactReader` from the resolved + artifact root and byte limit; +2. pass it to the public engine helper through + `scriptorium.WithArtifactReader`; +3. handle reader and engine construction failures as runtime errors; and +4. pass the public engine directly to `NewHandlerWithOptions`. + +Then remove `newRunner`, `newRunnerWithArtifactReader`, `newOpenAIClient`, and +all remaining Promptkit-destined internal imports from CLI production. The CLI +may continue importing Scriptorium-owned configuration, defaults, formatter, +and HTTP adapter packages. + +After all CLI and handler production and test consumers use public types: + +- delete `NewRestrictedCompositeReader`, + `NewRestrictedCompositeReaderWithLimit`, the denied and restricted file + reader implementations, and `ErrFileNotAllowed`, `ErrFileOutsideRoot`, and + `ErrFileTooLarge` from `internal/artifact`; +- simplify the general framework file reader to retain unrestricted reading + without the HTTP byte-limit path; and +- remove any obsolete restricted-reader test code left in + `internal/artifact/reader_test.go`. + +Run: + +```bash +go test ./internal/artifact ./internal/adapter/http +go test ./internal/adapter/cli +go test -count=20 ./internal/adapter/http +go test ./internal/adapter/http -run TestMaintainedHTTPRunExampleMatchesRequestContract +``` + +### Stage 4 Gate + +- HTTP production and tests use public framework values and errors only. +- CLI `serve` injects the Scriptorium-owned public reader into the public engine + and passes that engine directly to the handler. +- CLI production has no Promptkit-destined internal import. +- All HTTP artifact policy lives with the HTTP adapter. +- The legacy restricted reader and its Scriptorium-specific errors no longer + exist in the Promptkit-destined general artifact package. +- The HTTP contract and maintained request example remain unchanged. + +## Stage 5: Enforce The Boundary, Reconcile Documentation, And Validate + +### Dependency Guard + +Add `internal/adapter/dependency_test.go` with package `adapter_test`. Define +`TestScriptoriumAdaptersUseOnlyPublicFrameworkBoundary` to inspect direct +imports in non-test Go files for: + +- `internal/adapter/cli`; +- `internal/adapter/http`; and +- `internal/format`. + +Use the standard library parser with `parser.ImportsOnly`, locating the source +directories relative to the test file. Do not invoke network tools or inspect +transitive dependencies: importing the current public root necessarily reaches +the internal framework until extraction. + +Reject direct imports of: + +- `internal/domain`; +- `internal/usecase`; +- `internal/promptdef`; +- `internal/prompt`; +- `internal/profile` and `internal/profile/builtin`; +- `internal/validate`; +- `internal/llm`; and +- `internal/artifact`. + +The failure must identify both the importing file and forbidden path. This test +is the durable Step 4 boundary owner; do not add duplicate per-package import +tests. + +Run a final search over both production and test files. Scriptorium adapter and +formatter tests should also use public framework types, although the durable +architecture guard needs to enforce production imports only. + +### Documentation Reconciliation + +Update current-behavior documentation only after the boundary is implemented: + +- `docs/consumers/pkg-scriptorium.md` + - add `ArtifactReader` and `WithArtifactReader`; + - define replacement of the default reader, nil handling, error wrapping, and + custom-error preservation; + - add `ErrProfileRequired` and `ErrAPIKeyEnvMissing` and state that both also + match `ErrInvalidRequest`. +- `docs/policy/architecture.md` + - state that executable adapters consume framework behavior through the + public facade; + - retain the current single-repository system shape and avoid presenting the + future repository split as complete. +- `docs/internal/adapters.md` + - replace direct runner/domain composition with public engine construction, + public request/result mapping, and adapter-local consumer interfaces; + - describe `serve` reader injection and public error mapping. +- `docs/internal/sources.md` + - distinguish the framework's ordinary reader from the public extension and + Scriptorium's HTTP-owned restricted reader; + - remove the stale deleted integration-test reference. +- `docs/internal/overview.md` + - update the adapter, formatter, and split artifact responsibilities. +- `docs/internal/runner.md` + - clarify that executable adapters reach the runner through the public engine; + - remove the stale deleted integration-test reference. + +Review `README.md`, `docs/cli.md`, `docs/config.md`, `docs/api.md`, +`docs/operations.md`, and the integration documents. Do not edit them if their +observable contract remains accurate; canonical owners should not receive +refactor-only churn. + +### Full Validation + +Run: + +```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/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: + +- run the dependency guard explicitly; +- verify both maintained configuration files passed through the real loader; +- validate every local Markdown link in changed documentation; +- confirm no real credentials or private data were added; +- inspect `go.mod` and `go.sum` and confirm no dependency change was needed; +- confirm no `go.work`, `go.work.sum`, or local `replace` directive was added; + and +- inspect the final diff for unrelated or out-of-scope changes. + +### Completion Bookkeeping + +After every check passes: + +1. update `step4.md` to mark the target state complete and summarize the + achieved boundary without converting it into an implementation log; +2. update the Step 4 gate in `migration.md` with the completion date and a + concise validation summary; +3. mark this implementation plan complete; and +4. identify Step 5 repository creation as the next migration gate. + +Do not begin repository creation or extraction in the same change. + +### Stage 5 Gate + +- Every completion criterion in `step4.md` is satisfied. +- The dependency guard proves the intended direct-import boundary. +- Current documentation describes the implemented single-repository state. +- Full tests, vet, build, race checks, maintained examples, links, and + whitespace validation pass. +- The main roadmap records Step 4 as complete and Step 5 as next. + +## Open Questions + +None. diff --git a/docs/roadmap/step4.md b/docs/roadmap/step4.md new file mode 100644 index 0000000..1204ab2 --- /dev/null +++ b/docs/roadmap/step4.md @@ -0,0 +1,396 @@ +# Migration Step 4: Public-Facade Adapter Boundary + +## Status + +Proposed. Migration Steps 1 through 3 are complete; this roadmap defines the +required target state for Step 4. The separate +[implementation plan](implementation.md) defines the ordered work. + +## Purpose + +Make Scriptorium's CLI and HTTP adapters genuine consumers of the public +framework facade before that facade moves to Promptkit. This establishes and +tests the dependency boundary inside the current repository, where it can be +changed atomically, before the framework and application are separated across +repositories. + +The [accepted split decision](../adr/0002-split-promptkit-from-scriptorium.md) +owns the long-term project boundary. The +[main migration roadmap](migration.md) owns the overall sequence. This feature +roadmap defines the desired Step 4 state, not an implementation sequence. + +## Current Gap + +The public `Engine` facade already supports the ordinary `Prepare` and `Run` +workflows, directory and alternate framework sources, injected model clients, +public result values, and broad public error classification. The executable +adapters do not yet use that boundary consistently: + +- the CLI constructs framework repositories, readers, renderer, validator, and + OpenAI-compatible client directly, then calls the internal runner; +- the HTTP handler accepts internal domain request and result values; +- HTTP error mapping inspects framework-internal sentinels; +- the HTTP artifact-containment reader implements the internal artifact-reader + interface; +- prepared-run formatting accepts an internal domain value; and +- adapter tests frequently construct internal runners or use internal domain + values. + +Those dependencies would prevent Scriptorium from compiling after the +framework packages move to Promptkit. They also allow the executable to exercise +a different composition path from downstream Go consumers. + +## Target State + +At completion, Scriptorium's executable path is an ordinary consumer of the +same public framework boundary used by other Go applications: + +```text +cmd/scriptorium + | + v +Scriptorium CLI and HTTP adapters + | + v +public Engine, requests, results, errors, and extension interfaces + | + v +framework implementation packages +``` + +The CLI and HTTP production packages do not import framework-owned internal +packages. They translate application configuration, flags, and HTTP DTOs into +public engine configuration and request values; call `Prepare` or `Run`; and +translate public results and errors back into their owned interfaces. + +The root facade may continue to use the existing internal implementation during +Step 4. That implementation moves in later migration steps. The important +result here is that no Scriptorium-owned adapter or presentation component +depends on it directly. + +## Public Facade Requirements + +### Engine Consumption + +The current public `Engine`, `Config`, `RunRequest`, `PreparedRun`, +`RunResult`, request helpers, result values, and existing broad error sentinels +remain the primary boundary. Step 4 must not introduce a second facade, +adapter-specific engine, exported internal runner, or public repository +constructor. + +Scriptorium-local interfaces may abstract the methods an adapter needs for test +substitution. They must: + +- be declared on the consuming side; +- use only public facade types; +- contain only `Prepare`, `Run`, or the narrower subset required by that + consumer; and +- be satisfied directly by `*Engine`. + +Promptkit must not acquire CLI, HTTP, status-code, or response-format concepts +to satisfy these interfaces. + +### Artifact Reader Extension + +The public facade will expose the demonstrated artifact-loading extension point: + +```go +type ArtifactReader interface { + Read(context.Context, ArtifactRef) (*Artifact, error) +} + +func WithArtifactReader(ArtifactReader) Option +``` + +This option replaces the engine's ordinary composite artifact reader for all +input references. The default remains the framework's ordinary inline and +caller-selected file behavior. + +The boundary must have the same defensive behavior as other public injection +points: + +- a nil reader is rejected as `ErrInvalidConfig`; +- a reader response of `(nil, nil)` becomes `ErrArtifactLoad` rather than + causing a panic; +- reader failures are wrapped as `ErrArtifactLoad` while retaining the + original error identity for `errors.Is`; +- conversion between public and internal values does not expose internal domain + types; and +- mutable artifact content is copied across the boundary where needed to avoid + unintended aliasing. + +This is the only new framework extension interface required by Step 4. No +public prompt repository, profile repository, renderer, validator, or internal +runner interface is needed for the current adapters. + +### Public Error Detail + +The facade's existing broad errors remain authoritative for general consumers. +To preserve Scriptorium's current HTTP contract without inspecting Promptkit +internals, the public error chain must additionally retain stable identities +for: + +- profile selection being required because neither the request nor the prompt + supplies a profile; and +- a selected credential environment variable being unset or empty. + +These identities will be exposed as `ErrProfileRequired` and +`ErrAPIKeyEnvMissing`. Each remains nested under `ErrInvalidRequest`, so callers +that need only the broad category continue to match it. Scriptorium uses the +more specific identities only to preserve its existing HTTP error codes. + +No error-string parsing is permitted. Prompt, profile, artifact, render, +generation, and validation failures continue to use the existing public +sentinels. Scriptorium-owned HTTP artifact-policy errors remain in Scriptorium +and survive wrapping through the public artifact-reader boundary. + +## CLI Boundary + +The `run` and `render` commands will construct the public engine from resolved +application settings: + +- `prompt_dir`, `profile_dir`, and `schema_dir` map to public engine + configuration; +- the public engine supplies the built-in profile registry, renderer, + validator, ordinary artifact reader, and default OpenAI-compatible client; +- CLI input paths map to public file artifact references; +- CLI runtime flags map to the public request override while preserving + numeric presence, including explicit zero; and +- omitted application values remain omitted so framework defaults are not + duplicated in Scriptorium. + +`run` consumes the public result for artifact output, validation exit status, +and the stderr summary. `render` consumes the public prepared value. The +Scriptorium-owned prepared-run formatter will accept public prepared values +instead of internal domain values without changing its text or JSON contract. + +The `serve` command will construct the same public engine with Scriptorium's +restricted artifact reader injected through `WithArtifactReader`. It will pass +that engine to the HTTP handler through a Scriptorium-local run interface. + +CLI construction must handle public engine-construction errors explicitly. +The executable must not construct an internal LLM client, repository, renderer, +validator, artifact reader, or runner. + +## HTTP Boundary + +The HTTP handler's consumer interface will accept public `RunRequest` values +and return public `RunResult` values. DTO mapping will preserve the existing +HTTP contract: + +- strict JSON decoding and request-size enforcement remain in Scriptorium; +- prompt, profile, input, variable, and execution-override fields map to their + corresponding public values; +- pointer-valued numeric overrides retain omitted-versus-explicit-zero + semantics; +- raw API keys remain absent from the HTTP request shape; +- public run results map to the current response DTOs; +- raw model output remains opt-in; and +- response-size enforcement remains an HTTP concern. + +HTTP error mapping will inspect only: + +- public framework errors; +- Scriptorium's HTTP artifact-policy errors; and +- standard-library transport errors owned by the handler. + +It will not inspect internal prompt, profile, artifact, domain, or use-case +errors. Existing HTTP statuses, error codes, and sanitized messages remain +unchanged. + +## HTTP Artifact Policy + +The restricted artifact reader remains Scriptorium-owned and becomes an +implementation of the public `ArtifactReader` interface. It belongs with the +HTTP adapter rather than the Promptkit-destined general artifact package. + +The reader will continue to: + +- resolve inline references without permitting empty inline bodies; +- deny file references when no artifact root is configured; +- enforce the configured maximum artifact size; +- enforce the documented lexical root-containment rule without resolving + symlinks; +- return complete public artifact metadata; and +- preserve distinct Scriptorium errors for a denied or out-of-root file and an + oversized file. + +The public engine treats those errors as artifact-load failures while preserving +their identities. The HTTP mapper checks the Scriptorium-specific identity +before the broad public `ErrArtifactLoad` identity, retaining the current +`artifact_not_allowed`, `artifact_too_large`, and general artifact-read +outcomes. + +General inline and unrestricted file reading remains framework-owned. Step 4 +separates the HTTP policy from that implementation far enough that each side +can later move to its target repository without redesigning the interface. + +## Package And Dependency Boundaries + +The completed dependency state is: + +| Component | Permitted framework dependency | +| --- | --- | +| `internal/adapter/cli` | Public facade types, constructors, options, errors, and methods only | +| `internal/adapter/http` | Public facade types and errors only; Scriptorium-owned HTTP artifact policy remains local | +| `internal/format` | Public prepared-run and rendered-message values only | +| `internal/config` | Scriptorium application settings and Scriptorium-owned defaults; no framework orchestration | +| `cmd/scriptorium` | CLI adapter only | +| Root facade | Existing internal framework implementation until extraction | + +In particular, Scriptorium-owned adapter, formatter, and HTTP artifact-policy +production files must not import: + +- `internal/domain`; +- `internal/usecase`; +- `internal/promptdef` or `internal/prompt`; +- `internal/profile` or `internal/profile/builtin`; +- `internal/validate`; +- `internal/llm`; or +- the Promptkit-destined general artifact implementation. + +Tests for Scriptorium-owned components should follow the same public boundary +except when directly testing a Scriptorium-owned package. + +## Observable Behavior + +Step 4 is an architectural refactor plus the minimum additive public extension +surface required to support it. It is not a redesign of the executable +interfaces. + +The following behavior must remain unchanged: + +- CLI commands, flags, aliases, precedence, output routing, summaries, and exit + codes; +- application configuration discovery, strict decoding, fields, and defaults; +- HTTP routes, strict decoding, DTOs, statuses, codes, messages, and limits; +- HTTP artifact containment and size enforcement; +- prompt/profile selection and override precedence; +- explicit numeric-zero behavior; +- built-in profile fallback and custom-profile overlays; +- structured-output and validation behavior; +- timeout layering; +- secret handling and redaction; and +- maintained executable examples. + +The intended public additions are limited to artifact-reader injection and the +specific error identities required by the HTTP mapper. No compatibility shim is +needed because this repository still owns the facade during Step 4 and the +overall migration is intentionally breaking. + +## Test Ownership And Verification + +Tests will protect the boundary at the layer that owns each risk: + +- public engine tests own artifact-reader option validation, conversion, + invocation, nil-response handling, error wrapping, and error identity; +- CLI tests own flag and configuration mapping into public requests, public + engine wiring, presentation, output, and exit behavior; +- HTTP tests own strict DTO mapping to public requests, public result mapping, + error/status mapping, limits, and raw-output opt-in; +- HTTP artifact-reader tests own denied, contained, escaped, oversized, inline, + cancellation, and metadata behavior; +- formatter tests own stable text and JSON presentation of public prepared + values; and +- existing framework tests continue to own orchestration, source, validation, + provider, and broad public error behavior. + +Adapter tests that currently construct internal runners or assert internal +sentinels will be rewritten against the public engine or small public-typed +fakes. Duplicate framework-semantic assertions should be removed when the +public contract suite already owns the risk. + +The final suite must include an enforceable dependency check showing that +Scriptorium-owned adapter and presentation production packages do not import +Promptkit-destined internal packages. This may be a focused architecture test +or an equivalent deterministic repository check; it must diagnose the +forbidden import clearly. + +## Documentation Outcome + +When the boundary is implemented, current-behavior documentation will be +reconciled in the same change: + +- the public Go package contract will define `ArtifactReader`, + `WithArtifactReader`, and the new error identities; +- adapter internals will describe public-engine composition and public-value + mapping; +- source internals will distinguish the public reader extension, general + framework readers, and Scriptorium's HTTP reader; +- the internal overview and architecture policy will reflect that executable + adapters consume the public facade; and +- CLI, configuration, HTTP, integration, and operations contracts will change + only if verification finds an observable correction is necessary. + +Documents will keep exact external contracts in their existing canonical +owners and link rather than duplicate them. + +## Required Validation Outcome + +The completed boundary must pass: + +- the full Go test suite; +- `go vet` across all packages; +- a temporary-output executable build; +- repeated public artifact-reader and adapter boundary tests; +- both maintained application configurations; +- maintained render, HTTP-request, and Go-package examples; +- CLI and HTTP smoke checks that exercise the public engine path; +- the forbidden-import dependency check; +- local Markdown-link validation; and +- whitespace validation. + +All default validation remains offline, deterministic, and independent of real +credentials. + +## Out Of Scope + +Step 4 does not: + +- create the Promptkit repository or module; +- change the module or root package name; +- move framework implementation packages or built-in profiles out of this + repository; +- remove the current root facade; +- add compatibility aliases or forwarding packages; +- make Scriptorium depend on an external Promptkit revision; +- broadly export framework repositories, domain values, validators, renderers, + or runner constructors; +- redesign prompt, profile, schema, request, response, CLI, or configuration + formats; +- change HTTP containment from lexical path checking to symlink resolution; +- add new execution or repair behavior; or +- perform unrelated facade cleanup. + +Those changes belong to later migration steps or a separately accepted feature. + +## Completion Criteria + +Step 4 is complete when: + +- CLI `run` and `render` execute through the public engine; +- CLI `serve` injects the Scriptorium-owned restricted reader into the public + engine and passes that engine to the HTTP handler; +- HTTP and CLI map only public framework request, result, and error values; +- prepared-run formatting consumes the public prepared value; +- the public artifact-reader extension has the specified validation, + conversion, nil-response, and error-preservation behavior; +- public error identities preserve every distinction required by the current + HTTP contract; +- Scriptorium-owned adapters, formatter, and HTTP artifact reader have no + Promptkit-destined internal imports; +- HTTP containment, limits, error mapping, and all existing executable + behavior remain protected by passing tests; +- current-behavior documentation reflects the implemented boundary; +- every required validation check passes; +- no out-of-scope extraction or compatibility work is included; and +- the main migration roadmap records Step 4 as complete and identifies + repository creation in Step 5 as the next gate. + +Migration Step 5 must not begin until these criteria are satisfied. + +## Lifecycle + +This feature roadmap is a temporary migration artifact. It may be removed after +Step 4 is complete and no longer needs to guide active work; repository history +will retain the decision and completion record.