Skip to content

Make production-bound architecture review the standard escalation after related review failures #3070

Closed as not planned

Description

@EHotwagner

Final scoped lifecycle disposition — 2026-10-19

The current user-selected policy under #4701 retires this original recipe as not planned; it is not a completed implementation claim. Source PR4711 and selected109/020 distribution/installed joins are accepted at their recorded scope. The final issue comment records independent owners and preserved remainder. Original promise, owners, evidence and supersession history below remain intact. #4701/#4712 and original Unknown custody remain open.


Observed behavior

Two live roadmap repair clusters crossed the related-finding threshold. Implementer-led deep dives found nearby state and isolation defects, but fresh critics still found caller-selected immutable authority, historical live topology poisoning, vacuous shell assertions, fixture-only workflow proofs, missing artifact custody, unsafe retry semantics, and unobserved live repository settings.

Root cause

The current review protocol bounds repair rounds but does not define or enforce what a deep architecture review must cover. It permits implementation to resume from a self-authored subsystem checklist without a pre-implementation adversarial design review, production-entry-point mutation, live configuration binding, authority-input classification, or retry/recovery custody proof.

Acceptance

After two consecutive material findings that share a subsystem or authority boundary, freeze source edits and require a typed production-bound architecture-review packet. The packet states the governing invariant; maps actual entry points, API endpoints, live configuration, current and historical topology, state/cache boundaries, immutable writes, artifact custody, partial failure, retry, recovery and rollback; classifies every trusted input as derived, independently validated, caller assertion, fixture assumption or live configuration; and defines production-surface positive and inversion controls. A fresh architecture adversary must accept the packet before implementation resumes. A different fresh implementation critic reviews the resulting head. Any further related finding escalates to boundary redesign or formal modeling and records why the prior completeness claim failed. Default worker/driver/reviewer projections and the typed engine must refuse ordinary repair dispatch while the required packet/design verdict is absent, stale, self-authored, fixture-only, or not bound to the current finding cluster.

Verification

Exercise the actual review state machine and generated commands end to end. Mutate or omit each packet field, reuse an implementer or implementation-critic identity as design critic, bind a stale head/finding cluster, provide fixture-only evidence, omit live settings, and attempt another repair dispatch; every case must fail before source-authoring or critic-queue mutation. Prove a complete production-bound packet plus independent design pass permits exactly one implementation-resume transition and that a post-review related finding routes to redesign/formal-model escalation rather than another deep-dive loop. Forward-test the updated skill with independent agents on realistic coordination and release scenarios.

Class: hardening
Severity: High
Backlog reason: not-yet-actionable

Paths: .agents/skills/pnext-item/references/independent-review.md .claude/skills/pnext-item/references/independent-review.md .agents/skills/drive-board/references/host-loop.md .claude/skills/drive-board/references/host-loop.md docs/coordination/structured-decisions.md src/FS.GG.Coord.Core/Protocol.fs src/FS.GG.Coord.Core/Protocol.fsi src/FS.GG.Coord.Core/Review.fs src/FS.GG.Coord.Core/Review.fsi src/FS.GG.Coord.Cli.Lifecycle tests/FS.GG.Coord.Core.Tests tests/FS.GG.Coord.Cli.Lifecycle.Tests tests/coord-engine-e2e tests/skill-quality scripts/generate-projections registry/repos.lock work readiness

