chore(cleanup): non-blocking review nits deferred from #806 — doctor comment/detail accuracy, Blueprint modelId validation, docs geography default

[isadeks]

Component

CLI (cli/src/platform-doctor.ts), CDK (cdk/src/constructs/blueprint.ts, cdk/src/handlers/shared/bedrock-model-constants.ts), docs (DEVELOPER_GUIDE.md + mirror), agent diagnostics (agent/scripts/diagnostics/test_sdk_smoke.py)

Describe the feature

Clear the non-blocking findings deferred from the review of #806 (approved at 5640d879, merged with these outstanding). Nothing here blocks a deploy today: the items are comment accuracy, one asymmetric validation boundary, one diagnostic exit-code gap, and one operator-facing doc default. Line numbers are as of 5640d879 and will shift; the symbol names are the anchors.

One is more than cosmetic — item 1 is the only remaining path that writes an unvalidated model id to the RepoTable.

• 1. blueprint.ts writes agent.modelId to the RepoTable with no validation (cdk/src/constructs/blueprint.ts:319, and the onUpdate path at :426/:470). feat(cdk): switch to the global Bedrock geo, with doctor and --model validation #806 added assertModelIdUsable so repo onboard --model rejects bare ids, wrong geographies, ungranted models and doubled prefixes before the write. The Blueprint is a sanctioned co-writer of the same column and rejects none of them, so a Blueprint-pinned us.anthropic.claude-opus-5 on a global deploy still fails at turn 0 with AccessDenied naming no cause. The grant list and geography are both known at synth, so this can throw at synth. Symmetry with the CLI writer is the goal.
• 2. platform-doctor.ts carries two copies of a stale rationale comment (:441-447 in checkBedrockModel, :518-523 in checkBedrockInferenceProfile). Both explain the err.name-vs-err.message matching that now lives in classifyProbeFailure (which has its own copy of the same reasoning), so neither function does what its comment describes. Both also credit the idiom to checkGithubToken, which has no try/catch and no error matching at all (:280) — a maintainer sent there finds nothing. Delete both.
• 3. checkBedrockModel gives the same remedy for denied and absent (platform-doctor.ts:452-455). Both render "Enable model access in the Bedrock console" — drawing a conclusion about the MODEL from an error about the CALLER, the split that checkBedrockInferenceProfile and checkGrantedModelProfiles already make correctly. The status is right (warn on denial); only the detail conflates the two.
• 4. bedrock_granted_model_profiles states a half-false fact on a partially-updated stack (platform-doctor.ts:574-582). The guard is if (!bedrockModelIds || !geoRegion) — either output missing — but the detail says the stack "does not export BedrockModelIds and BedrockGeoRegion". Interpolate the actually-missing name(s).
• 5. cli/test/platform-doctor.test.ts:217 restates the geography literal (const GEOS = ['global', 'us-gov', …]) instead of importing BEDROCK_GEO_PREFIXES from cli/src/model-id.ts. The list itself now has a real forcing function (see "Already fixed" below); this restatement is the one copy left that nothing pins.
• 6. Stale, self-contradictory comment in cdk/test/constructs/blueprint.test.ts:429. The rationale "this blueprint declares no agent.modelId, so it legitimately removes model_id" is false against current behaviour — model_id is a non-asset override that is carried forward, not cleared — and contradicts the very next test (onUpdate does NOT remove model_id). The per-column assertions are correct; only the comment is wrong.
• 7. test_sdk_smoke.py prints PASS and exits 0 after an exception (agent/scripts/diagnostics/test_sdk_smoke.py:123). ok is derived from message counts only and ignores the errors list, so a run that streams a result and then raises reports success with the traceback printed below the verdict — the exact silent-success this diagnostic exists to rule out. Suggest ok = counts["assistant"] > 0 and counts["result"] > 0 and not errors, and print errors above the verdict.
• 8. inferenceProfileId(geoRegion, bareModelId) takes two interchangeable unvalidated strings (cdk/src/handlers/shared/bedrock-model-constants.ts:135). The one function that owns the bare/prefixed distinction cannot enforce it — a swapped or already-prefixed argument compiles and produces global.global.anthropic.…. All callers are correct today; a guard (or branded/validated inputs) would make that structural rather than reviewed.
• 9. DEVELOPER_GUIDE.md:125 says the geography default is us — "Geography via context bedrockGeoRegion (default us)" — in the section the file calls canonical. Every other artefact feat(cdk): switch to the global Bedrock geo, with doctor and --model validation #806 touched uses the precise form, e.g. the BedrockGeoRegion CfnOutput: "global in the shipped cdk.json, us if no context is supplied at all". An operator reading the row concludes their deploy grants us. profiles and pins us.<model> per repo; on the CLI path the new --model guard catches that, but via a Blueprint (item 1) it is written unvalidated. Use the CfnOutput wording and regenerate the Starlight mirror (developer-guide/Model-configuration.md).

Use case

Every item is drift-prevention on surfaces #806 just reworked: comments that now point at the wrong symbol are the mechanism by which the next maintainer reintroduces the bug that PR fixed, and items 1 and 7 are the same failure class the PR exists to eliminate — a value that cannot work being accepted silently and resurfacing as an unattributed turn-0 (or exit-0) failure.

Proposed solution

Items 2-6, 8 and 9 are comment/detail/test edits and can land as one cleanup PR. Item 1 (synth-time validation of agent.modelId in the Blueprint construct, with a test for each class assertModelIdUsable rejects) and item 7 are behaviour changes and are each worth their own commit within it.

Other information

• Raised during review of feat(cdk): switch to the global Bedrock geo, with doctor and --model validation #806 (aws-samples/sample-autonomous-cloud-coding-agents#806), which is approved and merged; deliberately kept out of that PR to keep it reviewable.
• Already fixed at 5640d879, do not redo: the approving review's nits 1-3 and 9 were addressed in that head and its list is stale on them — BEDROCK_GEO_PREFIXES now has a text-read parity test against the CDK literal (cli/test/model-id.test.ts, stays in sync with the CDK geography list); all four doctor probes route through classifyProbeFailure (no remaining inline regex copies); checkBedrockModel's three-way classification is pinned by it.each on Throttling/InternalServer/Timeout plus explicit denial and absent cases; and the workflows.ts allow-list doc block and bedrock-models.ts GEO_REGION_ENUM_VALUES comment were both rewritten to describe the derivation correctly.
• Related and already tracked: feat(orchestration): scope workflow model admission to the deployed geography #846 (scope workflow model admission to the deployed geography) — the runtime-admission instance of the same geography-blindness, not in scope here.
• Also raised in the feat(cdk): switch to the global Bedrock geo, with doctor and --model validation #806 description and not covered above: the platform default model id is still hand-copied across TypeScript and Python, though model-default-docs-parity.test.ts now pins the copies to each other.