Improve error handling and diagnostics for the audita subprocess
This commit is contained in:
11
README.md
11
README.md
@@ -26,8 +26,8 @@ Seriatim config contract in `pipeline.yml`:
|
|||||||
Audita config contract in `pipeline.yml`:
|
Audita config contract in `pipeline.yml`:
|
||||||
|
|
||||||
- required: `audita.binary` (name or path; existence is checked at execution time, not config validation time)
|
- required: `audita.binary` (name or path; existence is checked at execution time, not config validation time)
|
||||||
- optional: `audita.llm_api_key_env` (environment variable name holding the API key secret; defaults to `AUDITA_LLM_API_KEY`)
|
- optional: `audita.llm_api_key_env` (environment variable name holding the API key secret; no automatic default)
|
||||||
- defaulted when omitted: `audita.timeout` (`3h`), `audita.llm_api_key_env` (`AUDITA_LLM_API_KEY`), `audita.modules` (`glossary,homophones,glossary,spoken_word,grammar,homophones,glossary`), `audita.base_url` (`https://openrouter.ai/api/v1`), `audita.model` (`openrouter/google/gemma-4-31b-it`), `audita.llm_concurrency` (`1`), `audita.validation_model` (`""`), `audita.validation_llm_concurrency` (`1`), `audita.report` (`true`)
|
- defaulted when omitted: `audita.timeout` (`3h`), `audita.modules` (`glossary,homophones,glossary,spoken_word,grammar,homophones,glossary`), `audita.base_url` (`https://openrouter.ai/api/v1`), `audita.model` (`openrouter/google/gemma-4-31b-it`), `audita.llm_concurrency` (`1`), `audita.validation_model` (`""`), `audita.validation_llm_concurrency` (`1`), `audita.report` (`true`)
|
||||||
- allowed `audita.modules` values: `glossary`, `homophones`, `spoken_word`, `grammar` (order and repeats are allowed)
|
- allowed `audita.modules` values: `glossary`, `homophones`, `spoken_word`, `grammar` (order and repeats are allowed)
|
||||||
- `audita.base_url` must be a valid URL when provided
|
- `audita.base_url` must be a valid URL when provided
|
||||||
- `audita.llm_concurrency` and `audita.validation_llm_concurrency` must be `> 0`
|
- `audita.llm_concurrency` and `audita.validation_llm_concurrency` must be `> 0`
|
||||||
@@ -36,7 +36,8 @@ Audita credentials note:
|
|||||||
|
|
||||||
- store only the environment variable **name** in config (`audita.llm_api_key_env`), never the API key value itself
|
- store only the environment variable **name** in config (`audita.llm_api_key_env`), never the API key value itself
|
||||||
- API key values must not be written to pipeline config, generated configs, logs, or manifest metadata
|
- API key values must not be written to pipeline config, generated configs, logs, or manifest metadata
|
||||||
- if the named env var is not set (or is empty), Narratio omits `AUDITA_LLM_API_KEY` from the Audita subprocess environment instead of failing
|
- if `audita.llm_api_key_env` is configured and the named env var is not set (or is empty), Narratio fails before invocation with a redacted error
|
||||||
|
- if `audita.llm_api_key_env` is omitted/empty, Narratio does not require a credential and omits `AUDITA_LLM_API_KEY` from the subprocess overrides
|
||||||
|
|
||||||
Audita runtime note:
|
Audita runtime note:
|
||||||
|
|
||||||
@@ -98,6 +99,10 @@ Default CLI wiring builds and uses:
|
|||||||
- real Seriatim subprocess adapter from `pipeline.seriatim`
|
- real Seriatim subprocess adapter from `pipeline.seriatim`
|
||||||
- real Audita subprocess adapter from `pipeline.audita`
|
- real Audita subprocess adapter from `pipeline.audita`
|
||||||
|
|
||||||
|
Subprocess runtime note:
|
||||||
|
|
||||||
|
- subprocesses inherit the parent environment by default, then apply Narratio override values (override values win).
|
||||||
|
|
||||||
Real `polish` stage output paths:
|
Real `polish` stage output paths:
|
||||||
|
|
||||||
- `transcripts/processed.json`
|
- `transcripts/processed.json`
|
||||||
|
|||||||
@@ -132,7 +132,7 @@ Audita config keys:
|
|||||||
|
|
||||||
- `pipeline.audita.binary` (required)
|
- `pipeline.audita.binary` (required)
|
||||||
- `pipeline.audita.timeout` (default: `3h`)
|
- `pipeline.audita.timeout` (default: `3h`)
|
||||||
- `pipeline.audita.llm_api_key_env` (optional; default: `AUDITA_LLM_API_KEY`)
|
- `pipeline.audita.llm_api_key_env` (optional; no automatic default)
|
||||||
- `pipeline.audita.modules` (default sequence: `glossary,homophones,glossary,spoken_word,grammar,homophones,glossary`)
|
- `pipeline.audita.modules` (default sequence: `glossary,homophones,glossary,spoken_word,grammar,homophones,glossary`)
|
||||||
- `pipeline.audita.base_url` (default: `https://openrouter.ai/api/v1`)
|
- `pipeline.audita.base_url` (default: `https://openrouter.ai/api/v1`)
|
||||||
- `pipeline.audita.model` (default: `openrouter/google/gemma-4-31b-it`)
|
- `pipeline.audita.model` (default: `openrouter/google/gemma-4-31b-it`)
|
||||||
@@ -145,7 +145,8 @@ Audita secret-handling policy:
|
|||||||
|
|
||||||
- `llm_api_key_env` stores only the environment variable **name**.
|
- `llm_api_key_env` stores only the environment variable **name**.
|
||||||
- API key values are read from the process environment at runtime and are not stored in `pipeline.yml`, manifest metadata, generated configs, or logs.
|
- API key values are read from the process environment at runtime and are not stored in `pipeline.yml`, manifest metadata, generated configs, or logs.
|
||||||
- If the named env var is missing/empty, the Audita adapter omits `AUDITA_LLM_API_KEY` from subprocess env overrides and continues.
|
- If `llm_api_key_env` is configured and the named env var is missing/empty, the Audita adapter fails before invocation with a redacted error.
|
||||||
|
- If `llm_api_key_env` is empty/omitted, the Audita adapter omits `AUDITA_LLM_API_KEY` from subprocess env overrides and continues.
|
||||||
|
|
||||||
Validation currently enforces:
|
Validation currently enforces:
|
||||||
|
|
||||||
@@ -331,6 +332,7 @@ All adapters currently have fake/no-op implementations for tests/scaffold execut
|
|||||||
|
|
||||||
- Context cancellation + optional timeout.
|
- Context cancellation + optional timeout.
|
||||||
- Explicit executable/args, working dir, env overrides.
|
- Explicit executable/args, working dir, env overrides.
|
||||||
|
- Parent environment inheritance with override merge semantics (override values win).
|
||||||
- Stdout/stderr log file handling.
|
- Stdout/stderr log file handling.
|
||||||
- Exit code and timing capture.
|
- Exit code and timing capture.
|
||||||
- Actionable error wrapping.
|
- Actionable error wrapping.
|
||||||
|
|||||||
@@ -157,14 +157,16 @@ func (r *SubprocessRunner) Run(ctx context.Context, req PolishRequest) (PolishRe
|
|||||||
}
|
}
|
||||||
args := r.buildArgs(req, reqModules)
|
args := r.buildArgs(req, reqModules)
|
||||||
|
|
||||||
|
credentialEnvVar := strings.TrimSpace(r.llmAPIKeyEnv)
|
||||||
credentialPresent := false
|
credentialPresent := false
|
||||||
env := map[string]string{}
|
env := map[string]string{}
|
||||||
if strings.TrimSpace(r.llmAPIKeyEnv) != "" {
|
if credentialEnvVar != "" {
|
||||||
credential, ok := os.LookupEnv(r.llmAPIKeyEnv)
|
credential, ok := os.LookupEnv(credentialEnvVar)
|
||||||
if ok && strings.TrimSpace(credential) != "" {
|
if !ok || strings.TrimSpace(credential) == "" {
|
||||||
env["AUDITA_LLM_API_KEY"] = credential
|
return PolishResult{}, fmt.Errorf("audita: required credential environment variable %s is not set", credentialEnvVar)
|
||||||
credentialPresent = true
|
|
||||||
}
|
}
|
||||||
|
env["AUDITA_LLM_API_KEY"] = credential
|
||||||
|
credentialPresent = true
|
||||||
}
|
}
|
||||||
primaryConcurrencyViaEnv := false
|
primaryConcurrencyViaEnv := false
|
||||||
if r.llmConcurrency != nil {
|
if r.llmConcurrency != nil {
|
||||||
@@ -187,7 +189,13 @@ func (r *SubprocessRunner) Run(ctx context.Context, req PolishRequest) (PolishRe
|
|||||||
StderrLogPath: req.StderrLogPath,
|
StderrLogPath: req.StderrLogPath,
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return r.failureResult(req, reqModules, runRes, credentialPresent, primaryConcurrencyViaEnv), fmt.Errorf("run audita process (binary=%q): %w", r.binary, err)
|
return r.failureResult(req, reqModules, runRes, credentialPresent, primaryConcurrencyViaEnv), fmt.Errorf(
|
||||||
|
"run audita process (binary=%q, stdout_log=%q, stderr_log=%q): %w",
|
||||||
|
r.binary,
|
||||||
|
req.StdoutLogPath,
|
||||||
|
req.StderrLogPath,
|
||||||
|
err,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := validateProcessedOutput(req.OutputProcessedPath); err != nil {
|
if err := validateProcessedOutput(req.OutputProcessedPath); err != nil {
|
||||||
|
|||||||
@@ -120,7 +120,33 @@ func TestSubprocessRunnerSuccessArgsEnvAndValidation(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSubprocessRunnerMissingCredentialEnvOmitsCredential(t *testing.T) {
|
func TestSubprocessRunnerMissingConfiguredCredentialFails(t *testing.T) {
|
||||||
|
if runtime.GOOS == "windows" {
|
||||||
|
t.Skip("helper wrapper script uses /bin/sh")
|
||||||
|
}
|
||||||
|
llmConcurrency := 1
|
||||||
|
runner := mustAuditaRunner(t, SubprocessRunnerConfig{
|
||||||
|
Binary: writeAuditaHelperWrapper(t),
|
||||||
|
Timeout: mustParseAuditaDuration(t, "2s"),
|
||||||
|
LLMAPIKeyEnv: "MISSING_AUDITA_KEY",
|
||||||
|
Modules: []string{"glossary"},
|
||||||
|
BaseURL: "https://openrouter.ai/api/v1",
|
||||||
|
Model: "openrouter/google/gemma-4-31b-it",
|
||||||
|
LLMConcurrency: &llmConcurrency,
|
||||||
|
Report: false,
|
||||||
|
})
|
||||||
|
req := auditaReqForTest(t, false)
|
||||||
|
|
||||||
|
_, err := runner.Run(context.Background(), req)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error, got nil")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "required credential environment variable MISSING_AUDITA_KEY is not set") {
|
||||||
|
t.Fatalf("error = %q, want missing credential guidance", err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSubprocessRunnerUnconfiguredCredentialEnvOmitsCredential(t *testing.T) {
|
||||||
if runtime.GOOS == "windows" {
|
if runtime.GOOS == "windows" {
|
||||||
t.Skip("helper wrapper script uses /bin/sh")
|
t.Skip("helper wrapper script uses /bin/sh")
|
||||||
}
|
}
|
||||||
@@ -133,7 +159,7 @@ func TestSubprocessRunnerMissingCredentialEnvOmitsCredential(t *testing.T) {
|
|||||||
runner := mustAuditaRunner(t, SubprocessRunnerConfig{
|
runner := mustAuditaRunner(t, SubprocessRunnerConfig{
|
||||||
Binary: writeAuditaHelperWrapper(t),
|
Binary: writeAuditaHelperWrapper(t),
|
||||||
Timeout: mustParseAuditaDuration(t, "2s"),
|
Timeout: mustParseAuditaDuration(t, "2s"),
|
||||||
LLMAPIKeyEnv: "MISSING_AUDITA_KEY",
|
LLMAPIKeyEnv: "",
|
||||||
Modules: []string{"glossary"},
|
Modules: []string{"glossary"},
|
||||||
BaseURL: "https://openrouter.ai/api/v1",
|
BaseURL: "https://openrouter.ai/api/v1",
|
||||||
Model: "openrouter/google/gemma-4-31b-it",
|
Model: "openrouter/google/gemma-4-31b-it",
|
||||||
@@ -186,6 +212,9 @@ func TestSubprocessRunnerSubprocessFailure(t *testing.T) {
|
|||||||
if !strings.Contains(err.Error(), "exit code") {
|
if !strings.Contains(err.Error(), "exit code") {
|
||||||
t.Fatalf("error = %q, want exit code context", err.Error())
|
t.Fatalf("error = %q, want exit code context", err.Error())
|
||||||
}
|
}
|
||||||
|
if !strings.Contains(err.Error(), req.StdoutLogPath) || !strings.Contains(err.Error(), req.StderrLogPath) {
|
||||||
|
t.Fatalf("error = %q, want stdout/stderr log paths", err.Error())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSubprocessRunnerMissingOutputFails(t *testing.T) {
|
func TestSubprocessRunnerMissingOutputFails(t *testing.T) {
|
||||||
|
|||||||
@@ -109,6 +109,67 @@ func TestRunTimeout(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunInheritsParentEnvironmentByDefault(t *testing.T) {
|
||||||
|
exe, err := os.Executable()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("os.Executable() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Setenv("GO_WANT_SUBPROCESS_HELPER", "1")
|
||||||
|
t.Setenv("SUBPROCESS_HELPER_ENV_KEY", "SUBPROCESS_PARENT_VALUE")
|
||||||
|
t.Setenv("SUBPROCESS_PARENT_VALUE", "inherited-value")
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
stdoutPath := filepath.Join(dir, "stdout.log")
|
||||||
|
req := RunRequest{
|
||||||
|
Executable: exe,
|
||||||
|
Args: []string{"-test.run=TestSubprocessHelper", "--", "printenv"},
|
||||||
|
StdoutLogPath: stdoutPath,
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err := Run(context.Background(), req); err != nil {
|
||||||
|
t.Fatalf("Run() error = %v", err)
|
||||||
|
}
|
||||||
|
data, err := os.ReadFile(stdoutPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("read stdout log: %v", err)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(string(data)) != "inherited-value" {
|
||||||
|
t.Fatalf("stdout = %q, want inherited-value", strings.TrimSpace(string(data)))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRunEnvOverridesWinOverInheritedValues(t *testing.T) {
|
||||||
|
exe, err := os.Executable()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("os.Executable() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Setenv("GO_WANT_SUBPROCESS_HELPER", "1")
|
||||||
|
t.Setenv("SUBPROCESS_HELPER_ENV_KEY", "SUBPROCESS_PARENT_VALUE")
|
||||||
|
t.Setenv("SUBPROCESS_PARENT_VALUE", "parent-value")
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
stdoutPath := filepath.Join(dir, "stdout.log")
|
||||||
|
req := RunRequest{
|
||||||
|
Executable: exe,
|
||||||
|
Args: []string{"-test.run=TestSubprocessHelper", "--", "printenv"},
|
||||||
|
EnvOverrides: map[string]string{"SUBPROCESS_PARENT_VALUE": "override-value"},
|
||||||
|
StdoutLogPath: stdoutPath,
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err := Run(context.Background(), req); err != nil {
|
||||||
|
t.Fatalf("Run() error = %v", err)
|
||||||
|
}
|
||||||
|
data, err := os.ReadFile(stdoutPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("read stdout log: %v", err)
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(string(data)) != "override-value" {
|
||||||
|
t.Fatalf("stdout = %q, want override-value", strings.TrimSpace(string(data)))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestWriteYAMLAtomic(t *testing.T) {
|
func TestWriteYAMLAtomic(t *testing.T) {
|
||||||
dir := t.TempDir()
|
dir := t.TempDir()
|
||||||
path := filepath.Join(dir, "config.generated.yml")
|
path := filepath.Join(dir, "config.generated.yml")
|
||||||
@@ -190,6 +251,10 @@ func TestSubprocessHelper(t *testing.T) {
|
|||||||
case "sleep":
|
case "sleep":
|
||||||
time.Sleep(500 * time.Millisecond)
|
time.Sleep(500 * time.Millisecond)
|
||||||
os.Exit(0)
|
os.Exit(0)
|
||||||
|
case "printenv":
|
||||||
|
key := os.Getenv("SUBPROCESS_HELPER_ENV_KEY")
|
||||||
|
_, _ = os.Stdout.WriteString(os.Getenv(key) + "\n")
|
||||||
|
os.Exit(0)
|
||||||
default:
|
default:
|
||||||
os.Exit(2)
|
os.Exit(2)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -137,9 +137,6 @@ func applyAuditaDefaults(cfg *AuditaConfig) {
|
|||||||
if cfg.Timeout == "" {
|
if cfg.Timeout == "" {
|
||||||
cfg.Timeout = "3h"
|
cfg.Timeout = "3h"
|
||||||
}
|
}
|
||||||
if cfg.LLMAPIKeyEnv == "" {
|
|
||||||
cfg.LLMAPIKeyEnv = "AUDITA_LLM_API_KEY"
|
|
||||||
}
|
|
||||||
if cfg.Modules == nil {
|
if cfg.Modules == nil {
|
||||||
cfg.Modules = []string{
|
cfg.Modules = []string{
|
||||||
"glossary",
|
"glossary",
|
||||||
|
|||||||
@@ -602,8 +602,8 @@ inputs:
|
|||||||
if cfg.Pipeline.Audita.Timeout != "3h" {
|
if cfg.Pipeline.Audita.Timeout != "3h" {
|
||||||
t.Fatalf("audita.timeout = %q, want %q", cfg.Pipeline.Audita.Timeout, "3h")
|
t.Fatalf("audita.timeout = %q, want %q", cfg.Pipeline.Audita.Timeout, "3h")
|
||||||
}
|
}
|
||||||
if cfg.Pipeline.Audita.LLMAPIKeyEnv != "AUDITA_LLM_API_KEY" {
|
if cfg.Pipeline.Audita.LLMAPIKeyEnv != "" {
|
||||||
t.Fatalf("audita.llm_api_key_env = %q, want %q", cfg.Pipeline.Audita.LLMAPIKeyEnv, "AUDITA_LLM_API_KEY")
|
t.Fatalf("audita.llm_api_key_env = %q, want empty by default", cfg.Pipeline.Audita.LLMAPIKeyEnv)
|
||||||
}
|
}
|
||||||
if got := strings.Join(cfg.Pipeline.Audita.Modules, ","); got != "glossary,homophones,glossary,spoken_word,grammar,homophones,glossary" {
|
if got := strings.Join(cfg.Pipeline.Audita.Modules, ","); got != "glossary,homophones,glossary,spoken_word,grammar,homophones,glossary" {
|
||||||
t.Fatalf("audita.modules = %q, want default sequence", got)
|
t.Fatalf("audita.modules = %q, want default sequence", got)
|
||||||
|
|||||||
Reference in New Issue
Block a user