feat(steward): encode fail-closed provisioning invariant#950
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (23)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new session reload-directive platform-runtime operation (bringing the total to 22) to surface harness-appropriate reload commands like /reload-plugins on Claude, replacing the previous session restart requirement. It also implements a shared fail-closed provisioning-write guard (reject_unknown_provisioning_field) to reject unknown configuration fields per ADR-009, and optimizes the multi-version PYTHONPATH pollution check by marking superseded directories with an .orphaned_at deferral marker rather than deleting them immediately. Review feedback identifies a compatibility issue with datetime.now(UTC) on Python versions prior to 3.11, recommending timezone.utc instead, and suggests registering dependency modules in sys.modules before dynamically loading test modules to ensure correct import resolution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…sion, not first-hit find_installed_manifest_path now reads every existing candidate version and returns the max version tuple, with ties resolving to the earlier candidate. Empty or all-unresolvable still returns None (fail-closed). The existing version-tuple helper is reused. Co-Authored-By: Claude <noreply@anthropic.com>
…a orphan-GC deferral marker cmd_preflight now marks non-newest version dirs with the orphan-GC .orphaned_at deferral marker after a pollution-triggered regen, and _detect_multi_version_pollution excludes marker-carrying dirs. A second consecutive preflight then sees one live version dir per bundle and reports fresh, closing the regenerated-every-run loop. Removal defers to the existing 7-day orphan GC (no delete-live-dir race). Co-Authored-By: Claude <noreply@anthropic.com>
New enumeration doc records every provisioning write + status site across manage-config, marshall-steward, and generate_executor with a fix-or-justify disposition against the two failure shapes (silent-success write, vacuous-safe verdict). Names cmd_system retention set as the one silent-success write to fix and save_config as the shared choke point deliverable 4 encodes the invariant at. Co-Authored-By: Claude <noreply@anthropic.com>
…via shared guard Adds reject_unknown_provisioning_field to _config_core as the single fail-closed encoding: a provisioning write of an unknown/out-of-schema field refuses (status error, error_type unknown_field) instead of silently persisting. cmd_system retention set now routes through it (fixing the concrete silent-success write) and cmd_project set is generalized onto the same seam. A structural test enumerates the provisioning-write handlers and asserts each fails closed on an unknown field. ADR-009 promoted Proposed to Accepted and broadened to name the generalized write-side invariant and the shared guard. Co-Authored-By: Claude <noreply@anthropic.com>
… seam (22nd op) Adds the session reload-directive operation to the Runtime ABC, router, and both targets: Claude resolves /reload-plugins plus the monitor caveat, OpenCode returns a no-op whose alternative is a full session restart. Bumps the operation count to 22 across SKILL.md, contract.md, and the runtime module docstrings, and swaps the blanket-restart remediation in marshall-steward upgrade-flow / SKILL for a call to the new seam. Reconciles the four convergent WHY surfaces (sync-plugin-cache, variant_emitter, ext-point-dynamic-level-executor, marshall-steward) from restart-only to reload-directive wording while keeping the session-pinned-registry rationale unchanged. Per-target resolver tests plus router-dispatch tests for the 22nd operation. Co-Authored-By: Claude <noreply@anthropic.com>
…n_reload_directive D5 added session_reload_directive as the 22nd abstract method on the Runtime ABC; this base-class test file was under-scoped out of D5's declared test files. Bump the abstract-method count assertions 21->22, add the method to ALL_ABSTRACT_METHODS and the _ConcreteRuntime test double, and validate its TOON output alongside the other operations. Co-Authored-By: Claude <noreply@anthropic.com>
…anchor drift Two self-review corrections in PLAN-13's own deliverables: - marshall-steward/SKILL.md: the "Re-Run Remediation Pass" step (d) still cross-referenced the pre-rename heading "Session Restart Required After Executor / Agent Changes" and said "surface the session-restart warning". D5 renamed that section to "Session Reload Directive After Executor / Agent Changes" with reload-not-restart body; update the cross-reference and wording to match, consistent with the already-updated sibling references. - manage-config/standards/provisioning-fail-closed-audit.md: the D4-anchor description wrongly named `_config_core.save_config` as the choke point where the fail-closed provisioning-write invariant is encoded. The shipped code instead encodes it via the standalone `reject_unknown_provisioning_field` guard, invoked by each provisioning-write call site (cmd_system retention set, cmd_project set) before save_config, which itself stays unvalidated. Corrected the prose, the save_config table row, and the Summary to describe the real architecture. Co-Authored-By: Claude <noreply@anthropic.com>
…ail-closed audit The D4 guard insertion (reject_unknown_provisioning_field) in _cmd_system_plan.py shifted several function line ranges without re-numbering the audit's "Surveyed sites" table. Re-verified every line-number citation across _cmd_system_plan.py, _config_core.py, _cmd_sync_defaults.py, _config_defaults.py, and determine_mode.py against the current file contents and corrected the 9 that had drifted (cmd_system retention set, cmd_project set/get/pr-decision, the three unknown-verb terminators, merge_build_map, get_build_map, ext_defaults_set/set_default, and the determine_mode/cmd_mode split now cited separately since cmd_mode moved away from determine_mode). Co-Authored-By: Claude <noreply@anthropic.com>
…nerate_executor Extract the shared _live_version_dirs(bundle_dir) helper from _detect_multi_version_pollution and _mark_superseded_version_dirs — both built the identical live-version-dir filter (is_dir, non-dot, has skills/, no .orphaned_at marker). The per-call-site OSError handling stays where it is. Co-Authored-By: Claude <noreply@anthropic.com>
15b09da to
e99c2a6
Compare
1 participant
Summary
PLAN-13 (epic
plan-optimization, workstream WS-03) operationalizes ADR-009's fail-closed provisioning principle across the steward / manage-config provisioning surface, following up on WS-03's three shipped point-fixes (PLAN-07/08/09). It fixes a fail-silent-fresh root cause in the executor manifest resolver, stops the executor being reported as regenerated on every preflight run, documents a provisioning fail-closed audit sweep, adds a shared fail-closed provisioning guard backed by a structural test and a promoted ADR, and gives the platform-runtime layer a per-target session reload-directive seam that the steward upgrade flow now surfaces instead of a blanket restart instruction.Changes
generate_executor.py'sfind_installed_manifest_path()now collects all existing manifest candidates and resolves the highestversioninstead of returning on the first hit, closing the fail-silent-fresh defect where a stale cache-root manifest could permanently shadow the authoritative clone-root manifest.executor_action: regeneratedon every run; superseded plugin-cache version directories are pruned on regen via the orphan-GC deferral marker mechanism.marketplace/bundles/plan-marshall/skills/manage-config/standards/provisioning-fail-closed-audit.mdrecords the enumerated audit sweep over every manage-config / marshall-steward / executor-provisioning write+status site for silent-success and vacuous-default failure shapes, with a fix-or-justify disposition per site.reject_unknown_provisioning_fieldfail-closed guard is added to_config_core.py/_cmd_system_plan.py, covered bytest_provisioning_fail_closed_invariant.py, and ADR-009 (doc/adr/009-Status_reporting_fails_closed_with_an_explicit_unknown_state.adoc) is promoted from Proposed to Accepted.platform-runtimegains asession reload-directive-style operation (runtime_base.py,claude_runtime.py/_claude_runtime_impl.py,opencode_runtime.py,platform_runtime.py) that resolves the harness-appropriate post-upgrade action per target; the stewardupgradeflow and the "Session Restart Required After Executor / Agent Changes" section inmarshall-steward/SKILL.mdandupgrade-flow.mdnow surface that resolved directive instead of a blanket restart instruction.ext-point-dynamic-level-executor.mdandvariant_emitter.pywere adjusted for the same reconciliation.Test Plan
python3 .plan/execute-script.py plan-marshall:build-pyproject:pyproject_build run --command-args "verify")Related Issues
None
Generated by plan-finalize skill