Activity

  1. EHotwagner commented on Aug 30, 2026

    @EHotwagner
    ContributorAuthor
    No description provided.
  2. moved this to Backlog in Coordinationon Aug 30, 2026
  3. EHotwagner commented on Aug 30, 2026

    @EHotwagner
    ContributorAuthor
    No description provided.
  4. moved this from Backlog to Blocked in Coordinationon Aug 30, 2026
  5. EHotwagner commented on Aug 30, 2026

    @EHotwagner
    ContributorAuthor

    {
    "schema": "fsgg.coord.route-decision/v2",
    "subject": "#3070",
    "revision": 1,
    "previousDigest": null,
    "scope": [
    "make production-bound architecture review the enforced standard escalation after two related material findings"
    ],
    "dependencies": [
    "#3068 must land the live repair-phase assertion authority before this protocol extension is authored"
    ],
    "touchSet": [
    ".agents/skills/pnext-item/references/independent-review.md",
    ".claude/skills/pnext-item/references/independent-review.md",
    ".agents/skills/drive-board/references/host-loop.md",
    ".claude/skills/drive-board/references/host-loop.md",
    "docs/coordination/structured-decisions.md",
    "src/FS.GG.Coord.Core/Protocol.fs",
    "src/FS.GG.Coord.Core/Protocol.fsi",
    "src/FS.GG.Coord.Core/Review.fs",
    "src/FS.GG.Coord.Core/Review.fsi",
    "src/FS.GG.Coord.Cli.Lifecycle",
    "tests/FS.GG.Coord.Core.Tests",
    "tests/FS.GG.Coord.Cli.Lifecycle.Tests",
    "tests/coord-engine-e2e",
    "tests/skill-quality",
    "scripts/generate-projections",
    "work/3070-production-bound-architecture-review",
    "readiness/3070-production-bound-architecture-review"
    ],
    "policyVersion": "structured-decisions/1",
    "route": "sdd-required",
    "agent": "process-host-7a31",
    "timestamp": "2026-08-30T10:00:13Z",
    "reasonCodes": [
    "cross-cutting-review-protocol",
    "immutable-authority",
    "production-bound-negative-controls",
    "default-skill-contract"
    ],
    "rationale": "This changes the default repair/review state machine, published worker and driver guidance, structured authority, and generated protocol facts. It therefore requires a Standard SDD package and independent design plus implementation review.",
    "sddWorkId": "3070-production-bound-architecture-review",
    "specHome": "work/3070-production-bound-architecture-review/spec.md",
    "requiredGates": [
    "implementationReady",
    "analyze",
    "verify",
    "ship"
    ],
    "digest": "7280ae41e449114fc46f6a218f69670beccda91e30d5b329a3f180ade027560c"
    }

  6. EHotwagner commented on Aug 30, 2026

    @EHotwagner
    ContributorAuthor

    48-hour observation checkpoint 1

    Window: 2026-08-30T08:05:30Z through 2026-09-01T08:05:30Z (still running).

    The production-bound architecture gate is being forward-tested on two live clusters before this issue changes the default protocol:

    • .github#3068 / PR Add live accountable repair assertion authority #3069: five implementer-led design revisions and an implementation-critic pass did not expose the delayed-visibility authority flaw. Architecture review plus a Quint shared-state model did. The unsafe revision-5 election race was witnessed in 1,260/10,000 sampled traces (12.60%). Revision 6 removed election/lease, but a hidden non-equivalent same-generation write still allowed irreversible wait entry before collision in 2,136/10,000 traces (21.36%). Production implementation remains frozen pending a real linearizable authority design.
    • FS.GG.SDD#944 / PR coord: lint decides touch-set usability with its OWN List.forall — the third copy #864 could not remove, and it needs the every/some distinction back #945: two ordinary implementation-critic rounds found test-custody and production-boundary failures. Three pre-implementation architecture rounds then found release-finalization, identity/access, recovery, and receipt-topology defects before any live settings, tag, release, or feed mutation. All earlier findings are closed in direction; the remaining design requires a version-scoped receipt DAG with unique active selection/tip and lost-response recovery for ledger writes themselves.

    Early process finding: “deep dive” needs a typed completeness claim and an independent production-bound adversary before source changes; an implementer-authored checklist alone repeatedly converged locally while leaving cross-boundary races. Formal modeling is valuable after the first architecture revision involving distributed visibility or irreversible writes, rather than only after another implementation failure.

    This is interim evidence, not the final adoption verdict. Continue collecting iteration counts, elapsed time, defects shifted before implementation, false positives, and residual escape rate through the end of the window.

  7. EHotwagner commented on Aug 30, 2026

    @EHotwagner
    ContributorAuthor

    Adoption decision — accepted during observation

    Decision time: 2026-08-30T13:20:00Z.

    The user accepted the default result of the observation and authorized immediate implementation. The 48-hour window through 2026-09-01T08:05:30Z continues as post-adoption monitoring; it is no longer a gate on adoption.

    The standard process will enforce:

    1. After two related material findings at one subsystem or authority boundary, freeze production source and external mutations.
    2. Reduce the problem to its smallest business invariant and identify the single irreversible boundary before proposing machinery.
    3. Require a typed production-bound packet covering actual entry points, API behavior, permissions/settings, stale/cache visibility, immutable writes, custody, partial failure, retry, recovery, migration, and explicit trust assumptions.
    4. Classify each input as derived, independently validated, caller assertion, fixture assumption, or live configuration.
    5. Require a fresh architecture adversary before implementation. The implementer and later implementation critic cannot supply that verdict.
    6. Introduce a formal shared-state model at the first design involving delayed visibility, concurrent writers, retries around irreversible writes, or distributed authority—not after another code-review failure.
    7. Require executable counterexamples for rejected designs and all declared invariants/witnesses for the selected design.
    8. Add a scope-reduction checkpoint whenever the proposed repair begins replacing adjacent protocols. Overdesign is rejected evidence, not accumulated implementation scope.
    9. Resume implementation only on an exact packet/model digest and independent architecture pass. A separate fresh critic then reviews the implemented head.
    10. A new related finding after architecture pass routes back to boundary redesign/formal analysis and records which completeness claim failed.

    Interim forward-test result: both active repair clusters shifted material defects before further production code/settings/tag/feed mutation. The gate is effective but expensive when introduced late; therefore the default must front-load invariant reduction, production API qualification, threat boundaries, and formal modeling.

    Implementation remains sequenced in .github#3070 behind .github#3068 so the typed lifecycle extension builds on the landed live repair authority rather than maintaining two conflicting review protocols.

  8. EHotwagner commented on Aug 30, 2026

    @EHotwagner
    ContributorAuthor

    External research checkpoint — review authority across moved revisions

    Observed production defect: after a pass on head 5c2faca, PR #3069 moved to
    402a898d. The live oracle correctly requested a fresh review, but the ledger
    kept the old PR-wide critic generation and refused the fresh critic. The current
    design conflates the long-lived finding chain, the immutable reviewed revision,
    and the critic seat.

    Primary-source comparison:

    Concurrency finding: GitHub documents that conditional requests for unsafe
    methods such as comment POST are unsupported unless an endpoint says otherwise.
    The present GET-then-comment sequence is therefore not a literal compare-and-swap.
    By contrast, Git fast-forward ref updates, Kubernetes resourceVersion, and
    expected stream revisions reject stale writers. Sources:
    https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api,
    https://docs.github.com/en/rest/git/refs,
    https://kubernetes.io/docs/reference/using-api/api-concepts/,
    https://docs.kurrent.io/server/v25.1/http-api/introduction.

    Formal results:

    • The current pass-plus-head-move path reaches dispatchSuccessor but cannot
      append the mandated fresh critic record. Unsafe witness seed 0x74921f51ce8e3dd.
    • Adding only a writer head check is insufficient: head H3 can replace H2 after
      the read and before the comment append. Unsafe witness seed 0x6caae504fcac15e1.
    • An open multi-reviewer epoch with visible pass && no visible changes-required
      is unsafe under delayed visibility: a hidden already-authored negative can arrive
      after acceptance. Unsafe witness seed 0x5e245e224d6e7e5d.
    • A closed roster is safe but loses liveness when one reviewer disappears and its
      state grows approximately exponentially with roster size.

    Decision for architecture revision 22:

    1. Separate the durable finding chain from an immutable, content-addressed review
      epoch and from its critic seat.
    2. Hash the full reviewable snapshot: head plus mutable reviewed authority,
      evidence and live-configuration digests; exclude review records to avoid
      recursive identity.
    3. Use one deterministic critic seat per epoch. A new snapshot creates a new
      epoch and admits an independently eligible fresh critic without succession.
      Succession only replaces a critic within the same epoch.
    4. Make old-epoch waits and records historical. A shared reducer must drive
      inspect, wait, record and acceptance. Perform pre/post snapshot validation on
      the comment-backed implementation and never treat a moved-snapshot record as
      current authority.
    5. Do not adopt the one-case hot patch, open multi-reviewer authority, native
      GitHub-only authority, or a new transactional service in this repair.
    6. Keep a separately scoped Git-ref fast-forward CAS authority-stream prototype as
      the next escalation if comment-backed pre/post validation still produces a
      related race. Comments/check runs would then become projections, not locks.

    This is a post-adoption observation result and is now the default design direction
    for the active experiment. Source remains frozen until the revised composed model
    and a fresh architecture verdict pass.

  9. EHotwagner commented on Oct 19, 2026

    @EHotwagner
    ContributorAuthor

    The specified after-two-findings edit freeze, typed mandatory packet, fresh design adversary plus different implementation critic, and engine-enforced ordinary-dispatch refusal are deliberately declined for current routine work. Close this specific implementation recipe as not planned; it was not implemented merely because substantive review remains.

    The selected replacement requires coherent investigation of recurring failures across real callers, authority/state/cleanup boundaries, one independent exact-source judgement and actual causal qualification. Architecture uses its accepted design and adds specialists only for a distinct justified contribution. The original August30 adoption, revision22 review-epoch research and #3068/SDD944 acceptance remain citable with their real owners. This changes the default ceremony; it neither cancels an independently selected technical repair nor claims its model/API complete. Explicitly selected heavyweight/product contracts and effect-specific original custody retain their own requirements; no hypothetical packet/API is delivered by this retirement.

    Evidence boundary: source PR4711; actual coherent109 and Creator020; selected public fresh/retained instruction and runtime acceptance remains durably owned by #4712. PR4715 is normally delivered at4fadcce5004e30d122e41bac2bca6c404edbd902/tree41b683bf067acb3adb722297fc7e1c4216f901ea with exact review/required CI/coherence accepted; amended37 native continuation remains pending. Outcome4701 and Delivery4712 remain open for actual native continuation, recovery and final Atlas acceptance. All439 inherited and46 later Unknown originals remain distinct and unreleased.

  10. moved this from Blocked to Done in Coordinationon Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions