Skip to content

add ws diagnose util to detect cyclic dependencies and multiple variants of the same dependency#17

Merged
iurimatias merged 1 commit intomasterfrom
docs/nix-diagnose
Mar 20, 2026
Merged

add ws diagnose util to detect cyclic dependencies and multiple variants of the same dependency#17
iurimatias merged 1 commit intomasterfrom
docs/nix-diagnose

Conversation

@iurimatias
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings March 20, 2026 15:39
@iurimatias iurimatias merged commit 90a75a4 into master Mar 20, 2026
2 of 3 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new ws nix-diagnose command to help troubleshoot Nix flake dependency problems (cycles and duplicate versions) and documents the new workflow in the repo docs.

Changes:

  • Add ws nix-diagnose <repo> command to analyze flake.lock for circular deps and duplicate dependency revisions.
  • Register nix-diagnose in ws help/dispatch and expand documentation in README.md and CLAUDE.md.
  • Add logos-nix to the workspace repo registry.

Reviewed changes

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

File Description
scripts/ws Adds cmd_nix_diagnose, registers the new command, and extends the REPOS registry.
README.md Documents the new ws nix-diagnose command and its intended output/fixes.
CLAUDE.md Updates the command quick-reference with the new diagnostics command.

Comment on lines +2054 to +2055
select(.value | type == "string") |
.value as $child |
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The jq extraction treats dependency edges as only those inputs whose value is a string, but in this repo’s flake.lock many inputs are arrays (e.g. ["nixpkgs"] or longer paths). This will drop most edges and can incorrectly report “No circular dependencies found”. Update the jq to also handle array inputs (e.g. map arrays to their first element / target node key) so the dependency graph is complete.

Suggested change
select(.value | type == "string") |
.value as $child |
.value as $input |
(if ($input | type) == "string" then
$input
elif ($input | type) == "array" and ($input | length) > 0 then
$input[0]
else
empty
end) as $child |

Copilot uses AI. Check for mistakes.
Comment on lines +2115 to +2122
.nodes.root.inputs | to_entries[] |
select(.value | type == "string") | .key
' "$lock_file" 2>/dev/null | head -1 || true)
# Visit all repos reachable from root inputs
local root_inputs
root_inputs=$(jq -r '
.nodes.root.inputs | to_entries[] |
select(.value | type == "string") | .value
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Root inputs are filtered to type == "string", but root inputs can also be arrays in flake.lock (path-form). That can make root_inputs incomplete/empty and skip traversal entirely. Consider normalizing input refs so both string and array forms are traversed.

Suggested change
.nodes.root.inputs | to_entries[] |
select(.value | type == "string") | .key
' "$lock_file" 2>/dev/null | head -1 || true)
# Visit all repos reachable from root inputs
local root_inputs
root_inputs=$(jq -r '
.nodes.root.inputs | to_entries[] |
select(.value | type == "string") | .value
.nodes.root.inputs
| to_entries[]
| .value
| if type == "string" then .
elif type == "array" and length > 0 then .[0]
else empty
end
' "$lock_file" 2>/dev/null | head -1 || true)
# Visit all repos reachable from root inputs
local root_inputs
root_inputs=$(jq -r '
.nodes.root.inputs
| to_entries[]
| .value
| if type == "string" then .
elif type == "array" and length > 0 then .[0]
else empty
end

Copilot uses AI. Check for mistakes.
Comment on lines +2192 to +2193
select(.value | type == "string") |
$parent + " " + .value + " " + .key
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The adjacency list for _find_path only includes inputs whose value is a string. In flake.lock, many inputs are arrays, so paths to conflicting nodes will often show as “(unreachable)” even when reachable. Normalize .value so both string and array forms contribute edges.

Suggested change
select(.value | type == "string") |
$parent + " " + .value + " " + .key
.key as $input_name |
.value as $v |
(if ($v | type) == "string" then [$v]
elif ($v | type) == "array" then $v
else [] end)[] as $child |
$parent + " " + $child + " " + $input_name

Copilot uses AI. Check for mistakes.
Comment on lines +2029 to +2030
local input_name dir_name
input_name=$(get_field "$entry" 1)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

input_name is computed but never used in this command. Remove it (and the assignment) or use it in output so the function doesn’t carry dead variables.

Suggested change
local input_name dir_name
input_name=$(get_field "$entry" 1)
local dir_name

Copilot uses AI. Check for mistakes.
Comment on lines +2113 to +2117
local start_repo
start_repo=$(jq -r '
.nodes.root.inputs | to_entries[] |
select(.value | type == "string") | .key
' "$lock_file" 2>/dev/null | head -1 || true)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

start_repo is computed but never used. This looks like leftover scaffolding; consider removing it to avoid confusion and keep the cycle detection logic focused.

Suggested change
local start_repo
start_repo=$(jq -r '
.nodes.root.inputs | to_entries[] |
select(.value | type == "string") | .key
' "$lock_file" 2>/dev/null | head -1 || true)

Copilot uses AI. Check for mistakes.
Comment on lines +2084 to +2091
local deps_str
deps_str=$(echo "$graph_data" | grep "^${node}|" | head -1 | cut -d'|' -f2 || true)
local dep
for dep in ${deps_str//,/ }; do
[[ -z "$dep" ]] && continue
local state
state=$(grep "^${dep} " "$_dfs_state_file" 2>/dev/null | tail -1 | cut -d' ' -f2 || true)
if [[ "$state" == "visiting" ]]; then
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

These grep patterns interpolate repo/node names directly into a regex (e.g. ^${node}|). Repo IDs / node keys can contain characters like . that are meaningful in regex, which can cause incorrect matches. Prefer fixed-string matching (grep -F) or escape the variables before using them in regex patterns.

Copilot uses AI. Check for mistakes.
Comment on lines +2047 to +2064
repo_edges=$(jq -r '
.nodes as $nodes |
[ $nodes | to_entries[] |
select(.value.inputs != null) |
.key as $parent |
( $nodes[$parent].original.repo // ($parent | sub("_[0-9]+$"; "")) ) as $src |
.value.inputs | to_entries[] |
select(.value | type == "string") |
.value as $child |
( $nodes[$child].original.repo // ($child | sub("_[0-9]+$"; "")) ) as $dst |
select($src != $dst) |
$src + " " + $dst
] | unique[]
' "$lock_file" 2>/dev/null || true)

if [[ -z "$repo_edges" ]]; then
echo -e " ${GREEN}No circular dependencies found${NC}"
else
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

In the cycle section, jq errors are suppressed (2>/dev/null || true), which can turn parse failures into an empty edge list and a misleading “No circular dependencies found”. Consider checking jq’s exit status and reporting a clear parse/error message instead of silently continuing.

Copilot uses AI. Check for mistakes.
# has_flake: "yes" or "no"

REPOS=(
# Shared Nix infrastructure
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

logos-nix is added to the REPOS registry, but it isn’t present in .gitmodules yet. With this change, ws init will try to git submodule add it in user workspaces, which creates local .gitmodules/index changes. Consider adding the submodule entry to .gitmodules in this PR (or documenting why it should remain auto-added).

Suggested change
# Shared Nix infrastructure
# Shared Nix infrastructure
# NOTE: logos-nix is intentionally auto-added as a submodule by `ws init`
# and is not predeclared in .gitmodules; this keeps it workspace-local.

Copilot uses AI. Check for mistakes.
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