Skip to content

[nix-440] add determinate-nix-config option#266

Open
colemickens wants to merge 1 commit into
mainfrom
colemickens/nix-440-det-nix-config
Open

[nix-440] add determinate-nix-config option#266
colemickens wants to merge 1 commit into
mainfrom
colemickens/nix-440-det-nix-config

Conversation

@colemickens

@colemickens colemickens commented Jul 1, 2026

Copy link
Copy Markdown
Member
Description

NOTE: I would like feedback:

  1. There's no good way to know if a runner is ephemeral or expected to be ephemeral.
  2. But, technically writing out the nix.conf, etc, is similarly "stompy".
  3. Thus maybe this is the right set of tradeoffs for allowing stomping, and making sure the user has a way to know it happened.
Checklist
  • Tested changes against a test repository
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • (If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)

Summary by CodeRabbit

  • New Features

    • Added an optional configuration input for providing installer settings as JSON before installation begins.
    • The installer now writes this configuration ahead of setup, with validation and a warning if an existing file will be replaced.
  • Documentation

    • Improved the configuration section formatting in the README for easier reading, without changing any settings or defaults.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new optional action input, determinate-nix-config, that carries literal JSON written to /etc/determinate/config.json before Nix installation. Implements validation, existing-file overwrite warning, privileged write logic, and wiring into the install flow, alongside minor README table reformatting.

Changes

Determinate Nix Config Support

Layer / File(s) Summary
Input declaration and README
action.yml, README.md
Declares the new optional determinate-nix-config string input with JSON validation and overwrite notes; reformats the README configuration table without changing content.
Config field, constructor wiring, and write logic
src/index.ts
Adds a determinateNixConfig field, reads the new input in the constructor, and implements writeDeterminateNixConfig() to validate JSON, warn on existing file overwrite, ensure the target directory exists, and write via sudo when not root.
Install flow invocation
src/index.ts
Calls writeDeterminateNixConfig() from install() before the "Installing Nix" group begins.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Action as NixInstallerAction
  participant Config as writeDeterminateNixConfig()
  participant FS as /etc/determinate/config.json
  participant Installer as Nix Installer

  Action->>Action: read determinate-nix-config input
  Action->>Config: install() calls writeDeterminateNixConfig()
  Config->>Config: validate JSON (throw if invalid)
  Config->>FS: check for existing config.json (warn if present)
  Config->>FS: create /etc/determinate directory
  Config->>FS: write JSON via sudo or direct exec
  Config-->>Action: return
  Action->>Installer: fetch and run installer binary
Loading

Related PRs: None identified.

Suggested labels: enhancement, documentation

Suggested reviewers: None identified.

Poem:
A rabbit hops with JSON in paw,
To write a config, per new law,
Sudo whispers, root stands tall,
Warnings flag an overwrite call,
Then off to Nix, install and all! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding the determinate-nix-config option.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch colemickens/nix-440-det-nix-config

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 109-110: The documented default for the github-server-url input is
incorrect in the README and should match the action definition. Update the
github-server-url entry in the README so its default uses the same value
declared in action.yml, and verify the surrounding
github-token/github.server_url wording stays consistent with the action’s actual
input default.

In `@src/index.ts`:
- Around line 486-489: The writeDeterminateNixConfig method only checks
determinateNixConfig for presence, so it can still write Determinate-specific
config even when determinate is false. Update writeDeterminateNixConfig in the
Determinate installer flow to also guard on the determinate flag before writing
/etc/determinate/config.json, and make sure any caller paths that install
upstream Nix skip this config write when Determinate Nix was not selected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bfe10f7b-8ee6-4065-b2d8-9cd5d20976e7

📥 Commits

Reviewing files that changed from the base of the PR and between a7ad9c4 and f603008.

⛔ Files ignored due to path filters (1)
  • dist/index.js is excluded by !**/dist/**
📒 Files selected for processing (3)
  • README.md
  • action.yml
  • src/index.ts

Comment thread README.md
Comment on lines +109 to +110
| `github-token` | A [GitHub token] for making authenticated requests (which have a higher rate-limit quota than unauthenticated requests) | string | `${{ github.token }}` |
| `github-server-url` | The URL for the GitHub server, to use with the `github-token` token. Defaults to the current GitHub server, supporting GitHub Enterprise Server automatically. Only change this value if the provided `github-token` is for a different GitHub server than the current server. | string | `${{ github.server }}` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the documented default for github-server-url.

Line 110 shows ${{ github.server }}, but action.yml declares ${{ github.server_url }}. Copying the current README value into a workflow will not match the action’s actual default.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~109-~109: The official name of this software platform is spelled with a capital “H”.
Context: ...g | ${{ github.token }} ...

(GITHUB)


[uncategorized] ~110-~110: The official name of this software platform is spelled with a capital “H”.
Context: ...g | ${{ github.server }} ...

(GITHUB)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 109 - 110, The documented default for the
github-server-url input is incorrect in the README and should match the action
definition. Update the github-server-url entry in the README so its default uses
the same value declared in action.yml, and verify the surrounding
github-token/github.server_url wording stays consistent with the action’s actual
input default.

Comment thread src/index.ts
Comment on lines +486 to +489
async writeDeterminateNixConfig(): Promise<void> {
if (this.determinateNixConfig === null) {
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject determinate-nix-config when determinate is false.

This method only checks whether the input is present, so a workflow that installs upstream Nix can still write /etc/determinate/config.json. That leaves Determinate-specific state behind even though the run opted out of Determinate Nix.

Possible fix
  async writeDeterminateNixConfig(): Promise<void> {
    if (this.determinateNixConfig === null) {
      return;
    }
+   if (!this.determinate) {
+     throw new Error(
+       "`determinate-nix-config` requires `determinate: true`",
+     );
+   }

    const configDir = "/etc/determinate";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async writeDeterminateNixConfig(): Promise<void> {
if (this.determinateNixConfig === null) {
return;
}
async writeDeterminateNixConfig(): Promise<void> {
if (this.determinateNixConfig === null) {
return;
}
if (!this.determinate) {
throw new Error(
"`determinate-nix-config` requires `determinate: true`",
);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.ts` around lines 486 - 489, The writeDeterminateNixConfig method
only checks determinateNixConfig for presence, so it can still write
Determinate-specific config even when determinate is false. Update
writeDeterminateNixConfig in the Determinate installer flow to also guard on the
determinate flag before writing /etc/determinate/config.json, and make sure any
caller paths that install upstream Nix skip this config write when Determinate
Nix was not selected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant