diff --git a/docs/operating.md b/docs/operating.md index a2e925a..c0bbb87 100644 --- a/docs/operating.md +++ b/docs/operating.md @@ -199,7 +199,12 @@ A profile source that exists but cannot be read safely is refused, never silentl replaced: a catalog root reached through a symlink, a symlinked or non-regular profile file, an oversized or malformed file, or an unreadable `defaultProfile` all end in `profile_invalid` before anything is launched. Only a catalog with no -`cortex/qa-profiles.json` file at all uses the placeholder `nan` route. +`cortex/qa-profiles.json` file at all uses the placeholder `nan` route. A refused +`profile_invalid` prints `profile_selected` (when `--profile` was passed), the +source's `profile_default` and `profile_valid_ids` once the file is readable, and +the `route_setup` path to edit, so an unknown id points at the file instead of +guessing. `profile_valid_ids` is the sorted set the route policy itself accepts, +read through the same decoder the run uses. Neither command accepts a catalog root reached through a symlink: every path component must be a real directory, and `qa setup` refuses to create a missing @@ -209,6 +214,27 @@ includes the system temporary directory: `/tmp` is a symlink to `/private/tmp`, a catalog under `/tmp/...` is refused until it is addressed through its canonical `/private/tmp/...` path (`cd` into it and use `pwd -P`). +## Read a qa run refusal + +A `qa run` refusal names the preflight question that refused it (#374), using one +fixed vocabulary and no guessed cause: + +- `stage=route_resolution` — the route could not be resolved: an unknown or + unusable `--profile`, a backend route the profile does not carry, or a route the + policy does not admit. Nothing is launched and no provider is claimed, because + none was resolved. +- `stage=actor_admission` — the installed assets of the selected role were refused; + the following `admission_stage` names the check. +- `stage=model_probe` — the backend's own availability probe answered that the + routed pair is unusable. `route_provider` and `route_model` name the pair that was + actually routed (nothing else), escaped for the terminal, and `model_unavailable` + or `auth_not_ready` prints its own remedy. +- `stage=evidence_admission` — the supplied evidence sidecar was refused before + launch, with the `evidence_field` and `evidence_reason` that refused it. + +The vocabulary is preflight-only. A post-launch outcome (execution, timeout, +stream, normalization) prints no stage, because no preflight question refused it. +The `note=prerequisites` line names runtimes only and never claims a route. ## Read an admission refusal diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 99a3508..0f02f62 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -248,33 +248,33 @@ const ( "cortex qa run --role requirements-analyst --request request.txt --catalog ./catalog\n" + "Optional evidence sidecar (--evidence evidence.json): a JSON array of execution records\n" + "(up to 8 records, <= 48 KiB total; custom top-level shapes like {\"schema\":...} are rejected).\n" + - "Record fields:\n" + - " - id (string, required): unique record identifier, trimmed, non-empty\n" + - " - command (string, optional): executed command invocation\n" + - " - exit_code (integer, optional): process exit code\n" + - " - output_tail (string, optional): tail of command output (<= 4096 bytes)\n" + - " - provenance (string, optional): origin or fixture source\n" + - " - truncated (boolean, optional): true if output was truncated\n" + + "Record fields: id (required); command, exit_code, output_tail (<= 4096 bytes), provenance, truncated.\n" + "Example:\n" + `[{"id":"selected-test","command":"go test ./...","exit_code":0,"output_tail":"ok example","provenance":"caller fixture","truncated":false}]` + "\n" + "Schema reference: docs/operating.md\n" + + "Optional --profile selects a route profile; without it the file's defaultProfile applies.\n" + + "A refused preflight names its stage: route_resolution, actor_admission, model_probe or\n" + + "evidence_admission; a refused profile also lists profile_valid_ids from the profile file.\n" + "To audit a code diff, prefix every diff line with '>' (blockquote): quoted lines are content,\n" + "so frame directives stay rejected while the quoted diff passes validation (issue #305).\n" rootHelp = "usage: cortex [arguments]\ncommands: doctor, install, update, uninstall, qa\noptions: --version, --help\n" + "install [--target-dir ]: configure every compatible present runtime, or project the\ncatalog as plain SKILL.md files into an existing directory (Agent Skills layout)\n" + "doctor [--target-dir ]: probe runtimes, or report one directory target\n" + "uninstall [--target-dir ]: remove owned files from every runtime root, or from \n\n" + qaHelp - // qaReportNote surfaces runtime prerequisites on non-catalog failures. - // The default route provider is the policy placeholder "nan"; Cortex applies - // no model fallback and owns no automatic configuration. + // qaReportNote surfaces runtime prerequisites on non-catalog failures. It + // names no route: a refusal happens with a profile route resolved, with a + // placeholder route, or with no route resolved at all, so claiming one here + // was wrong for every failure it accompanied (#374 S5). The stage line and + // the routed pair are printed where the pipeline actually knows them. // // It names the three backends rather than one runtime, and pins no version // on any of them: the route policy admits all three, and decision A of the // backend port removed the exact version pin from every binding, so a note // that still demanded "Pi 0.85.1" named a prerequisite that no longer // exists and hid two that do. `cortex doctor` is named because it is the - // command that answers, per runtime, which of them can actually run. - qaReportNote = "note=prerequisites: one of the pi, claude or opencode runtimes with a usable backend and installed cortex assets; run cortex doctor for per-runtime backend availability; the default route provider is the policy placeholder \"nan\" and cortex applies no model fallback\n" + // command that answers, per runtime, which of them can actually run — for + // the embedded catalog and the policy default route only. + qaReportNote = "note=prerequisites: one of the pi, claude or opencode runtimes with a usable backend and installed cortex assets; run cortex doctor for per-runtime backend availability; cortex applies no model fallback and owns no automatic configuration\n" ) func writeHelp(stdout, stderr io.Writer, help string) int { @@ -384,6 +384,9 @@ func runQA(ctx context.Context, args []string, stdout, stderr io.Writer) int { return exitFailure } failure := qaFailure(stderr, string(code)) + if stage := qaRefusalStage(string(code), err); stage != "" { + _, _ = fmt.Fprintf(stderr, "stage=%s\n", stage) + } var evidenceErr *qapi.EvidenceValidationError if errors.As(err, &evidenceErr) { if evidenceErr.Field != "" { @@ -414,6 +417,9 @@ func runQA(ctx context.Context, args []string, stdout, stderr io.Writer) int { if code == "route_incomplete" { _, _ = fmt.Fprintf(stderr, "route_missing_backend=%s remedy=cortex qa setup --catalog %s --backend %s --provider

