Skip to content

ReviewAgent: make helper configuration inputs explicit #2408

Closed as not planned

Description

@WaelAbuSeada

Follow-up to #2402, as requested in review: #2402 (comment)

Several review-engine helpers depend on variables inherited through PowerShell dynamic scope. For example, Invoke-CopilotCli accepts only Prompt while relying on caller configuration for the executable, model, working directory, report path, and execution settings. Assert-Config has similar implicit inputs. This makes isolated testing and future refactoring fragile.

Scope:

  • Replace implicit configuration inputs with explicit, typed parameters or a narrowly scoped configuration/context object, starting with Invoke-CopilotCli and Assert-Config.
  • Update direct callers and audit related root/leaf execution helpers for the same pattern without introducing global state.
  • Preserve root/leaf model selection, exact report-file write permissions, child-process environment isolation, npm launch behavior, and existing failure/telemetry semantics.
  • Add focused tests that call helpers without pre-populated caller-scope variables, alongside existing root/leaf execution tests.

This is intentionally tracked separately from the correctness fixes in the PR to avoid a broad late-stage refactor.

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

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions