promptkit/docs/roadmap/implementation.md

# Optional API-Key Environment Implementation Plan

## Purpose

Change Promptkit's credential handling so an effective `APIKeyEnv` names an
optional credential source rather than implicitly requiring that environment
variable to be populated. When neither a direct key nor a populated optional
environment variable is available, the built-in OpenAI-compatible client must
omit the `Authorization` header and let the upstream provider accept or reject
the unauthenticated request.

This document owns implementation sequencing for that change. Follow the
architecture, documentation, and testing policies under [`docs/policy/`](../policy/)
and the task-specific reading guide in [`docs/development.md`](../development.md)
throughout the work.

## Target Outcome

For ordinary and prepared execution:

1. a nonblank direct `RunRequest.APIKey` remains the highest-precedence
   credential and produces `Authorization: Bearer <key>`;
2. otherwise, a nonblank effective `APIKeyEnv` is read when generation begins;
3. a nonblank trimmed environment value produces the bearer header;
4. an absent, empty, or whitespace-only optional environment value produces no
   `Authorization` header and does not prevent preparation or execution; and
5. the provider response follows the normal success or error path, including
   public `GenerationError` conversion for a non-2xx response.

An explicitly required credential retains local validation. `APIKeyRequired`
continues to require either a nonblank direct key or a populated environment
variable explicitly selected by the request. Missing required credentials
fail before provider transport.

## Fixed Decisions

- `APIKeyEnv` is a lookup location, not an assertion that authentication is
  required. This applies whether the name comes from a built-in backend, a
  consumer backend, a file profile, or a request override.
- An environment value is usable only after `strings.TrimSpace`; unset, empty,
  and whitespace-only values are equivalent and cause header omission when
  optional.
- Direct keys retain precedence. When a direct key is nonblank, Promptkit does
  not need the environment value and must not let a missing environment value
  block the request.
- `APIKeyRequired` remains the only existing explicit local requirement flag.
  Do not add a backend field, YAML field, request field, or new public API.
- With `APIKeyRequired` true:
  - a nonblank direct key satisfies the requirement;
  - an explicitly selected nonblank `APIKeyEnv` satisfies it only when that
    variable currently contains a nonblank value;
  - no selected environment name retains the existing required-key failure;
    and
  - a selected but unset environment retains `ErrAPIKeyEnvMissing` and
    `ErrInvalidRequest` at the public boundary.
- `ErrAPIKeyEnvMissing` remains exported for compatibility but is narrowed to
  a missing environment credential in an explicitly required flow. Optional
  missing environment values do not return it.
- Optional credential availability is not frozen during preparation. The
  built-in client reads the selected environment variable for each generation,
  including repair generation. Prepared execution therefore uses the value
  visible when it runs.
- Required prepared execution continues to recheck credential availability
  before admission. Optional prepared execution proceeds even if the variable
  becomes unset after preparation.
- The built-in transport sets no `Authorization` header at all when no usable
  credential exists. It must not send `Bearer ` with an empty value.
- Injected `LLMClient` behavior is not redefined. Promptkit stops rejecting an
  optional missing environment before the injected client is called and
  continues to pass the effective target through the public adapter without
  resolving or exposing a secret on the client's behalf.
- Upstream authentication policy remains upstream. Promptkit adds no provider-
  specific authentication rules, retry behavior, status mapping, or special
  handling beyond the existing structured non-2xx error path.

## Execution Rules

- Complete the stages in numerical order. Each stage is scoped for one coding-
  agent prompt and must finish its focused tests before the next stage begins.
- Treat all stages as one behavior change. Intermediate stages intentionally
  leave the use-case and transport policies temporarily different; do not tag,
  release, or claim the change is complete until Stage 3 passes.
- At the start of each stage, inspect the working tree and preserve all
  unrelated changes. In particular, merge rather than overwrite any existing
  edits in files touched by this plan.
- Use controlled transports, local servers, and `t.Setenv`; never contact a
  live provider or depend on the developer machine's credentials.
- Update existing tests whose asserted policy is changing instead of retaining
  contradictory tests or adding duplicate coverage under new names.
- Do not add dependencies, commit, tag, push, or edit release documentation
  unless separately instructed.
- Current-state prose documentation changes only in Stage 3, after the runtime
  behavior exists. GoDoc changes alongside the public contract in that stage.

## Stage 1: Make Use-Case Credential Validation Requirement-Aware

### Objective

Stop preparation and execution orchestration from treating every named
environment variable as required, while preserving the explicit
`APIKeyRequired` and prepared-execution contracts.

### Implementation

1. Update `validateAPIKey` in `internal/usecase/runner.go` with this policy:
   - return success immediately for a nonblank direct key;
   - return success when `apiKeyRequired` is false, regardless of whether
     `apiKeyEnv` is blank, populated, or missing;
   - when `apiKeyRequired` is true and the trimmed environment name is blank,
     return the existing `ErrAPIKeyRequired`;
   - when `apiKeyRequired` is true and the selected environment variable is
     unset, empty, or whitespace-only, return the existing wrapped
     `ErrAPIKeyEnvMissing` diagnostic naming the variable; and
   - otherwise return success.
2. Keep the calls from preparation and `RunPrepared` in place. Do not move
   environment lookup into backend/profile resolution, admission, the domain
   model, or the root facade.
3. Do not clear `APIKeyEnv` from effective or prepared metadata merely because
   its current value is absent. The name remains the configured lookup source;
   the secret value remains excluded.
4. Preserve credential-source precedence and `APIKeyRequired` merge behavior.
   A request environment override remains capable of satisfying an in-memory
   profile's explicit requirement when its value is populated.

### Tests

Update the narrow `internal/usecase` behavior owners rather than adding tests
for the private helper itself:

1. Replace `TestRunnerRunAPIKeyEnvMissingEnvironmentValueFailsClearly` with a
   test proving an unset optional profile `APIKeyEnv` reaches the injected
   model client and completes successfully. Explicitly set the named variable
   to an empty value so the test is independent of the process environment.
2. Preserve the existing populated-environment and direct-key-precedence tests.
3. Preserve `TestRunnerPrepareAPIKeyRequiredFailsWithoutDirectKey` and the
   existing direct-key success coverage.
4. Revise the prepared credential test matrix to prove both distinct cases:
   - an optional environment value that disappears after
     `PrepareExecution` does not prevent admission and generation; and
   - an `APIKeyRequired` profile using an explicit request `APIKeyEnv` override
     still fails with `ErrInvalidRequest` and `ErrAPIKeyEnvMissing` if that
     value disappears before `RunPrepared`, before admission or generation.
5. Keep prepared handles single-use in both outcomes and avoid duplicating
   unrelated preparation, validation, or admission assertions.

### Verification

```sh
go test ./internal/usecase -run 'APIKey|Credential|Prepared'
go test ./internal/usecase
go test ./...
```

Stage 1 is complete when optional missing environment values no longer block
the use-case layer and explicit requirements retain their prior identities and
timing.

**Status:** Complete.

## Stage 2: Omit Authentication in the Built-In Client When Optional Keys Are Missing

### Objective

Make the OpenAI-compatible transport implement the optional lookup contract at
the boundary that owns the outbound `Authorization` header.

### Implementation

1. Refactor the authentication branch in
   `internal/llm/openai_compatible_client.go` so it resolves one credential in
   this order:
   - trimmed `req.Target.APIKey`; then
   - the trimmed value of the environment variable named by the trimmed
     `req.Target.APIKeyEnv`.
2. Set `Authorization` to `Bearer <credential>` only when the resolved value is
   nonblank. If the direct key is blank and the optional environment lookup is
   absent, empty, or whitespace-only, leave the header unset and continue to
   `http.Client.Do`.
3. Retain defensive enforcement for direct internal callers of the transport:
   - if `req.Target.APIKeyRequired` is true and no usable source exists, return
     `ErrInvalidRequest` before transport;
   - use the existing missing-environment diagnostic when a nonblank
     environment name was selected; and
   - use a concise required-key diagnostic when no environment name exists.
   Do not add an internal or public error type for this branch.
4. Preserve request construction, endpoint validation, timeout behavior,
   response-body ownership, and direct-key redaction. Never serialize
   `APIKeyEnv`, `APIKeyRequired`, or a credential into the JSON request body.
5. Allow an unauthenticated upstream non-2xx response to flow through the
   existing `ProviderHTTPError` and public `GenerationError` machinery without
   adding a credential-specific translation.

### Tests

Update `TestOpenAICompatibleClientAuthentication` in
`internal/llm/openai_compatible_client_test.go` as the transport contract owner.
Its table should cover only the meaningful credential states:

- a direct key takes precedence over a populated environment value;
- a populated environment value supplies the bearer header when no direct key
  is present;
- no configured source sends the request without `Authorization`;
- a named but unset optional environment sends the request without
  `Authorization`;
- a named but unset required environment fails with `ErrInvalidRequest` before
  transport; and
- a required target with no source also fails before transport.

Use exact header assertions because presence or absence of `Authorization` is
the wire contract. Do not repeat endpoint, body, timeout, or response-error
matrices in this test.

### Verification

