#1872 fix: drop the legacy dot and repair dangling metafile paths - #1881
Conversation
📝 WalkthroughWalkthroughInstaller dot metadata removes stale source references, updates current sync and preserve paths, removes the legacy Hyprland configuration, and adds validation that declared non-glob source paths exist. ChangesDot metadata maintenance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/python/check_dots.py`:
- Around line 104-108: Update the declared-path validation loop in
declared_paths(table) to resolve each string candidate relative to the resolved
source_root and reject it unless it remains inside that source-root directory,
including rejecting absolute paths and .. traversal. Perform this containment
check before the has_glob(relative) exemption, while preserving the existing
missing-path validation for allowed candidates.
- Around line 22-25: Update declared_paths to normalize only string and list
values; for any other paths value, return it wrapped in a one-element list
instead of passing it to list(). This lets the existing validator report invalid
scalar types such as integers and booleans without raising TypeError.
In `@tests/README.md`:
- Line 25: Update the test_dots.sh README description to state that filesystem
existence validation applies only to non-glob source paths, while preserving the
existing guarantees for installer metafile parsing, required keys, known
actions, source directories, and Grimblast’s fixed official source.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 276709e8-5902-40da-8b4a-7f3ff31308b7
📒 Files selected for processing (9)
CHANGELOG.mdScripts/dots/fish.tomlScripts/dots/hyprland-legacy.tomlScripts/dots/hyprland.tomlScripts/dots/swaync.tomlScripts/dots/waybar.tomlScripts/dots/zsh.tomltests/README.mdtests/python/check_dots.py
💤 Files with no reviewable changes (2)
- Scripts/dots/hyprland-legacy.toml
- Scripts/dots/hyprland.toml
| for relative in declared_paths(table): | ||
| if not isinstance(relative, str): | ||
| fail(f"{where} declares a path as {type(relative).__name__}, expected a string") | ||
| elif not has_glob(relative) and not (REPO_ROOT / source_root / relative).exists(): | ||
| fail(f"{where} points at a missing path {relative!r}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Constrain declared paths to source_root.
Only source_root is checked with inside_repo; an absolute path or a path containing .. can escape that directory and pass whenever the resulting filesystem entry exists. Resolve each candidate and require it to remain beneath the resolved source root before applying the glob exemption.
🤖 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 `@tests/python/check_dots.py` around lines 104 - 108, Update the declared-path
validation loop in declared_paths(table) to resolve each string candidate
relative to the resolved source_root and reject it unless it remains inside that
source-root directory, including rejecting absolute paths and .. traversal.
Perform this containment check before the has_glob(relative) exemption, while
preserving the existing missing-path validation for allowed candidates.
There was a problem hiding this comment.
Fixed. Each declared path is resolved against the resolved source root and has to stay under it, checked before the glob exemption. ../../etc/passwd and /etc/passwd are both rejected now.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Current relevant implementation ---'
sed -n '15,125p' tests/python/check_dots.py
echo
echo '--- Working-tree diff for the validation file ---'
git diff -- tests/python/check_dots.pyLength of output: 4583
@RAprogramm, I can’t verify the fix in the current PR revision: tests/python/check_dots.py still lacks a resolved-path containment check, and there is no diff for that file. It continues to check only path existence after the glob exemption.
Please push or update the commit containing the change, then retry so I can confirm and mark this resolved.
This chat is LLM-generated and may be inaccurate.
2 participants
Pull Request
Description
Fixes #1872. Both calls in that thread are yours: drop the legacy dot, and the
four remaining ones were an oversight.
Legacy dot removed
Scripts/dots/hyprland-legacy.tomldeployed eleven.conffiles that the Luarelease deleted. Selecting it produced an empty deployment, not a legacy
configuration. The file is gone, and the stale
conflictsentry naming it isgone from
hyprland.toml.Four dangling paths repaired
fish.tomlcompletionsConfigs/.config/fishhas no such directoryswaync.tomlhyde/wallbash/scripts/swaync.dcolswaync.sh, which is the file that exists — the hook is a script, not a templatewaybar.tomlscriptsConfigs/.local/share/waybarhas no such directoryzsh.tomlconf.d/bindsconf.d/binds.zshThe check that would have caught them
tests/python/check_dots.pyvalidatedsource_rootbut not the paths underit. It now resolves every declared path, skipping patterns since those are
expanded at deploy time. Against
devas it stands the check reports allthirteen:
Type of change
Removing the legacy dot is a removal rather than a breaking change: it
deployed nothing, so nobody depended on it working.
Checklist
Testing
sh tests/run.sh— 8 cases, 0 failed. The metafile case reports 24 files afterthe legacy one is gone, all clean.
The new assertion was checked from both sides: it reports the thirteen paths
above against
dev, and is silent on this branch.Summary by CodeRabbit
Removed
Improvements
Tests
Documentation