Define closure criteria for architecture-sensitive reviews #435

Description

@taras

Goal

Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

Problem

The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

That produced a counterexample-driven loop:

  1. one concrete bypass was reported;
  2. the implementation closed it;
  3. the next review introduced an adjacent invariant or adversarial capability;
  4. tests and durable documentation expanded again; and
  5. there was no explicit condition under which the review was complete.

The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

Desired process

Identify architecture-sensitive work

Treat a change as architecture-sensitive when it affects any of:

  • authority or trusted-host ownership;
  • durable identity, admission, or publication;
  • replay and retained-history compatibility;
  • concurrency, cancellation, resource lifetime, or teardown;
  • failure precedence or exact identity;
  • loaded-package-copy composition; or
  • a public middleware contract.

Before implementation begins, such work receives one settled architecture record covering:

  • trusted and untrusted actors;
  • adversarial capabilities that are in scope;
  • explicit non-goals and process-level attacks that are out of scope;
  • authority, identity, persistence, and lifecycle ownership;
  • success, refusal, failure, cancellation, and teardown behavior;
  • live, partial-replay, and completed-replay behavior;
  • loaded-copy behavior where applicable;
  • the acceptance matrix and discriminating evidence; and
  • the condition under which architecture review passes.

Define when a finding blocks

A new finding blocks the current PR only when all of these are true:

  1. It reproduces against the exact reviewed head.
  2. It violates a named, already-settled invariant.
  3. It uses an in-scope input or supported public surface.
  4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
  5. Its correction belongs within the PR's existing purpose.

Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

Classify every finding as one of:

  • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
  • correctness blocker — violates an explicit public contract on a supported path; or
  • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

Freeze decisions and close review

  • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
  • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
  • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
  • The Implementor reports against that checklist at one exact head.
  • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

Keep prerequisite PRs reviewable

Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

Deliverables

  • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
  • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
  • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
  • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
  • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
  • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

Acceptance criteria

  • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
  • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
  • Review findings must name the settled invariant they violate and include an exact-head reproducer.
  • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
  • Two correction rounds trigger a consolidated final ruling and pass condition.
  • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
  • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

Non-goals

  • Declaring unusual inputs irrelevant merely because they are unusual.
  • Allowing green CI to substitute for architecture evidence.
  • Preventing a demonstrated high-severity defect from blocking late in review.
  • Turning every ordinary PR into a formal threat-modeling exercise.