```sh
go test ./internal/llm -run TestOpenAICompatibleClientAuthentication
go test ./internal/llm
go test ./...
```

Stage 2 is complete when the built-in transport sends unauthenticated requests
for optional missing sources, still enforces explicit requirements, and all
existing provider-response behavior remains green.

**Status:** Complete.

## Stage 3: Align the Public Contract, Durable Documentation, and Full Validation

### Objective

Prove the assembled consumer workflow, update canonical contract owners, and
complete repository-wide validation.

### Public Contract And Tests

1. Update the GoDoc for `ErrAPIKeyEnvMissing` in `engine.go` so it applies only
   when an explicitly required credential names an unset or empty environment
   variable. Retain the exported value and its `ErrInvalidRequest` identity.
2. Update credential GoDoc where the exact public semantics are exposed:
   - `Backend.APIKeyEnv` in `backends.go`;
   - `ExecutionTarget.APIKeyEnv` and
     `ExecutionTargetOverride.APIKeyEnv` in `types.go`; and
   - any nearby `APIKeyRequired` wording that would otherwise imply every
     named environment source is mandatory.
   State that optional missing values cause the built-in client to omit
   `Authorization`; do not promise that injected clients resolve environment
   variables identically.
3. Replace `TestMissingCredentialsFailClearlyWhenProfileRequiresAuth` in
   `engine_test.go`, whose old assertion is intentionally obsolete, with one
   focused external-package contract test. Assemble a real engine with a
   controlled HTTP client or local server, select a backend or file profile
   whose `APIKeyEnv` is explicitly empty in the process environment, and have
   the controlled upstream return a structured authentication failure. Assert:
   - the request reaches upstream;
   - `Authorization` is absent;
   - the result is nil;
   - the error does not match `ErrInvalidRequest` or
     `ErrAPIKeyEnvMissing`; and
   - the error matches `ErrLLMGenerate` and is discoverable as a
     `*GenerationError` with the upstream status and representative provider
     detail.
4. Keep this root test representative. The transport authentication table owns
   the full credential matrix, and the existing generation-error tests own
   envelope parsing, bounds, and formatting.

### Documentation

Update each canonical owner only for its topic:

1. In `docs/formats.md`, define profile, backend, and request `api_key_env`
   values as optional lookup sources. In the credentials section, distinguish
   them from `APIKeyRequired`, document header omission for a missing optional
   value, retain direct-key precedence, and state that required availability is
   validated during preparation and rechecked for prepared execution.
2. In `docs/integrations/openai-compatible-chat.md`, document the outbound
   authentication wire behavior: a bearer header is sent only for a usable
   direct or environment credential; otherwise the header is omitted and the
   provider response is handled normally.
3. In `docs/internal/llm.md`, update the internal flow and failure categories to
   distinguish optional omission from explicit required-key rejection. Preserve
   any unrelated edits already present in this file.
4. In `docs/consumers/pkg-promptkit.md`, clarify near profile inspection or
   credential guidance that a reported `APIKeyEnv` is a configured optional
   source, while `APIKeyRequired` is the explicit local requirement. Link to
   the exact public GoDoc or format reference rather than reproducing the full
   precedence contract.
5. Do not change architecture policy, backend IDs/default names, release notes,
   the README, or examples unless implementation uncovers a concrete
   inaccurate current-state statement in one of those owners.

### Final Validation

Run the complete maintainer workflow from
[`docs/development.md#maintainer-validation`](../development.md#maintainer-validation):

```sh
go test ./...
go test -race ./...
go vet ./...
go build ./...
go run ./examples/go-library/prepare
go run ./examples/go-library/run
```

Also run the documented tracked-Go formatting check, local Markdown-link
validation, repository-hygiene checks, ignored-file check, credential scan, and
`git diff --check`. Review the complete diff and confirm that:

- optional absent, empty, and whitespace-only environment values omit
  authentication and reach transport;
- direct and populated environment credentials still produce the correct
  bearer header;
- explicit required flows still fail locally with the intended identities;
- ordinary, prepared, built-in, injected-client, and repair paths follow their
  stated ownership boundaries;
- upstream non-2xx responses remain structured generation errors rather than
  local credential errors;
- no credential value is serialized, retained in public metadata, or exposed
  by formatting; and
- durable documentation describes only the now-implemented behavior with one
  canonical owner per exact contract.

Stage 3 is complete when the public workflow, documentation, and full
maintainer validation all match the target outcome.

## Open Questions

None. Optional environment lookup, explicit requirement behavior, precedence,
prepared-execution timing, outbound header semantics, error compatibility,
documentation ownership, and test boundaries are fixed by this plan.