From 4d7e1327addb37d41054ed2ad98f56b755c4d92d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 27 Jul 2026 20:18:02 -0500 Subject: [PATCH] Revise the implementation plan to address remaining step 4 gaps --- docs/roadmap/implementation.md | 624 +++++++++------------------------ 1 file changed, 172 insertions(+), 452 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 614a48e..365fe05 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -2,510 +2,228 @@ ## Status -Complete as of 2026-07-28. All implementation gates, the dependency guard, -documentation reconciliation, and required validation passed. Migration Step 5 -repository creation is the next gate; it was not started here. +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 -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. +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 current-behavior -updates and canonical ownership. +[documentation policy](../policy/documentation.md) governs completion +bookkeeping 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. +- 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. -- Do not begin Migration Step 5 in this change. -## Stage 1: Complete The Public Framework Boundary +## Completed Work -Add the public artifact-reader extension and error identities before changing -any adapter consumer. +The original five implementation stages established the Step 4 boundary: -### Public Artifact Reader +- 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. -In `types.go`, add: +The post-implementation review confirmed the production dependency direction +but found that the completion gate needs the corrections below. -```go -type ArtifactReader interface { - Read(context.Context, ArtifactRef) (*Artifact, error) -} -``` +## Stage 1: Preserve The Active Public Error Category -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. +Correct `mapPublicError` so a public error returned by an injected collaborator +does not suppress the public category added by the operation currently being +performed. -Reject a nil public reader as `ErrInvalidConfig`. Keep option construction -errors wrapped by `NewEngine` consistently with the existing option behavior. +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: -Create `artifact_reader.go` in the root facade with a private adapter that -implements `internal/artifact.Reader` by: +- 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. -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`. +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. -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. +### Regression Tests -### Public Error Detail +Extend the public engine contract tests at the real `Engine.Prepare` and +`Engine.Run` boundaries: -Add public `ErrProfileRequired` and `ErrAPIKeyEnvMissing` sentinels alongside -the existing facade errors. +- 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. -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. +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|ProfileRequired|MissingCredentials)' . +go test -count=20 -run 'Test.*(ArtifactReader|PublicError|LLM)' . ``` ### 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. +- 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: Establish The Scriptorium-Owned HTTP Artifact Reader +## Stage 2: Make HTTP Artifact MIME Coverage Deterministic -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. +Repair `TestRestrictedArtifactReaderReadsContainedFiles` without changing the +implemented MIME policy. The contract remains: -Expose an internal-repository constructor for the CLI adapter: +- call `mime.TypeByExtension` for a file extension; and +- use `text/plain` only when that lookup returns an empty string. -```go -func NewRestrictedArtifactReader(root string, maxBytes int64) (scriptorium.ArtifactReader, error) -``` +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. -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. +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/artifact ./internal/adapter/http +go test ./internal/adapter/http +go test -count=20 ./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. +- 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: Rewrite Formatting And Move CLI Run And Render +## Stage 3: Strengthen The Public-Boundary Dependency Guard -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/adapter/dependency_test.go` so the durable guard proves the +entire intended production boundary rather than only today's flat directory +layout. -Update `internal/format/prepared_run_test.go` to construct public values. Keep -the existing behavioral assertions for: +### Recursive Source Inspection -- 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: +Walk these roots recursively with `filepath.WalkDir`: - `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. +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`. -Reject direct imports of: +### Forbidden Package Families + +Represent each Promptkit-destined internal dependency as a package-family root: - `internal/domain`; - `internal/usecase`; - `internal/promptdef`; - `internal/prompt`; -- `internal/profile` and `internal/profile/builtin`; +- `internal/profile`; - `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. +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. -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. +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. -### Documentation Reconciliation +### Guard Regression Test -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 +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/cli ./internal/adapter/http ./internal/format +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 \ @@ -520,36 +238,38 @@ 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. +- 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. -### Completion Bookkeeping +Once every check passes: -After 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. -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 extraction or modify Promptkit as part of this stage. -Do not begin repository creation or extraction in the same change. +### Stage 4 Gate -### 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. +- 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