Activity

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

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
       blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
      }
      } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
      })();
      (function(){
      try {
      var __m = "github.com";
      var __re = new RegExp('^' + "github\\.com" + '
      
      Skip to content

      Define closure criteria for architecture-sensitive reviews #435

      Description

      @taras

      Goal

      Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

      Problem

      The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

      That produced a counterexample-driven loop:

      1. one concrete bypass was reported;
      2. the implementation closed it;
      3. the next review introduced an adjacent invariant or adversarial capability;
      4. tests and durable documentation expanded again; and
      5. there was no explicit condition under which the review was complete.

      The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

      Desired process

      Identify architecture-sensitive work

      Treat a change as architecture-sensitive when it affects any of:

      • authority or trusted-host ownership;
      • durable identity, admission, or publication;
      • replay and retained-history compatibility;
      • concurrency, cancellation, resource lifetime, or teardown;
      • failure precedence or exact identity;
      • loaded-package-copy composition; or
      • a public middleware contract.

      Before implementation begins, such work receives one settled architecture record covering:

      • trusted and untrusted actors;
      • adversarial capabilities that are in scope;
      • explicit non-goals and process-level attacks that are out of scope;
      • authority, identity, persistence, and lifecycle ownership;
      • success, refusal, failure, cancellation, and teardown behavior;
      • live, partial-replay, and completed-replay behavior;
      • loaded-copy behavior where applicable;
      • the acceptance matrix and discriminating evidence; and
      • the condition under which architecture review passes.

      Define when a finding blocks

      A new finding blocks the current PR only when all of these are true:

      1. It reproduces against the exact reviewed head.
      2. It violates a named, already-settled invariant.
      3. It uses an in-scope input or supported public surface.
      4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
      5. Its correction belongs within the PR's existing purpose.

      Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

      Classify every finding as one of:

      • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
      • correctness blocker — violates an explicit public contract on a supported path; or
      • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

      Freeze decisions and close review

      • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
      • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
      • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
      • The Implementor reports against that checklist at one exact head.
      • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

      Keep prerequisite PRs reviewable

      Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

      Deliverables

      • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
      • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
      • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
      • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
      • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
      • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

      Acceptance criteria

      • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
      • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
      • Review findings must name the settled invariant they violate and include an exact-head reproducer.
      • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
      • Two correction rounds trigger a consolidated final ruling and pass condition.
      • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
      • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

      Non-goals

      • Declaring unusual inputs irrelevant merely because they are unusual.
      • Allowing green CI to substitute for architecture evidence.
      • Preventing a demonstrated high-severity defect from blocking late in review.
      • Turning every ordinary PR into a formal threat-modeling exercise.

      Activity

      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

        enhancementNew feature or request

        Projects

        No projects

          Milestone

          No milestone

          Relationships

          None yet

          Development

          No branches or pull requests

          Issue actions

          , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
          Skip to content

          Define closure criteria for architecture-sensitive reviews #435

          Description

          @taras

          Goal

          Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

          Problem

          The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

          That produced a counterexample-driven loop:

          1. one concrete bypass was reported;
          2. the implementation closed it;
          3. the next review introduced an adjacent invariant or adversarial capability;
          4. tests and durable documentation expanded again; and
          5. there was no explicit condition under which the review was complete.

          The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

          Desired process

          Identify architecture-sensitive work

          Treat a change as architecture-sensitive when it affects any of:

          • authority or trusted-host ownership;
          • durable identity, admission, or publication;
          • replay and retained-history compatibility;
          • concurrency, cancellation, resource lifetime, or teardown;
          • failure precedence or exact identity;
          • loaded-package-copy composition; or
          • a public middleware contract.

          Before implementation begins, such work receives one settled architecture record covering:

          • trusted and untrusted actors;
          • adversarial capabilities that are in scope;
          • explicit non-goals and process-level attacks that are out of scope;
          • authority, identity, persistence, and lifecycle ownership;
          • success, refusal, failure, cancellation, and teardown behavior;
          • live, partial-replay, and completed-replay behavior;
          • loaded-copy behavior where applicable;
          • the acceptance matrix and discriminating evidence; and
          • the condition under which architecture review passes.

          Define when a finding blocks

          A new finding blocks the current PR only when all of these are true:

          1. It reproduces against the exact reviewed head.
          2. It violates a named, already-settled invariant.
          3. It uses an in-scope input or supported public surface.
          4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
          5. Its correction belongs within the PR's existing purpose.

          Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

          Classify every finding as one of:

          • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
          • correctness blocker — violates an explicit public contract on a supported path; or
          • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

          Freeze decisions and close review

          • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
          • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
          • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
          • The Implementor reports against that checklist at one exact head.
          • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

          Keep prerequisite PRs reviewable

          Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

          Deliverables

          • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
          • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
          • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
          • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
          • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
          • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

          Acceptance criteria

          • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
          • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
          • Review findings must name the settled invariant they violate and include an exact-head reproducer.
          • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
          • Two correction rounds trigger a consolidated final ruling and pass condition.
          • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
          • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

          Non-goals

          • Declaring unusual inputs irrelevant merely because they are unusual.
          • Allowing green CI to substitute for architecture evidence.
          • Preventing a demonstrated high-severity defect from blocking late in review.
          • Turning every ordinary PR into a formal threat-modeling exercise.

          Activity

          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

            enhancementNew feature or request

            Projects

            No projects

              Milestone

              No milestone

              Relationships

              None yet

              Development

              No branches or pull requests

              Issue actions

              , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
              Skip to content

              Define closure criteria for architecture-sensitive reviews #435

              Description

              @taras

              Goal

              Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

              Problem

              The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

              That produced a counterexample-driven loop:

              1. one concrete bypass was reported;
              2. the implementation closed it;
              3. the next review introduced an adjacent invariant or adversarial capability;
              4. tests and durable documentation expanded again; and
              5. there was no explicit condition under which the review was complete.

              The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

              Desired process

              Identify architecture-sensitive work

              Treat a change as architecture-sensitive when it affects any of:

              • authority or trusted-host ownership;
              • durable identity, admission, or publication;
              • replay and retained-history compatibility;
              • concurrency, cancellation, resource lifetime, or teardown;
              • failure precedence or exact identity;
              • loaded-package-copy composition; or
              • a public middleware contract.

              Before implementation begins, such work receives one settled architecture record covering:

              • trusted and untrusted actors;
              • adversarial capabilities that are in scope;
              • explicit non-goals and process-level attacks that are out of scope;
              • authority, identity, persistence, and lifecycle ownership;
              • success, refusal, failure, cancellation, and teardown behavior;
              • live, partial-replay, and completed-replay behavior;
              • loaded-copy behavior where applicable;
              • the acceptance matrix and discriminating evidence; and
              • the condition under which architecture review passes.

              Define when a finding blocks

              A new finding blocks the current PR only when all of these are true:

              1. It reproduces against the exact reviewed head.
              2. It violates a named, already-settled invariant.
              3. It uses an in-scope input or supported public surface.
              4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
              5. Its correction belongs within the PR's existing purpose.

              Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

              Classify every finding as one of:

              • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
              • correctness blocker — violates an explicit public contract on a supported path; or
              • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

              Freeze decisions and close review

              • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
              • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
              • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
              • The Implementor reports against that checklist at one exact head.
              • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

              Keep prerequisite PRs reviewable

              Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

              Deliverables

              • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
              • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
              • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
              • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
              • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
              • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

              Acceptance criteria

              • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
              • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
              • Review findings must name the settled invariant they violate and include an exact-head reproducer.
              • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
              • Two correction rounds trigger a consolidated final ruling and pass condition.
              • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
              • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

              Non-goals

              • Declaring unusual inputs irrelevant merely because they are unusual.
              • Allowing green CI to substitute for architecture evidence.
              • Preventing a demonstrated high-severity defect from blocking late in review.
              • Turning every ordinary PR into a formal threat-modeling exercise.

              Activity

              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

                enhancementNew feature or request

                Projects

                No projects

                  Milestone

                  No milestone

                  Relationships

                  None yet

                  Development

                  No branches or pull requests

                  Issue actions

                  , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
                  Skip to content

                  Define closure criteria for architecture-sensitive reviews #435

                  Description

                  @taras

                  Goal

                  Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

                  Problem

                  The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

                  That produced a counterexample-driven loop:

                  1. one concrete bypass was reported;
                  2. the implementation closed it;
                  3. the next review introduced an adjacent invariant or adversarial capability;
                  4. tests and durable documentation expanded again; and
                  5. there was no explicit condition under which the review was complete.

                  The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

                  Desired process

                  Identify architecture-sensitive work

                  Treat a change as architecture-sensitive when it affects any of:

                  • authority or trusted-host ownership;
                  • durable identity, admission, or publication;
                  • replay and retained-history compatibility;
                  • concurrency, cancellation, resource lifetime, or teardown;
                  • failure precedence or exact identity;
                  • loaded-package-copy composition; or
                  • a public middleware contract.

                  Before implementation begins, such work receives one settled architecture record covering:

                  • trusted and untrusted actors;
                  • adversarial capabilities that are in scope;
                  • explicit non-goals and process-level attacks that are out of scope;
                  • authority, identity, persistence, and lifecycle ownership;
                  • success, refusal, failure, cancellation, and teardown behavior;
                  • live, partial-replay, and completed-replay behavior;
                  • loaded-copy behavior where applicable;
                  • the acceptance matrix and discriminating evidence; and
                  • the condition under which architecture review passes.

                  Define when a finding blocks

                  A new finding blocks the current PR only when all of these are true:

                  1. It reproduces against the exact reviewed head.
                  2. It violates a named, already-settled invariant.
                  3. It uses an in-scope input or supported public surface.
                  4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
                  5. Its correction belongs within the PR's existing purpose.

                  Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

                  Classify every finding as one of:

                  • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
                  • correctness blocker — violates an explicit public contract on a supported path; or
                  • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

                  Freeze decisions and close review

                  • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
                  • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
                  • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
                  • The Implementor reports against that checklist at one exact head.
                  • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

                  Keep prerequisite PRs reviewable

                  Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

                  Deliverables

                  • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
                  • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
                  • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
                  • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
                  • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
                  • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

                  Acceptance criteria

                  • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
                  • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
                  • Review findings must name the settled invariant they violate and include an exact-head reproducer.
                  • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
                  • Two correction rounds trigger a consolidated final ruling and pass condition.
                  • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
                  • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

                  Non-goals

                  • Declaring unusual inputs irrelevant merely because they are unusual.
                  • Allowing green CI to substitute for architecture evidence.
                  • Preventing a demonstrated high-severity defect from blocking late in review.
                  • Turning every ordinary PR into a formal threat-modeling exercise.

                  Activity

                  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

                    enhancementNew feature or request

                    Projects

                    No projects

                      Milestone

                      No milestone

                      Relationships

                      None yet

                      Development

                      No branches or pull requests

                      Issue actions

                      , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
                      Skip to content

                      Define closure criteria for architecture-sensitive reviews #435

                      Description

                      @taras

                      Goal

                      Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

                      Problem

                      The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

                      That produced a counterexample-driven loop:

                      1. one concrete bypass was reported;
                      2. the implementation closed it;
                      3. the next review introduced an adjacent invariant or adversarial capability;
                      4. tests and durable documentation expanded again; and
                      5. there was no explicit condition under which the review was complete.

                      The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

                      Desired process

                      Identify architecture-sensitive work

                      Treat a change as architecture-sensitive when it affects any of:

                      • authority or trusted-host ownership;
                      • durable identity, admission, or publication;
                      • replay and retained-history compatibility;
                      • concurrency, cancellation, resource lifetime, or teardown;
                      • failure precedence or exact identity;
                      • loaded-package-copy composition; or
                      • a public middleware contract.

                      Before implementation begins, such work receives one settled architecture record covering:

                      • trusted and untrusted actors;
                      • adversarial capabilities that are in scope;
                      • explicit non-goals and process-level attacks that are out of scope;
                      • authority, identity, persistence, and lifecycle ownership;
                      • success, refusal, failure, cancellation, and teardown behavior;
                      • live, partial-replay, and completed-replay behavior;
                      • loaded-copy behavior where applicable;
                      • the acceptance matrix and discriminating evidence; and
                      • the condition under which architecture review passes.

                      Define when a finding blocks

                      A new finding blocks the current PR only when all of these are true:

                      1. It reproduces against the exact reviewed head.
                      2. It violates a named, already-settled invariant.
                      3. It uses an in-scope input or supported public surface.
                      4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
                      5. Its correction belongs within the PR's existing purpose.

                      Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

                      Classify every finding as one of:

                      • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
                      • correctness blocker — violates an explicit public contract on a supported path; or
                      • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

                      Freeze decisions and close review

                      • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
                      • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
                      • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
                      • The Implementor reports against that checklist at one exact head.
                      • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

                      Keep prerequisite PRs reviewable

                      Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

                      Deliverables

                      • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
                      • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
                      • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
                      • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
                      • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
                      • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

                      Acceptance criteria

                      • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
                      • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
                      • Review findings must name the settled invariant they violate and include an exact-head reproducer.
                      • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
                      • Two correction rounds trigger a consolidated final ruling and pass condition.
                      • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
                      • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

                      Non-goals

                      • Declaring unusual inputs irrelevant merely because they are unusual.
                      • Allowing green CI to substitute for architecture evidence.
                      • Preventing a demonstrated high-severity defect from blocking late in review.
                      • Turning every ordinary PR into a formal threat-modeling exercise.

                      Activity

                      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

                        enhancementNew feature or request

                        Projects

                        No projects

                          Milestone

                          No milestone

                          Relationships

                          None yet

                          Development

                          No branches or pull requests

                          Issue actions

                          , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
                          Skip to content

                          Define closure criteria for architecture-sensitive reviews #435

                          Description

                          @taras

                          Goal

                          Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

                          Problem

                          The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

                          That produced a counterexample-driven loop:

                          1. one concrete bypass was reported;
                          2. the implementation closed it;
                          3. the next review introduced an adjacent invariant or adversarial capability;
                          4. tests and durable documentation expanded again; and
                          5. there was no explicit condition under which the review was complete.

                          The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

                          Desired process

                          Identify architecture-sensitive work

                          Treat a change as architecture-sensitive when it affects any of:

                          • authority or trusted-host ownership;
                          • durable identity, admission, or publication;
                          • replay and retained-history compatibility;
                          • concurrency, cancellation, resource lifetime, or teardown;
                          • failure precedence or exact identity;
                          • loaded-package-copy composition; or
                          • a public middleware contract.

                          Before implementation begins, such work receives one settled architecture record covering:

                          • trusted and untrusted actors;
                          • adversarial capabilities that are in scope;
                          • explicit non-goals and process-level attacks that are out of scope;
                          • authority, identity, persistence, and lifecycle ownership;
                          • success, refusal, failure, cancellation, and teardown behavior;
                          • live, partial-replay, and completed-replay behavior;
                          • loaded-copy behavior where applicable;
                          • the acceptance matrix and discriminating evidence; and
                          • the condition under which architecture review passes.

                          Define when a finding blocks

                          A new finding blocks the current PR only when all of these are true:

                          1. It reproduces against the exact reviewed head.
                          2. It violates a named, already-settled invariant.
                          3. It uses an in-scope input or supported public surface.
                          4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
                          5. Its correction belongs within the PR's existing purpose.

                          Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

                          Classify every finding as one of:

                          • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
                          • correctness blocker — violates an explicit public contract on a supported path; or
                          • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

                          Freeze decisions and close review

                          • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
                          • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
                          • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
                          • The Implementor reports against that checklist at one exact head.
                          • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

                          Keep prerequisite PRs reviewable

                          Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

                          Deliverables

                          • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
                          • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
                          • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
                          • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
                          • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
                          • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

                          Acceptance criteria

                          • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
                          • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
                          • Review findings must name the settled invariant they violate and include an exact-head reproducer.
                          • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
                          • Two correction rounds trigger a consolidated final ruling and pass condition.
                          • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
                          • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

                          Non-goals

                          • Declaring unusual inputs irrelevant merely because they are unusual.
                          • Allowing green CI to substitute for architecture evidence.
                          • Preventing a demonstrated high-severity defect from blocking late in review.
                          • Turning every ordinary PR into a formal threat-modeling exercise.

                          Activity

                          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

                            enhancementNew feature or request

                            Projects

                            No projects

                              Milestone

                              No milestone

                              Relationships

                              None yet

                              Development

                              No branches or pull requests

                              Issue actions

                              , 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
                              Skip to content

                              Define closure criteria for architecture-sensitive reviews #435

                              Description

                              @taras

                              Goal

                              Improve the architecture and implementation review process so foundational PRs converge without weakening real authority, durability, replay, or lifecycle guarantees.

                              Problem

                              The review sequence around #432 and #433 found real defects, but it also exposed a process failure: the threat model, blocker criteria, and final acceptance boundary were still evolving while implementation was underway.

                              That produced a counterexample-driven loop:

                              1. one concrete bypass was reported;
                              2. the implementation closed it;
                              3. the next review introduced an adjacent invariant or adversarial capability;
                              4. tests and durable documentation expanded again; and
                              5. there was no explicit condition under which the review was complete.

                              The result was a large number of individually reasonable correction rounds without a stable definition of “done.” This makes delivery hard to predict, mixes architecture discovery with implementation review, and risks treating increasingly remote edge cases as equivalent to authority or durability defects.

                              Desired process

                              Identify architecture-sensitive work

                              Treat a change as architecture-sensitive when it affects any of:

                              • authority or trusted-host ownership;
                              • durable identity, admission, or publication;
                              • replay and retained-history compatibility;
                              • concurrency, cancellation, resource lifetime, or teardown;
                              • failure precedence or exact identity;
                              • loaded-package-copy composition; or
                              • a public middleware contract.

                              Before implementation begins, such work receives one settled architecture record covering:

                              • trusted and untrusted actors;
                              • adversarial capabilities that are in scope;
                              • explicit non-goals and process-level attacks that are out of scope;
                              • authority, identity, persistence, and lifecycle ownership;
                              • success, refusal, failure, cancellation, and teardown behavior;
                              • live, partial-replay, and completed-replay behavior;
                              • loaded-copy behavior where applicable;
                              • the acceptance matrix and discriminating evidence; and
                              • the condition under which architecture review passes.

                              Define when a finding blocks

                              A new finding blocks the current PR only when all of these are true:

                              1. It reproduces against the exact reviewed head.
                              2. It violates a named, already-settled invariant.
                              3. It uses an in-scope input or supported public surface.
                              4. It has an observable authority, durability, replay, lifecycle, or public-completion consequence.
                              5. Its correction belongs within the PR's existing purpose.

                              Findings that fail one of these conditions become follow-up issues rather than silently extending the current architecture contract.

                              Classify every finding as one of:

                              • architecture blocker — changes what executes, what history or identity is accepted, what durable state is published, or what authoritative outcome wins;
                              • correctness blocker — violates an explicit public contract on a supported path; or
                              • hardening follow-up — extends the threat model, requires arbitrary process sabotage, affects only non-authoritative diagnostics, or is outside the PR's purpose.

                              Freeze decisions and close review

                              • Once an architecture ruling is accepted, later reviews check the implementation against it rather than introducing adjacent invariants implicitly.
                              • A new material invariant requires an explicit architecture amendment with its scope and consequence stated. Unless it exposes a high-severity violation in the current boundary, deliver it separately.
                              • After two correction rounds, the Architect supplies one consolidated ruling: final invariants, adversary, required evidence, non-goals, and the exact pass condition.
                              • The Implementor reports against that checklist at one exact head.
                              • The final reviewer either identifies a checklist violation with a reproducer or passes the PR. Documentation cleanup and speculative hardening do not restart architecture review.

                              Keep prerequisite PRs reviewable

                              Prefer one authority or lifecycle boundary per prerequisite PR when main can remain coherent. Do not combine independent invariants merely because they were discovered during the same review loop.

                              Deliverables

                              • Update AGENTS.md with the blocker rubric, finding classifications, and review-closure rule.
                              • Update .agents/architect.md to require the threat model, acceptance matrix, explicit non-goals, and consolidated ruling after repeated correction rounds.
                              • Update .agents/planner.md so implementor handoffs carry the frozen architecture checklist and route newly discovered architecture decisions back instead of embedding them in implementation prompts.
                              • Update .agents/implementor.md so correction reports map changes and tests to the frozen checklist and clearly identify any requested scope expansion.
                              • Add or update a reusable architecture-sensitive review/handoff template. Keep ordinary PRs lightweight.
                              • Reconcile .github/pull_request_template.md only if a small conditional section helps identify architecture-sensitive work without burdening routine changes.

                              Acceptance criteria

                              • The documented process distinguishes architecture blockers, correctness blockers, and follow-up hardening.
                              • Every architecture-sensitive handoff has an explicit in-scope/out-of-scope threat model and a finite acceptance matrix before implementation.
                              • Review findings must name the settled invariant they violate and include an exact-head reproducer.
                              • The process specifies when a new finding becomes a separate issue instead of extending the active PR.
                              • Two correction rounds trigger a consolidated final ruling and pass condition.
                              • A worked retrospective against the 🔒 Make canonical core authoritative for execution #432/🔒 Make canonical core authoritative for document expansion #433 sequence demonstrates where the new process would have consolidated decisions earlier and which findings would still have blocked.
                              • The process does not cap necessary investigation or weaken authority, durability, replay, or lifecycle guarantees; it makes their boundaries explicit and reviewable.

                              Non-goals

                              • Declaring unusual inputs irrelevant merely because they are unusual.
                              • Allowing green CI to substitute for architecture evidence.
                              • Preventing a demonstrated high-severity defect from blocking late in review.
                              • Turning every ordinary PR into a formal threat-modeling exercise.

                              Activity

                              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

                                enhancementNew feature or request

                                Projects

                                No projects

                                  Milestone

                                  No milestone

                                  Relationships

                                  None yet

                                  Development

                                  No branches or pull requests

                                  Issue actions