add support for repository variables (#798)

[tonpatel]

This PR adds support for repository-level variables that can be used outside of a specific deployment environment.

Fixed find function and unit test from this PR and resubmitted for approval. All credit to @primetheus!

cc: @decyjphr, @primetheus

[Copilot]

Pull Request Overview

Adds first-class support for repository-level variables, including plugin registration, implementation, and associated tests.

• Introduces a new Variables plugin for listing, creating, updating, and deleting repo variables
• Registers the plugin in lib/settings.js
• Adds unit tests and fixtures for the variables functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
test/unit/lib/plugins/variables.test.js	New tests for the Variables plugin’s sync logic
test/fixtures/variables-config.yml	Fixture for variables configuration
lib/settings.js	Registers variables plugin alongside others
lib/plugins/variables.js	Implements the Variables class and its methods

[decyjphr]

👍🏾

[Copilot]

Pull Request Overview

Adds support for repository-level variables so they can be managed outside of specific deployment environments.
Key changes include:

• New variables plugin with CRUD operations against the GitHub Actions Variables API.
• Unit tests for the variables plugin behaviour.
• Registration of the variables plugin in settings.js.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
test/unit/lib/plugins/variables.test.js	New tests mocking GET/POST/DELETE flows for variables
lib/settings.js	Added variables plugin entry in Settings.PLUGINS
lib/plugins/variables.js	Implementation of the Variables diffable plugin

Comments suppressed due to low confidence (1)

test/unit/lib/plugins/variables.test.js:40

• [nitpick] Iterating over ['variables'] is unclear—if this is simulating multiple pages, rename the array or remove the loop and mock calls directly for readability.

['variables'].forEach(() => {

[Copilot]

You’re using await and then chaining .then()/.catch(), which is redundant. Use try { await this.github.request(...) } catch (e) { this.logError(e) } for clearer async error handling.

Suggested change

	await this.github
	.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
	org: this.repo.owner,
	repo: this.repo.repo,
	variable_name: variable.name.toUpperCase(),
	value: variable.value.toString()
	})
	.then((res) => {
	return res
	})
	.catch((e) => {
	this.logError(e)
	})
	}
	} else {
	await this.github
	.request('POST /repos/:org/:repo/actions/variables', {
	org: this.repo.owner,
	repo: this.repo.repo,
	name: variable.name.toUpperCase(),
	value: variable.value.toString()
	})
	.then((res) => {
	return res
	})
	.catch((e) => {
	this.logError(e)
	})
	}
	}
	for (const variable of existingVariables) {
	await this.github
	.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
	org: this.repo.owner,
	repo: this.repo.repo,
	variable_name: variable.name.toUpperCase()
	})
	.then((res) => {
	return res
	})
	.catch((e) => {
	this.logError(e)
	})
	try {
	await this.github.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
	org: this.repo.owner,
	repo: this.repo.repo,
	variable_name: variable.name.toUpperCase(),
	value: variable.value.toString()
	})
	} catch (e) {
	this.logError(e)
	}
	}
	} else {
	try {
	await this.github.request('POST /repos/:org/:repo/actions/variables', {
	org: this.repo.owner,
	repo: this.repo.repo,
	name: variable.name.toUpperCase(),
	value: variable.value.toString()
	})
	} catch (e) {
	this.logError(e)
	}
	}
	}
	for (const variable of existingVariables) {
	try {
	await this.github.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
	org: this.repo.owner,
	repo: this.repo.repo,
	variable_name: variable.name.toUpperCase()
	})
	} catch (e) {
	this.logError(e)
	}

[Copilot]

[nitpick] In add(), you rely on the constructor having uppercased variable.name. Consider explicitly normalizing variable.name = variable.name.toUpperCase() before sending the request to make the behavior clear.

Suggested change

	this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
	this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
	variable.name = variable.name.toUpperCase()

[Copilot]

Comparing two arrays via JSON.stringify is fragile. Consider using a deep comparison helper like _.isEqual on sorted arrays for more robust change detection.

Suggested change

	const result =
	JSON.stringify(
	existing.sort((x1, x2) => {
	return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
	})
	) !==
	JSON.stringify(
	variables.sort((x1, x2) => {
	return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
	})
	)
	return result
	const sortedExisting = existing.sort((x1, x2) => {
	return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase());
	});
	const sortedVariables = variables.sort((x1, x2) => {
	return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase());
	});
	const result = !_.isEqual(sortedExisting, sortedVariables);
	return result;

[Copilot]

fillVariables expects an array but is called with an object here; this mismatch can lead to mocks returning unexpected shapes. Pass an array of variable objects or adjust the helper to accept both shapes.

Suggested change

	fillVariables({
	variables: []
	})
	fillVariables([])