diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 365fe05..dd37a58 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -2,16 +2,14 @@ ## 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. +Complete as of 2026-07-28. The public-facade adapter boundary and its +post-implementation remediation are fully revalidated. ## 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. +This plan completed the target state in the +[Step 4 public-facade adapter roadmap](step4.md) 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 @@ -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 -but found that the completion gate needs the corrections below. +The post-implementation remediation is also complete: -## 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. +- public error mapping retains collaborator identities while adding the active + operation's public category; +- HTTP artifact MIME coverage uses a built-in media type while retaining the + unknown-extension fallback check; +- the dependency guard recursively scans production adapter and formatter + sources and rejects descendants of forbidden framework package families; and +- the final validation passed full and repeated tests, vet, a temporary-output + build, race checks, maintained examples, both maintained configuration files, + import and link inspection, and whitespace validation. diff --git a/docs/roadmap/migration.md b/docs/roadmap/migration.md index d4904a3..ded5708 100644 --- a/docs/roadmap/migration.md +++ b/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 -configuration smoke checks passed. Step 5 repository creation is next. +post-implementation public-error, deterministic MIME, and recursive +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 diff --git a/docs/roadmap/step4.md b/docs/roadmap/step4.md index 5f8e81e..42e774b 100644 --- a/docs/roadmap/step4.md +++ b/docs/roadmap/step4.md @@ -2,10 +2,11 @@ ## Status -Complete as of 2026-07-28. The public-facade adapter boundary is established; -the [implementation plan](implementation.md) records the completed work, and -the [main migration roadmap](migration.md) identifies repository creation as -the next gate. +Complete as of 2026-07-28. The public-facade adapter boundary and its +post-implementation error-classification, MIME-test, and dependency-guard +corrections are validated. The [implementation plan](implementation.md) +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