Skip to content

[Repo Assist] refactor: remove unintended this.standard mutation in getArgs()#120

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-getargs-no-side-effect-2026-04-01-1c804a517e376c9f
Draft

[Repo Assist] refactor: remove unintended this.standard mutation in getArgs()#120
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-getargs-no-side-effect-2026-04-01-1c804a517e376c9f

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Apr 1, 2026

🤖 This is an automated pull request from Repo Assist.

Problem

getArgs() contained an unintentional side effect: it called this.getStandard(document) and stored the result back into this.standard as a by-product of building the argument list.

// Before
getArgs(document, tmpFileName) {
    ...
    this.standard = this.getStandard(document);   // ← side effect
    if (this.standard) {
        args.push("--standard=" + this.standard);
    }
    ...
}

This is problematic because:

  • getArgs() reads like a pure builder (returns an array of CLI arguments) but silently mutates instance state.
  • this.standard is loaded by loadSettings() and read by getStandard(); having getArgs() overwrite it mid-flight makes the data flow harder to follow.
  • If call order ever changes (e.g., multi-format parallelism), the mutation could corrupt state between calls.

In practice the bug was harmless — loadSettings() is called before every format() invocation — but the pattern is misleading.

Fix

Use a local const so getArgs() computes the standard for the argument list without touching this.standard:

// After
const standard = this.getStandard(document);
if (standard) {
    args.push("--standard=" + standard);
}

this.standard is now exclusively managed by loadSettings().

Test Status

node --check extension.js — no syntax errors
npm run test:unit — 7/7 tests pass

Generated by Repo Assist

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

getArgs() called getStandard(document) and stored the result back to
this.standard as a side effect. This was unintentional: loadSettings()
is the only method that should update this.standard.

The mutation made getArgs() harder to reason about (a method whose name
implies it only builds an argument list also silently modifies state).
In practice the behaviour was harmless because loadSettings() is called
before every format() invocation, but the pattern could lead to
confusion or subtle bugs if the call order changes.

Use a local const instead so getArgs() remains a pure builder with no
observable side effects on the instance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Apr 2, 2026
Summarises all bug fixes that are ready to ship, drawn from the
open Repo Assist PRs (primarily #106, #114, #117, #120, #124 and
the earlier consolidated fixes).

The version number in package.json is intentionally not changed here
(that file requires maintainer action due to protected-file
restrictions). This PR exists so the maintainer can review the
intended release notes and bump the version when ready.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants