Skip to content

Use the same engineering checks during planning, implementation, and PR review #8240

Description

@jyaunches

Problem

The most important engineering questions often arrive after a pull request has opened and CI has run. That creates avoidable review churn when scope, existing architecture, failure paths, security boundaries, or test evidence could have been examined while the issue was planned and the code was written.

NemoClaw has strong specialist skills and review signals, but no general workflow owns issue planning or issue implementation. Some contributor and maintainer skills also repeat codebase details or review criteria that can drift from current source and from the PR Review Advisor.

Goal

Use the same compact engineering checks during planning, implementation, and PR review.

Current code, tests, workflows, and active AGENTS.md files remain the authority for repository rules and implementation architecture. Shared skill resources capture only durable questions that apply across lifecycle stages. The PR Review Advisor independently verifies those questions after a PR opens; it complements the contributor and maintainer workflows rather than replacing or duplicating them.

Proposed workflow

flowchart LR
  P["Plan issue"] --> I["Implement issue"] --> C["Create PR"] --> R["PR Review Advisor"]

  X["Shared change checks"] --> P
  X --> I
  X --> R

  S["Shared security rubric"] --> P
  S --> I
  S --> R

  A["Current code, tests,<br/>workflows, and AGENTS.md"] --> P
  A --> I
  A --> R
Loading

Shared inputs

Current implementation discovery

Planning and implementation begin with the current checkout. Agents identify the code that owns the behavior, nearby tests, active repository guidance, existing extension points, and related open or merged work. Issues, PRs, history, and documentation explain intent and prior decisions; they do not override current implementation behavior.

Change checks

A short, stage-neutral checklist asks:

  • What accepted outcome and current consumer require this change?
  • What current code owns the behavior, and can that owner be changed directly?
  • Would the change create a second source of truth or duplicate an existing structure?
  • Which state, error, ordering, concurrency, default, recovery, and bypass paths matter?
  • Can code or configuration be removed, or can an existing or native mechanism be used?
  • What is the shortest stable test that proves the behavior?
  • Does a real system boundary or checked-in risk mapping require deeper or E2E evidence?
  • Which active issues, PRs, or recent changes overlap or conflict?

Security rubric

One canonical, stage-neutral resource owns the security categories, NemoClaw trust-boundary questions, and expected evidence. Planning identifies applicable risks and controls. Implementation applies them and adds negative evidence. The procedural security-review skill and the PR Review Advisor independently evaluate the completed diff against the same rubric.

Workflow responsibilities

Workflow Responsibility
Issue planning Discover current ownership and related work, clarify the outcome, divide work into independently valuable capability slices, define observable acceptance examples, identify risks and dependencies, and make the test plan explicit.
Issue implementation Confirm accepted scope, rediscover current code and tests, apply shared checks before editing, implement the smallest valuable slice, validate it, and self-review the result.
Create PR Publish validated implementation evidence through the trusted template, contributor compliance gates, push, CI monitoring, and review follow-up.
Maintainer PR workflows Select, compare, salvage, secure, and qualify PRs using the shared checks while retaining their specific gate and decision procedures.
PR Review Advisor Independently inspect the trusted full diff and current repository context, then produce evidence-backed findings through its sandbox, stages, risk plan, ledger, schema, artifacts, and publisher.

Skill suite alignment

  • Add nemoclaw-contributor-plan-issue for issue planning and capability slicing.
  • Add nemoclaw-contributor-implement-issue as the general implementation owner.
  • Keep narrow specialists only for durable, non-default procedures such as dependency migration, contributor onboarding, security review, PR comparison, E2E, and release operations.
  • Make create-PR a publication handoff that consumes implementation evidence instead of re-owning design and test selection.
  • Fold component-specific implementation guidance into the general workflow plus the current package code, tests, and AGENTS.md guidance.
  • Make maintainer orchestrators compose canonical workflows rather than copy their rules.
  • Keep dynamic skill loading disabled in the PR Review Advisor. Load shared resources explicitly from its trusted checkout.
  • Keep Advisor-only mechanics in the Advisor: untrusted-input isolation, deterministic risk and E2E mapping, stage protocol, finding and terminology ledgers, schema validation, artifact handling, and publication.

Design rules

  • One durable rule has one canonical owner.
  • Shared resources remain concise, stage-neutral, and free of codebase inventories.
  • Current repository sources own paths, commands, registrations, test mappings, workflow selectors, and architecture.
  • Workflow skills own the actions appropriate to their moment in delivery.
  • Specialist skills augment the selected workflow; they do not compete with generic lifecycle routing.
  • Missing trusted shared resources fail visibly instead of activating a duplicated fallback.
  • GitHub, Project, publication, review, approval, and merge writes still require explicit authorization.

Acceptance criteria

  • Planning, implementation, and PR publication requests route to distinct workflow owners.
  • Issue planning reports related work, current structures to extend, independently valuable capability slices, observable acceptance examples, risks, and a clear test plan.
  • Issue implementation applies current source discovery plus shared change and security checks before editing and records focused validation evidence.
  • Create-PR remains a thin publication and follow-up workflow.
  • Manual security review and the PR Review Advisor use the same category names, meanings, and evidence expectations.
  • Maintainer PR workflows consume shared checks without treating Advisor output as merge authority.
  • The Advisor loads shared resources from its trusted revision, keeps dynamic skills disabled, and retains its independent review machinery.
  • Redundant component and review guidance is folded into its canonical owner.
  • Positive, negative, ambiguous, and clean-context routing tests protect the lifecycle boundaries.
  • No shared resource maintains implementation details already owned by current code, tests, workflows, or AGENTS.md.

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

    area: project-managementTaxonomy, triage, workflow, roadmap, or project process

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions