[barbatdev]

Linked Issue

Refs #324

Umbrella: #401

PR Type

• type:feature Feature
• type:bug Bug fix
• type:docs Documentation
• type:refactor Refactoring
• type:chore Maintenance
• type:breaking-change Breaking change

Summary

Validate the existing canonical catalog root and catalog.json before qa setup probes or writes. Relative setup and run address the same profile source; invalid roots create no tree.

Changes

This is slice 4/8, 162 authored changed lines. Code, focused tests and applicable operating documentation stay together. Exact reviewed source commit: 47ab76063bcf8f23c88e6504ef7c65a595e013a5. Delivery head 48aa19a7b343470ae2a527beddecefb475ea7414 has the identical complete source tree.

Focused Tests

Each cumulative prepared slice passed go test ./internal/cli ./internal/qapi ./internal/qaroute ./internal/installobserve -count=1, go vet ./... and whitespace checks. Final candidate additionally passed go test ./... -count=1, go vet ./..., go test -race ./... and all 15 repository Python policy tests on a byte-verified canonical clean snapshot. Independent source-agreement, typed-admission and final security verification passed with no severe blockers; final verification observed 19 named tests plus four package suites/vet.

Native assessment of this exact slice: medium native tier, reviewDue=false (under_budget); no new native review started. Security-affecting slices retained explicit high-risk escalation and independent verification rather than treating assessment as proof. Parser's separately requested native review completed approved and acknowledged.

Runtime Harness

Disposable public CLI fixtures, no real provider calls. Rejections pin no launch/write effects, route identity and safe profile tokens. macOS /tmp is a symlink and intentionally refused: canonical snapshot cwd passed unchanged guards. No credentials changed.

Rollback

Revert this slice. Revert any dependent later slice first; no stored-data migration.

Chain Dependencies / Out of Scope

Sequential verified-main strategy: merged #404 -> [current slice 4/8] -> remaining slices. This PR targets main only after its predecessor lands, so its diff contains only this work unit. Do not close the umbrella from this intermediate slice.

Known acceptance limit: #329's literal transient/persistent causal distinction is NOT established by stage reporting and remains open. Defensive untyped branches, a currently unreachable non_nan_route mapping and a brittle wording assertion were disclosed as nonblocking verifier advisories, not silently fixed. Provider transport #345 and lifecycle #373 are out of scope.

Checklist

• Linked issue is open and status:approved.
• Exactly one matching type:* label is applied.
• Focused tests and runtime harness evidence are recorded.
• Rollback and out-of-scope boundaries are documented.

[barbatdev]

Fleet review (glmflash)

Lens: adversarial. The slice's core claim holds: setup now validates the existing canonical root before probing or writing, reuses the exact canonicality rule qa run depends on (canonicalProfileRoot: every component a real directory, no symlinks, via Lstat), never creates the tree on refusal (test asserts it), and pins the issue #324 repro end to end with a relative setup+run pair. I attacked the validation boundary and the setup/run agreement and found no blocking defect: the refusal paths are loud, the probe-order guarantee is proven by refusingPi, and the macOS /tmp policy is documented. Four non-blocking observations remain, two of them questions about the boundary this slice intentionally draws.

Suggestion (2)

• Validation stops at the root: the cortex component is checked only after setup already wrote (internal/qapi/profile.go:216): ValidateProfileCatalogRoot validates the root chain and catalog.json, but writeQAProfile then does os.MkdirAll on /cortex and writes qa-profiles.json through it. A root/cortex entry that is a symlink to another directory is never inspected by setup, while qa run refuses exactly that source (inspectProfileSource/profileFileInfo reject symlinked components, profileSourcePresent classifies it as present-but-unusable). The source-agreement rule this PR establishes is enforced on the root but not on the component the profile is actually written through.
• Reason token catalog_json_missing is reported when catalog.json exists but is a symlink or non-regular (internal/qapi/profile.go:221): ValidateProfileCatalogRoot returns Reason "catalog_json_missing" for three distinct states: the file is absent, it is a symlink, or it is not a regular file. The token tells the operator the file is missing when it is actually present but unusable, which sends debugging in the wrong direction; the docs justify the symlink refusal but the diagnostic does not distinguish it.

Question (2)

• Test name overclaims: a root with an unparseable catalog.json passes setup validation (internal/cli/qa_test.go:424): TestQASetupRefusesCatalogRootsQaRunCouldNotLoad only covers absent root, absent catalog.json, and symlinked root. ValidateProfileCatalogRoot checks catalog.json existence and regularity, not loadability. The code comment in qasetup.go states the invariant 'the profile is only useful in a catalog qa run can load', but setup happily writes a profile into a root whose catalog.json exists yet cannot be loaded.
• Issue Bug: qa run silently ignores the route profile when the catalog path is not canonical-absolute #324 also complains the run refusal never mentions the absolute-path requirement (internal/qapi/preflight.go): The linked issue names three complaints: the silent profile ignore (covered here and by earlier slices), the refusal that never says the catalog path must be absolute/canonical, and the profile inside a release tarball's catalog directory. This slice touches only setup-side validation and operating.md; the run-side refusal text is unchanged. The PR body declares known limits for Bug: qa admission refusals can surface no stage and appear transient; doctor does not diagnose admission #329 but not for this second complaint.

Verdict: ready to merge

[barbatdev]

Post-merge QA (glm)

Slice 4/8: qa setup now validates the catalog root through the same agreement qa run depends on (existing root, no symlinks, catalog.json present and regular) before probing models or writing, so a typo is refused without materializing a tree no run reads. Relative setup and run addresses the same profile source, pinned end-to-end by TestQASetupAndRunAgreeOnARelativeCatalog, and the refusingPi fixture proves the runtime is never probed before validation. Docs document the macOS /tmp canonical-path requirement. Nothing found that breaks what landed.

Suggestion (1)

• catalog_json_missing is reported when catalog.json exists but is a symlink or non-regular (internal/qapi/profile.go:218): ValidateProfileCatalogRoot returns Reason catalog_json_missing when the Lstat succeeds but the entry is a symlink or not a regular file. The token says the file is missing when it actually exists but is unusable, which can send an operator hunting for a typo instead of checking what catalog.json points at. A distinct reason token (e.g. catalog_json_unusable) would make the refusal truthful.

Verdict: clean

[barbatdev]

Post-merge QA (deepseek)

Lens: evidence + test design. Setup and run now agree on the same profile source: qa setup resolves the path with filepath.Abs before writing, validates the existing root, and refuses before probing models, which the refusingPi resolver proves directly (calls must stay 0). The relative-path end-to-end test reproduces the issue's example. One product question is left open: the slice makes an existing CLI surface stricter than the linked issue asked for, and the only place that decision is recorded is the operating guide.

Question (1)

• qa setup now refuses every catalog root without catalog.json (internal/qapi/profile.go:213): Bug: qa run silently ignores the route profile when the catalog path is not canonical-absolute #324 asked for two things: qa run honouring the same relative path qa setup wrote, and the refusal naming the requirement. ValidateProfileCatalogRoot goes further and makes an existing command stricter — a directory that used to accept qa setup --catalog <dir> now fails with setup_failed, catalog_json_missing. The PR documents it in the walkthrough and the fixture shows the intent, but the linked issue never asks setup to refuse a tree it could legitimately host a caller-owned profile in, and qa run still creates nothing. Is refusing to create the tree the product decision (setup only writes where a run can read), or should setup keep writing and leave the refusal to run?

Verdict: clean