Plan the audit remediation sprint
This commit is contained in:
@@ -4,8 +4,9 @@
|
||||
|
||||
Prepare PromptKit's audited implementation for a focused round of correctness,
|
||||
configuration simplification, resource-use improvements, and internal cleanup.
|
||||
This document defines the target and work boundaries for a subsequent staged
|
||||
implementation plan; it is not that plan or a description of completed changes.
|
||||
This document defines the target and work boundaries. The companion
|
||||
[implementation plan](implementation.md) owns staged steps and validation;
|
||||
neither document describes completed remediation yet.
|
||||
|
||||
Baseline: repository commit `bd3dfa9`, with the
|
||||
[audit catalog](audit/catalog.md) covering 41 source findings consolidated into
|
||||
@@ -14,9 +15,10 @@ implementation remains pending. The planning assessment was completed at
|
||||
`3ef2a9e` against the unchanged production baseline, with evidence gathering
|
||||
starting at `21fea46`. Its recommendations, measurements, and subsequent review
|
||||
refinements are consolidated here; the separate assessment document is retired.
|
||||
The catalog and supporting audits own finding evidence and decision rationale. This roadmap owns remediation organization,
|
||||
dependencies, completion evidence, and subsequent workstream status. Earlier
|
||||
remediation suggestions in the source audits do not override accepted decisions.
|
||||
The catalog and supporting audits own finding evidence and decision rationale.
|
||||
This roadmap owns the selected end state, work boundaries, assessment evidence,
|
||||
and workstream status. Earlier remediation suggestions in the source audits do
|
||||
not override accepted decisions.
|
||||
|
||||
All workstreams below are pending implementation; their planning assessment is
|
||||
complete. No finding is proposed for deferral. Concrete designs below are the
|
||||
@@ -122,10 +124,11 @@ repeated across direct/prepared and host-file/`fs.FS` paths.
|
||||
**Selected source handoff:** Assign `config.ExtraParams` directly into the
|
||||
ordinary fields map in `ProfileSource` and `BackendSource`, then call
|
||||
`configkit.Mapping` as now. Optional typed fields and the `[]any` definition
|
||||
wrapper already use supported forms. Delete `provider_values.go` after replacing
|
||||
its two callers. ConfigKit's immediate snapshot and deferred unsupported/cyclic
|
||||
diagnostics make an extra shallow copy unnecessary. Do not add a replacement
|
||||
converter, precision checker, memoization, or raw-source validation pass.
|
||||
wrapper already use supported forms. The recursive compatibility adapter in
|
||||
`provider_values.go` is retired. ConfigKit's immediate snapshot and deferred
|
||||
unsupported/cyclic diagnostics make an extra shallow copy unnecessary. Do not
|
||||
add a replacement converter, precision checker, memoization, or raw-source
|
||||
validation pass.
|
||||
|
||||
**Selected schema design:** Retain source-specific root loading, then share
|
||||
`compileSchemaDocument(ctx, resourceURL, document)` for compilation. Register
|
||||
@@ -162,9 +165,9 @@ nil-plan error, cancellation, and frozen-plan behavior. No public facade
|
||||
signature change is required.
|
||||
|
||||
Cover `WithSchemaFile` through public construction and preparation/validation,
|
||||
including source replacement/precedence and relevant file errors. Coordinate
|
||||
removal of its unused private helper result with F. Replace external-resource
|
||||
success fixtures with early-error cases, retaining independent root-source tests.
|
||||
including source replacement/precedence and relevant file errors. Its unused
|
||||
private helper result is retired as described in F. External-resource success
|
||||
fixtures become early-error cases, retaining independent root-source tests.
|
||||
|
||||
**Completion evidence:** Focused source-helper cases demonstrate supported-value
|
||||
handoff, immediate source capture, and ConfigKit's deferred unsupported/cyclic
|
||||
@@ -193,14 +196,14 @@ errors can obscure their owning operation; response read failures can be
|
||||
misclassified; some pre-dispatch smoke failures produce a completed report;
|
||||
and successful authentication responses are read without a byte bound.
|
||||
|
||||
**Work:**
|
||||
**Target properties:**
|
||||
|
||||
1. Align artifact GoDoc, adapters, and tests with successful `Read` transferring
|
||||
- Artifact GoDoc, adapters, and tests align with successful `Read` transferring
|
||||
the artifact and mutable body storage to PromptKit. Readers stop mutating
|
||||
that storage, including retained aliases. Keep the interface signature and
|
||||
existing defensive copy initially. Readers may reuse private buffers when
|
||||
they return an independent copy; no release protocol is introduced.
|
||||
2. Preserve the owning public error category independently of nested causes,
|
||||
- Preserve the owning public error category independently of nested causes,
|
||||
retaining appropriate `errors.Is`/`errors.As` access. Do not require category
|
||||
exclusivity: retained causes can make more than one category match. Preserve
|
||||
meaningful direct-not-found versus load-failure distinctions. Use a small
|
||||
@@ -209,7 +212,7 @@ and successful authentication responses are read without a byte bound.
|
||||
including before nested capacity/provider/exhaustion projections can obscure
|
||||
it. Preserve those specialized public projections and matchable child causes.
|
||||
Reordering a whole-chain sentinel switch cannot express operation ownership.
|
||||
3. Distinguish successful-response body I/O failures from malformed JSON at both
|
||||
- Distinguish successful-response body I/O failures from malformed JSON at both
|
||||
initial decoding and end verification. Preserve the redacted transport
|
||||
boundary, cancellation precedence, and genuine malformed, truncated, or
|
||||
oversized-content behavior without adding retries. Record actual non-EOF
|
||||
@@ -218,7 +221,7 @@ and successful authentication responses are read without a byte bound.
|
||||
and otherwise propagate recorded I/O failures through `requestFailedError`.
|
||||
Distinguish reader-returned errors from parser-generated syntax/type/truncation
|
||||
errors, rather than treating every `io.ErrUnexpectedEOF` alike.
|
||||
4. Return local inference-diagnostic construction failures as `ErrInvalidRequest`
|
||||
- Return local inference-diagnostic construction failures as `ErrInvalidRequest`
|
||||
using explicit preparation-stage information. Preserve completed diagnostics
|
||||
for missing credentials, capacity outcomes, and unsupported/inapplicable
|
||||
operations; absence of dispatch alone does not imply an invalid request.
|
||||
@@ -226,7 +229,7 @@ and successful authentication responses are read without a byte bound.
|
||||
built-in `PrepareGeneration` failure return. Diagnostic orchestration checks
|
||||
it after caller cancellation. Injected-client errors retain their established
|
||||
semantics; no new public stage field is needed.
|
||||
5. Define the authentication response allowance as a named internal constant of
|
||||
- Define the authentication response allowance as a named internal constant of
|
||||
1 MiB (1,048,576 bytes). Enforce against actual reads regardless of declared
|
||||
length, detect overflow, and close without unbounded draining. Report an
|
||||
explicit limit diagnostic with credential acceptance unknown and retain
|
||||
@@ -239,7 +242,7 @@ and successful authentication responses are read without a byte bound.
|
||||
Read at most allowance plus one byte using `io.LimitReader`/`io.ReadAll`;
|
||||
check overflow before JSON recognition. Preserve 401 rejection without body
|
||||
consumption and other non-200 behavior independently of unused bodies.
|
||||
6. Move OpenRouter-specific authentication request/response interpretation into
|
||||
- OpenRouter-specific authentication request/response interpretation belongs in
|
||||
the outbound adapter. Leave selection, applicability, completion, and neutral
|
||||
reporting in the use case. Preserve anonymous probing and credential/body
|
||||
privacy. Narrow smoke copying to actual consumers, removing unused request
|
||||
@@ -251,7 +254,7 @@ and successful authentication responses are read without a byte bound.
|
||||
Consume a narrow authentication capability alongside reachability HTTP,
|
||||
without reverse dependencies or a plugin registry. Directly detach the
|
||||
target/provider map; retain facade and active-backend copying.
|
||||
7. Exercise `RunRequest`'s privacy formatting and JSON omission boundary with
|
||||
- Exercise `RunRequest`'s privacy formatting and JSON omission boundary with
|
||||
synthetic markers in omitted variables, bodies, and execution settings, not
|
||||
IDs deliberately present in summaries. Cover `%v`, `%+v`, `%#v`, direct
|
||||
String/GoString, value/pointer forms, and direct-key JSON omission.
|
||||
@@ -280,10 +283,6 @@ The public-boundary outcome matrix for planning is:
|
||||
| Authentication recognition overflow | Explicit limit reason; acceptance unknown | HTTP/timing facts, not secret-bearing response data. |
|
||||
| Caller cancellation | Existing cancellation precedence | No publication of success over cancellation. |
|
||||
|
||||
**Dependencies:** The authentication bound can land before protocol relocation;
|
||||
cleanup must not delay the defect remedy. Correct error/stage behavior before
|
||||
restructuring generation accounting in E.
|
||||
|
||||
### D. Resource Use And Inspection Efficiency
|
||||
|
||||
**Catalog:** 10, 16, 17, 18, 19, 20.
|
||||
@@ -347,8 +346,8 @@ The [baseline measurements](#planning-evidence) support implementing 17, 19, and
|
||||
expected savings and algorithmic estimates are not measured replacement results.
|
||||
No exact peak-heap promise, benchmark threshold, or recurring release gate is
|
||||
introduced. If new evidence changes the tradeoff, record it and the proposed
|
||||
disposition rather than silently dropping selected work. Coordinate evidence
|
||||
accumulation with E's traversal work and overlays with E's trusted copying.
|
||||
disposition rather than silently dropping selected work. The companion plan
|
||||
coordinates the related traversal and trusted-copying changes in E.
|
||||
|
||||
### E. Internal Coordination And Test Assembly
|
||||
|
||||
@@ -398,10 +397,10 @@ capacity wrapper remains a production constructor.
|
||||
and `StartedAt`. Share usage/evidence/event projections only where facts match;
|
||||
built-in retry and injected one-shot control flow remain distinct. C's
|
||||
preparation-failure marker is independent of attempt accounting.
|
||||
- Move the initial/repair runner scheduling test to the production generation
|
||||
coordinator via `generation.NewInjected` with the same manager, then remove
|
||||
`internal/capacity/client.go` and its retention prose. Preserve useful
|
||||
FIFO, cancellation, independent-pool, and panic-release scenarios with their
|
||||
- The initial/repair runner scheduling test uses the production generation
|
||||
coordinator via `generation.NewInjected` with the same manager. The production
|
||||
`internal/capacity/client.go` wrapper and its retention prose are retired.
|
||||
Preserve useful FIFO, cancellation, independent-pool, and panic-release scenarios with their
|
||||
owning components. Keep any useful one-shot scaffolding test-only; do not wrap
|
||||
the coordinator in another permit-acquisition layer.
|
||||
|
||||
@@ -457,34 +456,15 @@ contracts. Caller checks precede deletion; meaningful discovery, identity,
|
||||
namespace, schema, privacy, and bundled behavior remain covered. Tests do not
|
||||
merely reproduce the loader's output as their expected answer.
|
||||
|
||||
## Sequencing For The Implementation Plan
|
||||
## Implementation Plan
|
||||
|
||||
Use reviewable units with tests and owning documentation included. Workstream
|
||||
letters group outcomes; they are not mandatory serial phases.
|
||||
|
||||
1. Establish A's toolchain baseline for implementation validation. Independent
|
||||
documentation work can proceed without waiting for that environment work.
|
||||
2. Prioritize B's simplifications and C's confirmed defects. They can be split
|
||||
into independent units; avoid bundling unrelated public behavior changes.
|
||||
3. Sequence B's schema policy with its compilation simplification, and remove
|
||||
source conversion before E's copying consolidation. Coordinate B's schema
|
||||
coverage with F's helper-result removal.
|
||||
4. Fix C's error/stage behavior before E's generation accounting refactor. Bound
|
||||
authentication reads before or alongside relocating the protocol adapter,
|
||||
with the behavioral remedy independently reviewable.
|
||||
5. Coordinate D's evidence algorithm with E's traversal cleanup and D's overlays
|
||||
with E's copying work. Preserve each remedy's independent evidence rather
|
||||
than assuming a general refactor also fixes its associated defect.
|
||||
6. Migrate the scheduling test before deleting the wrapper. Adjust over-specified
|
||||
hash tests before or alongside captured-definition cleanup. Fold other small
|
||||
F changes into their owning units where useful, without obscuring fixes.
|
||||
|
||||
The later plan should assign concrete implementation units, validation, and
|
||||
status to these outcomes. No significant product decisions remain open now.
|
||||
The assessment has selected report representation, extraction boundaries, and
|
||||
listing layout as described above. Exact private names and equivalent coding
|
||||
details remain implementation choices. Escalate new evidence requiring a
|
||||
contract or scope change; do not reopen resolved choices without such evidence.
|
||||
The [companion plan](implementation.md) defines the ordered, prompt-sized
|
||||
implementation units, dependencies, and validation for one end-to-end sprint.
|
||||
Workstream letters group target outcomes, not delivery stages. Temporary
|
||||
intermediate compilation gaps are acceptable; completion concerns the combined
|
||||
end state. No significant product decisions remain open. Equivalent private
|
||||
coding details remain flexible; new evidence requiring a scope or contract
|
||||
change should be surfaced rather than silently changing the selected target.
|
||||
|
||||
## Compatibility And Documentation Delivery
|
||||
|
||||
|
||||
Reference in New Issue
Block a user