[buluoray]

Problem / Motivation

Goal: Bring #28885 to release/0.12.0 so an insider build keeps a stored agent.kas_persist_mcp_sign_in: false when an overlay sets a non-object agent section.

On release/0.12.0, a non-object agent section (for example an overlay's {"agent": null}) is dropped by schema validation and read as unconfigured. #29148 brought #28630, which turns agent.kas_persist_mcp_sign_in on by default, so on 0.12 such a config makes KAS store OAuth tokens against the user's false.

Why it matters

#28630's default flip is already on release/0.12.0 (#29148). Without #28885, the next insider RC ships the default without the fail-closed guard that main has for it.

Not a goal

• No change beyond fix(config): treat a non-object agent section as malformed #28885; the pick is identical to its main commit.
• No version bump and no tag.
• No change to release/0.11.0.

What changed (motivation → approach → change)

git cherry-pick -x of main commit 5691cc8eb8b94336d5715cdb16c58cfa7145bdb9 (#28885) onto origin/release/0.12.0 @ 0eee5c6082. It applied clean. Every added and removed line matches main's commit.

The agent section joins the fail-closed validation paths in config/validation.py, so the loader records it as degraded and the KAS sign-in vault gate fails closed. config.md documents it; test_kas_mcp_secret_storage.py covers it.

Backwards compatibility

Compatible: a valid agent object loads as before; only a non-object agent section is now degraded instead of dropped.

Removes nothing: no key, path or record is removed.

Tests

Proof: not provable -- other: a pick identical to main; its proof ran on #28885

• test_kas_mcp_secret_storage.py, test_config_loader_sections.py, test_config_load_pipeline_field_defaults.py: 330 passed.
• flake8, isort, black (py310), mypy on config/validation.py, docs-lint, check_harness_parity, check_comment_history, check_brand_name, check_file_line_cap, check_subprocess_encoding: all pass.

Manual verification

N/A - unit coverage from #28885 applies unchanged.

Related Issues

Part of #28867. Follows #29148.

Sibling sweep

Siblings: none -- an exact pick of #28885; its own sweep ran on main.

Pattern harvest

Not generalizable: a release-branch pick of a fix already harvested on #28885.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md.

[github-actions]

UX Review (Opus 5.5) — ⏭️ skipped

Revision dd33abbaa4013c3f5f3185d409b1444e2f12b787 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Does not block merge.

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

Design-level review of dd33abbaa4013c3f5f3185d409b1444e2f12b787 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks. This is the exact pick of #28885 from main. It makes the agent section fail closed, which supplies the guard the default flip from #28630 (#29148) relies on, and only a non-object agent changes behaviour.

Notes

• With agent now a fail-closed section, a non-object agent from a live reload also stays flagged until restart, and its config appliers wait for that restart (config.md, the paragraph around line 2992). This is reversible and matches main, but the Compatible: line could say so.

[DESIGN-REVIEWED] dd33abb

Description read

[DESCRIPTION-READ] 46ed8dee3ff3f04cac05a4342af4df524549c674a95b761c11154e06b3fb45ef

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of dd33abbaa4013c3f5f3185d409b1444e2f12b787 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks. This is a clean fix pick. The doc line also adds hooks, a correction the description never mentions; it matches the frozenset already in the code, so it carries no harm of its own.

What this change ships

Inventory (4 items) — 3 justified

Intent: keep the operator's stored agent.kas_persist_mcp_sign_in: false when an overlay replaces agent with a non-object, so KAS keeps storing nothing — a FIX.

1. A non-object agent section (null, a list, a number) is kept and recorded as degraded instead of being silently dropped — justified
2. In that case MCP sign-in persistence stays off, through the existing "agent" in degraded check in kas_secret_storage.persistence_enabled — justified
3. A degraded agent section now defers that section's live config appliers until the document validates again (live._section_applier) — justified
4. The config spec's list of fail-closed sections now also names hooks, which was already in _FAIL_CLOSED_PATHS — undeclared, rides along

[FIRST-PRINCIPLES-REVIEWED] dd33abb

Description read

[DESCRIPTION-READ] 536e444551a5557fc981dac2bef04da20b09cac9e993e769fc13ca0c8abbc406

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Opus 5.5 Review — ✅ no blocking findings

Reviewed dd33abbaa4013c3f5f3185d409b1444e2f12b787 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] dd33abb

Verdict parsed from the review's SHA-scoped output markers for commit dd33abbaa4013c3f5f3185d409b1444e2f12b787.

False positive or not applicable? A repository writer can comment:
/ai-review override fable dd33abbaa4013c3f5f3185d409b1444e2f12b787 false-positive: <reason, 20 to 500 characters>

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of dd33abbaa4013c3f5f3185d409b1444e2f12b787 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] dd33abb

False positive or not applicable? A repository writer can comment:
/ai-review override gpt dd33abbaa4013c3f5f3185d409b1444e2f12b787 false-positive: <reason, 20 to 500 characters>

[bolichen97]

This looks good to me at dd33abb. Adding agent to _FAIL_CLOSED_PATHS keeps a non-object agent section (null, a list, a number) instead of repairing it to absent. _coerced_section then records agent as degraded and uses defaults for its settings, and kas_secret_storage.persistence_enabled already reads a degraded agent as off. So an overlay that replaces the base file's agent with null can no longer drop a stored false and turn on stored OAuth sign-ins. The parametrized test covers null and a list in config.local.json and a number in config.json, and the config spec lists the section.

One maintainer check remains before I can approve: the main CI (Fast Gate and the test, E2E and Integration jobs) has not run on this head; only the review lanes and policy checks have. Once it has run green, I will approve.