--model \n", backend, catalogRoot, backend) } + if code == qaadmission.CodeProfileInvalid { + _, _ = io.WriteString(stderr, qaProfileDiagnostic(profile, catalogRoot)) + } if code == qaadmission.CodeNormalizationFailed { var diagnostic *qapi.ReportNormalizationError if errors.As(err, &diagnostic) { @@ -455,6 +461,44 @@ func qaFailure(stderr io.Writer, code string) int { return exitFailure } +// The four preflight questions a `qa run` refusal can answer (#374 S4). This +// is a fixed vocabulary and not a new set of gates: each token names the +// pipeline step whose existing error or code refused the run, and nothing +// else. evidence_admission covers the evidence sidecar validation, which runs +// before the route; route_resolution covers every qaroute failure code; +// actor_admission covers the typed installed-asset refusal; model_probe covers +// the backend availability verdict. Post-launch outcomes (execution, stream, +// normalization) are not preflight questions and print no stage line. +const ( + qaStageRouteResolution = "route_resolution" + qaStageActorAdmission = "actor_admission" + qaStageModelProbe = "model_probe" + qaStageEvidenceAdmission = "evidence_admission" +) + +// qaRefusalStage classifies one refused report run by the existing error and +// code it already carries. It never guesses a root cause: when no preflight +// step refused, it reports no stage at all. +func qaRefusalStage(code string, err error) string { + var evidence *qapi.EvidenceValidationError + if errors.As(err, &evidence) { + return qaStageEvidenceAdmission + } + var unavailable *qapi.BackendUnavailableError + if errors.As(err, &unavailable) { + return qaStageModelProbe + } + var admission *installobserve.AdmissionUnavailableError + if errors.As(err, &admission) { + return qaStageActorAdmission + } + switch code { + case "profile_invalid", "route_incomplete", "route_disallowed", "non_nan_route": + return qaStageRouteResolution + } + return "" +} + // qaModelUnavailableNote names the problem behind a refused routed pair and // its actual remedy (issue #339): qa runs probe pi with --no-extensions, so a // provider registered only by a Pi extension is invisible to qa forever, and @@ -478,6 +522,44 @@ func qaRouteSetupNote(catalogRoot string) string { return fmt.Sprintf("route_setup=%s/cortex/qa-profiles.json example=%s\n", catalogRoot, example) } +// qaProfileDiagnostic names the profile a refused run selected and what the +// catalog's profile source actually offers (#374 S6), so an unknown id leads to +// the file instead of a guess. It reads the same safe snapshot the route policy +// reads — never a second, weaker path — and every caller-supplied token is +// bounded and escaped for the operator's terminal. The valid ids are the ones +// the route policy itself accepts, so a refusal enumerates what the operator +// can actually select. +func qaProfileDiagnostic(selected, catalogRoot string) string { + fields := make([]string, 0, 5) + if selected != "" { + fields = append(fields, "profile_selected="+sanitizeProfileToken(selected)) + } + if defaultID, ids, ok := qapi.ProfileSelection(catalogRoot); ok { + fields = append(fields, "profile_source=readable", "profile_default="+defaultID) + fields = append(fields, "profile_valid_ids="+strings.Join(ids, ",")) + } else { + fields = append(fields, "profile_source=unusable") + } + fields = append(fields, "route_setup="+filepath.Join(catalogRoot, "cortex", "qa-profiles.json")) + return strings.Join(fields, " ") + "\n" +} + +// sanitizeProfileToken bounds one caller-supplied profile id for a diagnostic +// line: admitted ids are lowercase words joined by dashes, so anything else is +// replaced rather than echoed. +func sanitizeProfileToken(selected string) string { + if len(selected) == 0 || len(selected) > 64 { + return "" + } + for _, character := range selected { + allowed := character >= 'a' && character <= 'z' || character >= '0' && character <= '9' || character == '-' + if !allowed { + return "" + } + } + return selected +} + // qaAdmissionDiagnostic renders one bounded diagnostic line for admission // failures, naming the stage and the expected/found identity (issue #294). // Digests and the catalog fingerprint are the only values it may carry. diff --git a/internal/cli/qa_test.go b/internal/cli/qa_test.go index 66f476e..10debd6 100644 --- a/internal/cli/qa_test.go +++ b/internal/cli/qa_test.go @@ -3,11 +3,13 @@ package cli import ( "bytes" "context" + "crypto/sha256" "encoding/json" "fmt" "io" "os" "path/filepath" + "sort" "strings" "testing" "time" @@ -258,6 +260,63 @@ func TestQARunUnloadableCatalogNamesTheStage(t *testing.T) { } } +// TestQARunRefusesAShadowedActorBeforeLaunch pins the public half of #329 and +// #374 S3/S4: a run whose selected actor is shadowed by a project file is +// refused at the admission stage that refused, and the refusal names it instead +// of reporting only that assets were unavailable. Nothing is launched, and the +// disposable install tree and working directory are unchanged afterwards. +func TestQARunRefusesAShadowedActorBeforeLaunch(t *testing.T) { + args := roleReportFixture(t, qarole.RequirementsAnalyst) + cwd := must(os.Getwd()) + shadow := filepath.Join(cwd, ".pi", "agents", "other.md") + mustOK(os.MkdirAll(filepath.Dir(shadow), 0o700)) + mustOK(os.WriteFile(shadow, []byte("---\nname: cortex-requirements-analyst\n---\n"), 0o600)) + before := qaFixtureFingerprint(t, os.Getenv("HOME"), cwd) + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + code := Run(context.Background(), args, stdout, stderr, nil) + want := "error=actor_unavailable\n" + qaReportNote + "stage=actor_admission\nadmission_stage=actor_shadow_unverified\n" + if code != exitFailure || stdout.Len() != 0 || stderr.String() != want { + t.Fatalf("shadowed run = %d stdout %q stderr %q, want %q", code, stdout.String(), stderr.String(), want) + } + for _, artifact := range []string{"qa-report-frame", qaReportRoleEvidencePath, "qa-report-invocations"} { + if _, err := os.Stat(filepath.Join(cwd, artifact)); !os.IsNotExist(err) { + t.Fatalf("a shadowed actor wrote %q, want no launch", artifact) + } + } + if after := qaFixtureFingerprint(t, os.Getenv("HOME"), cwd); after != before { + t.Fatalf("a refused run changed the fixture tree:\nbefore:\n%s\nafter:\n%s", before, after) + } +} + +// qaFixtureFingerprint summarizes the disposable roots one refusal may touch — +// relative path, mode, size, and content digest per entry — so a test can prove +// a refused run wrote nothing. It reads only the fixture roots it is given. +func qaFixtureFingerprint(t *testing.T, roots ...string) string { + t.Helper() + lines := make([]string, 0, 64) + for _, root := range roots { + if err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + info, err := entry.Info() + if err != nil { + return err + } + line := fmt.Sprintf("%s %s %d", path, info.Mode(), info.Size()) + if info.Mode().IsRegular() { + line += fmt.Sprintf(" %x", sha256.Sum256(must(os.ReadFile(path)))) + } + lines = append(lines, line) + return nil + }); err != nil { + t.Fatal(err) + } + } + sort.Strings(lines) + return strings.Join(lines, "\n") +} + func TestQARunAcceptsPiInternalRetryWithoutRelaunch(t *testing.T) { args := reportFixture(t) mustOK(os.WriteFile("qa-report-scenario", []byte("retry-report"), 0o600)) @@ -512,6 +571,15 @@ func qaCatalogArgument(args []string) string { // The catalog root is the production catalog the admission binding compares // against, so any file already there is restored, never left mutated. func qaProfileFileFixture(t *testing.T, catalogRoot string, role qarole.RoleID) { + t.Helper() + profile := fmt.Sprintf(`{"schemaVersion":1,"defaultProfile":"fleet-default","profiles":{"fleet-default":{"routes":{"%s":{"pi":{"provider":"nan","model":"qwen3.6","effort":"low"}}}},"qa-fleet":{"routes":{"%s":{"pi":{"provider":"openai-codex","model":"gpt-5.6-terra","effort":"high"}}}}}}`, role, role) + qaProfileBodyFixture(t, catalogRoot, profile) +} + +// qaProfileBodyFixture stages one exact profile body in the catalog root and +// restores the previous state afterwards. A profile the test itself authored +// is the only way to pin what a diagnostic does with caller-supplied text. +func qaProfileBodyFixture(t *testing.T, catalogRoot, body string) { t.Helper() profileDirectory := filepath.Join(catalogRoot, "cortex") mustOK(os.MkdirAll(profileDirectory, 0o700)) @@ -525,8 +593,7 @@ func qaProfileFileFixture(t *testing.T, catalogRoot string, role qarole.RoleID) _ = os.Remove(path) _ = os.Remove(profileDirectory) }) - profile := fmt.Sprintf(`{"schemaVersion":1,"defaultProfile":"fleet-default","profiles":{"fleet-default":{"routes":{"%s":{"pi":{"provider":"nan","model":"qwen3.6","effort":"low"}}}},"qa-fleet":{"routes":{"%s":{"pi":{"provider":"openai-codex","model":"gpt-5.6-terra","effort":"high"}}}}}}`, role, role) - mustOK(os.WriteFile(path, []byte(profile), 0o600)) + mustOK(os.WriteFile(path, []byte(body), 0o600)) } // profileReportFixture materializes the report fixture plus the two-profile @@ -581,6 +648,111 @@ func TestQARunProfileFlagSelectsTheRequestedProfile(t *testing.T) { }) } +// TestQARunNamesThePreflightRefusalStage pins #374 S4: each preflight refusal +// says which question refused — route resolution, actor admission, model probe, +// or evidence admission — using the stage vocabulary the pipeline already +// distinguishes, and a post-launch outcome claims no preflight stage at all. +func TestQARunNamesThePreflightRefusalStage(t *testing.T) { + t.Run("route resolution names no resolved route", func(t *testing.T) { + args := profileReportFixture(t) + stderr, code := runQARefusal(t, append(append([]string{}, args...), "--profile", "no-such-profile")) + for _, want := range []string{"stage=route_resolution\n", "profile_valid_ids=fleet-default,qa-fleet"} { + if !strings.Contains(stderr, want) { + t.Fatalf("unknown --profile stderr %q does not name %q", stderr, want) + } + } + // A refused route resolution never claims a routed pair: none was + // resolved, so naming one would be a guess (S4/S5). + if code != exitFailure || strings.Contains(stderr, "route_provider=") || strings.Contains(stderr, "placeholder") { + t.Fatalf("unknown --profile = %d stderr %q, want no claimed route", code, stderr) + } + }) + t.Run("model probe names the routed pair the profile resolved", func(t *testing.T) { + args := profileReportFixture(t) + mustOK(os.WriteFile("qa-report-scenario", []byte("model-unavailable"), 0o600)) + stderr, code := runQARefusal(t, append(append([]string{}, args...), "--profile", "qa-fleet")) + for _, want := range []string{"stage=model_probe\n", "route_provider=openai-codex route_model=gpt-5.6-terra\n"} { + if !strings.Contains(stderr, want) { + t.Fatalf("model probe stderr %q does not name %q", stderr, want) + } + } + if code != exitFailure { + t.Fatalf("model probe = %d stderr %q", code, stderr) + } + }) + t.Run("an incomplete backend route is a route resolution refusal", func(t *testing.T) { + args := reportFixture(t) + catalogRoot := filepath.Join(must(os.Getwd()), "catalog-copy") + mustOK(os.CopyFS(catalogRoot, os.DirFS(qaCatalogArgument(args)))) + for index, arg := range args { + if arg == "--catalog" && index+1 < len(args) { + args[index+1] = catalogRoot + } + } + // The profile routes this role on claude only, so a pi run is + // incomplete rather than unresolvable. + qaProfileBodyFixture(t, catalogRoot, `{"schemaVersion":1,"defaultProfile":"fleet","profiles":{"fleet":{"routes":{"requirements-analyst":{"claude":{"provider":"claude","model":"claude-fixture","effort":"medium"}}}}}}`) + stderr, code := runQARefusal(t, args) + for _, want := range []string{"error=route_incomplete\n", "stage=route_resolution\n", "route_missing_backend=pi remedy="} { + if !strings.Contains(stderr, want) { + t.Fatalf("incomplete route stderr %q does not name %q", stderr, want) + } + } + if code != exitFailure { + t.Fatalf("incomplete route = %d stderr %q", code, stderr) + } + }) + t.Run("evidence admission is named before launch", func(t *testing.T) { + args := reportFixture(t) + mustOK(os.WriteFile("evidence.json", []byte(`{"schema":"custom"}`), 0o600)) + stderr, code := runQARefusal(t, append(append([]string{}, args...), "--evidence", "evidence.json")) + for _, want := range []string{"error=invalid_request\n", "stage=evidence_admission\n"} { + if !strings.Contains(stderr, want) { + t.Fatalf("evidence refusal stderr %q does not name %q", stderr, want) + } + } + if code != exitFailure { + t.Fatalf("evidence refusal = %d stderr %q", code, stderr) + } + if _, err := os.Stat("qa-report-frame"); !os.IsNotExist(err) { + t.Fatal("a refused evidence sidecar must not launch the backend") + } + }) + t.Run("a post-launch outcome claims no preflight stage", func(t *testing.T) { + args := reportFixture(t) + mustOK(os.WriteFile("qa-report-scenario", []byte("nonzero"), 0o600)) + stderr, code := runQARefusal(t, args) + if code != exitFailure || strings.Contains(stderr, "stage=") { + t.Fatalf("post-launch failure = %d stderr %q, want no preflight stage", code, stderr) + } + }) +} + +// TestQAReportNoteClaimsNoRoute pins #374 S5: the prerequisite note printed +// with every non-catalog refusal no longer claims the placeholder "nan" route, +// because a refusal often happens with a profile route already resolved (or +// with no route resolved at all). +func TestQAReportNoteClaimsNoRoute(t *testing.T) { + for _, banned := range []string{"nan", "placeholder", "default route"} { + if strings.Contains(qaReportNote, banned) { + t.Fatalf("the prerequisite note still claims %q: %q", banned, qaReportNote) + } + } +} + +// runQARefusal runs one public qa invocation that must be refused, and returns +// its stderr and exit code. Success is a test bug: the callers assert on +// refusals only. +func runQARefusal(t *testing.T, args []string) (string, int) { + t.Helper() + stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} + code := Run(context.Background(), args, stdout, stderr, nil) + if code == exitOK { + t.Fatalf("Run(%q) unexpectedly succeeded: stdout %q", args, stdout.String()) + } + return stderr.String(), code +} + // TestQARunAcceptsTheDocumentedOptionalFlagsTogether pins the documented // optional-flag surface (#374 S6): --backend, --evidence and --profile are // independent and combinable in any order after the required flags.