Skip to content

feat(review): place the boundary-trust, state-scope, no-lock and converge-on-rerun principle parts in the reviewer, coupling and loop-lane #12147

Description

@kyle-sexton

Parent #7589 decides that pstack's principles are placed "as reasoned sentences in the phase-skill step that applies them" (D58, Q174, Q175). The owner approved this 2026-10-15 (poteto-review decision rows 58 and 105). The wording rule the owner accepted (Q175) is that each placed part states the reason and the test that shows it applies, rather than a bare rule, and names the standard or discipline it comes from.

This slice places these parts:

  • 2b and 2g (boundary-discipline), in review:code-reviewer: inside a validated boundary, trust the types, with no re-validation and no deep nil checks; the test question is "Is this data crossing a system boundary right now?" The code-reviewer agent (plugins/review/agents/code-reviewer.md:73-80) has the pure-function question at :77 but not the boundary one. The implement side landed at plugins/implementation/skills/implement/SKILL.md:52.
  • 14e (minimize-reader-load), in the reviewer's mutable-state item: shrink state scope, preferring returns over mutation, locals over fields, and fields over globals. Architecture's scan has it at plugins/architecture/skills/improve/research/deepening/scan-briefing.md:41; the reviewer does not.
  • 20e (separate-before-serializing-shared-state), quoted as written in /coupling:reduce rung 1 (plugins/coupling/skills/reduce/reference/remediations.md:38-47): "Instructions and conventions are not concurrency control."
  • 12e (make-operations-idempotent), in the loop-lane contract: every step that writes to the forge converges when rerun. docs/contracts/loop-lane/README.md covers restart adoption (:949) and duplicate-escalation suppression (:518), but has no general rule.

Done when

  • plugins/review/agents/code-reviewer.md's code-quality list gains one item: re-validation or deep nil checks on data that crossed no system boundary, with the question "Is this data crossing a system boundary right now?" and the reason (checks inside a typed boundary add noise and false safety).
  • The reviewer's mutable-state item names the scope order (returns over mutation, locals over fields over globals), with the reason that a reader must track every place that can change the value.
  • plugins/coupling/skills/reduce/reference/remediations.md rung 1 carries the sentence "Instructions and conventions are not concurrency control." with one line of reason. When that file has plugin copies listed in scripts/shared-copies.txt, the copies match.
  • docs/contracts/loop-lane/README.md states that every step that writes to the forge (a label, comment, branch or PR) must leave the same state when rerun after a stop at any point, and that it reads current forge state before writing. It gives the reason: lanes restart and are re-dispatched. When that file has plugin copies listed in scripts/shared-copies.txt, the copies match.
  • .changes/review/*.md and .changes/coupling/*.md fragments with bump: patch describe the additions, plus one for each plugin whose loop-lane copy changed.
  • The PR's CI is green.

Out of scope

Parent: #7589

Activity

  1. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    status: readyTriaged, unblocked, and fully specified; eligible to pick up.
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Nov 1, 2026
  2. added
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    on Nov 1, 2026
  3. github-actions commented on Nov 1, 2026

    @github-actions
    Contributor
    No description provided.
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

    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.priority: mediumReal value, no hard deadline; normal backlog flow.status: readyTriaged, unblocked, and fully specified; eligible to pick up.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Fields

    No fields configured for Feature.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions