feat(provider-facts): declare constrained provider credential operations #486

Activity

1. wz-gsa commented on Oct 8, 2026

   

   wz-gsa

   on Oct 8, 2026

   Contributor

   More actions

   Reviewed this against the current tree and against GSA-TTS/agentic-coding-patterns#423's ADR-0004, which it extends. The containment reasoning holds up — I want to confirm one part of it with a measurement, then flag two overlaps and one gap.

   The no-executable-hook decision is correct, and I can show why

   Executing arbitrary provider-kit code with credentials would weaken that boundary

   This is the right call, and it is stronger than the issue states. ADR-0004's normalizer containment contract is documented but not enforced — I verified against acq.backends/ that none of its four properties has an existing primitive: no per-subprocess network restriction exists at all (policy is sandbox-wide and create-time-only), no environment-minimization helper exists (both exec paths only add -e via inheritance), the two timeout(1) uses fail open when timeout is absent and kill only the immediate child, and no subprocess output-size cap exists anywhere in either repo.

   So a credentialed provider-kit hook would not merely "weaken" the boundary — it would hand provider code a credential in an environment where nothing currently constrains what that code can reach. Static metadata plus one acq-owned HTTP probe is the only shape that is safe today.

   Overlap 1 — diagnostic.url and ADR-0004's modelsUrl are the same URL

   ADR-0004 facts.json:   "modelsUrl":      "https://api.gsa.usai.gov/api/v1/models"
   this proposal:         "diagnostic.url": "https://api.gsa.usai.gov/api/v1/models"
   

   Same URL, same artifact, two field names. That is not a coincidence — the models endpoint is the low-impact authenticated endpoint, which is exactly why it is a good diagnostic. If both fields ship, they will drift, and a reader will have no way to know which one acq actually dials.

   Suggestion: let diagnostic reference the existing modelsUrl rather than restate it — e.g. "diagnostic": { "source": "modelsUrl", "method": "GET", ... } — so the URL has one authority and the diagnostic contributes only the method, transport and success statuses.

   Overlap 2 — the credential env-var name is now recorded in three places

   ADR-0004 facts.json          keyEnv              = USAI_API_KEY
   this proposal                credential.keyEnv   = USAI_API_KEY
   providers/usai/catalog.json  gateway.apiKeyEnv   = USAI_API_KEY
   

   All three agree today. Nothing keeps them agreeing, and the env-var-ownership invariant this issue requires (keyEnv must match a variable in the same provider kit's own spec.yaml) can only be enforced against whichever copy the validator happens to read.

   GSA-TTS/agentic-coding-patterns#435 is the natural place to collapse this to one authority, and this issue's acceptance criteria already route the schema work there.

   A fourth catalog shape exists, and it is unowned

   Worth surfacing while the schema question is open — there are now four schemas describing one provider:

   Shape	Where	Carries
   acq-provider-facts/v1	ADR-0004 (pending, #423)	host, baseUrl, modelsUrl, keyEnv
   (this proposal)	pending	credential + diagnostic
   usai-model-catalog/v1	integrations/providers/usai/catalog.json	gateway, vendors, models
   acq-neutral-model-catalog/v1	acq-kits/usai-provider/files/home/usai-config/model-catalog-snapshot.json	providerId, models

   The last one already ships in the kit today and nothing in the epic tracks it. GSA-TTS/agentic-coding-patterns#357's own "Later" list says "reconcile coexisting catalog schemas first (#435)", so the reconciliation is at least named — but it is now four shapes, not two.

   Result contract — agreeing with the #460 alignment

   Align the distinction between pass and could-not-check with the verification contract introduced by GSA-TTS/agentic-coding-patterns#460.

   Strongly agree, and the eight-outcome taxonomy with indeterminate / could not check as a first-class state is the right shape. One detail from #460's implementation worth carrying over: give "could not check" its own exit code, not just its own label. A caller that cannot distinguish "we probed and the credential was rejected" from "we could not probe" will eventually treat the second as the first — which is the failure mode #460 exists to prevent, and which this repo has now hit nine times in other tools.

   Two concrete cases I would add to the acceptance criteria's test list:

   • The probe binary is absent from the guest. Must classify indeterminate, not healthy and not transport failure.
   • The diagnostic metadata is absent for a provider that declares no capability. Unsupported capability is an explicit declaration, never inferred as healthy already covers this in prose; a test should pin it, because the natural implementation is a falsy check that passes through to the success branch.

   One question

   successStatus: [200] — does a 204 or a 200 with an unexpected body count as healthy? The models endpoint returning 200 with an empty or malformed payload is the shape that would most plausibly fool a status-only check, and provider response failure is already in the taxonomy. Worth saying whether v1 validates the body shape at all, or deliberately does not.

   AI-assisted (OpenCode). Advisory — requires human review.