Skip to content

feat: add nix flake for reproducible builds#19

Open
Devin-Yeung wants to merge 8 commits into
reyamira:mainfrom
Devin-Yeung:nix-build
Open

feat: add nix flake for reproducible builds#19
Devin-Yeung wants to merge 8 commits into
reyamira:mainfrom
Devin-Yeung:nix-build

Conversation

@Devin-Yeung
Copy link
Copy Markdown

@Devin-Yeung Devin-Yeung commented Mar 15, 2026

It's seems that there's a test failing so I disable the test in the nix build

> thread 'tui::app::tests::picker_save_updates_agents_fetch_counters_for_newly_tracked_agents' (3196515) panicked at src/tui/app.rs:1420:9:
       > assertion `left == right` failed
       >   left: 0
       >  right: 2

But the binary looks fine for me

@Devin-Yeung
Copy link
Copy Markdown
Author

Devin-Yeung commented Mar 15, 2026

Should we add CI for nix building? (It usually takes longer time). If you want, I can do that :)

@arimxyer
Copy link
Copy Markdown
Collaborator

Thanks for the PR! This looks great overall — a few things to consider before merging:

  1. Scoped test skip — Rather than disabling all tests, could you scope it to just the failing test? Something like cargoTestExtraArgs = "--skip picker_save" would keep the rest of the test suite running in the Nix build.

  2. TLS certificates — The project uses rustls-tls-native-roots to load certs from the OS trust store. On NixOS or in a pure Nix environment, the built binary may not find certs at runtime. Worth verifying that the binary can actually make HTTPS calls (e.g. fetching from models.dev). If not, adding cacert to buildInputs or wrapping the binary with makeWrapper + SSL_CERT_FILE might be needed.

  3. Nix CI — Yes please! That would be great to have so the flake doesn't silently break on future commits. A simple nix build + nix flake check workflow would be enough.

Let me know if you have questions on any of these!

@Devin-Yeung
Copy link
Copy Markdown
Author

Devin-Yeung commented Mar 22, 2026

Done, and I also installed shell completion for nix build. Now nix users can use shell completion out of the box :)

Copy link
Copy Markdown
Collaborator

@arimxyer arimxyer left a comment

Choose a reason for hiding this comment

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

Hey @Devin-Yeung, thanks for putting this together — the flake looks solid! Crane is a great choice, and the source filtering, SSL cert wrapping, and shell completions are all well done.

A few suggestions:

  1. Pin determinate-nix-action to a release tag — Currently @main, which could break or pose a supply-chain risk if the branch moves unexpectedly. They publish a v3 major tag, so DeterminateSystems/determinate-nix-action@v3 would match the pinning convention used in the rest of the CI (actions/checkout@v6, etc.).

  2. Consider adding a nix cache action — Without caching, every CI run rebuilds from scratch. Something like DeterminateSystems/magic-nix-cache-action would speed things up significantly.

  3. Optional: devShell output — Not blocking, but a devShell would let contributors nix develop into a working environment. Could be a follow-up.

  4. Optional: concurrency grouping on the workflow — Right now multiple runs can pile up on the same PR. A concurrency block with cancel-in-progress: true would keep things tidy.

Overall this is in great shape — mainly just the action pinning that I'd want addressed before merging. The rest are nice-to-haves. Thanks again!

@Devin-Yeung
Copy link
Copy Markdown
Author

  1. Optional: devShell output — Not blocking, but a devShell would let contributors nix develop into a working environment. Could be a follow-up.

I am not familiar with devShell, I use devenv for development. Therefore, I will leave this issue to the community contribution.

@Devin-Yeung Devin-Yeung requested a review from arimxyer March 27, 2026 16:32
@arimxyer arimxyer force-pushed the main branch 2 times, most recently from 79e58a5 to f811615 Compare March 31, 2026 17:31
@Devin-Yeung
Copy link
Copy Markdown
Author

Just sync with main branch. No additional changes :)

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.

2 participants