Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ jobs:
sudo mv "$file" "${file}.before-nix-darwin"
fi
done
- name: Wait for PR Head Ref
if: github.event_name == 'pull_request'
run: |
for attempt in 1 2 3 4 5 6; do
if git ls-remote --exit-code origin "refs/pull/${{ github.event.pull_request.number }}/head"; then
Comment on lines +69 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For GitHub Actions pull_request events, what does github.sha point to (head SHA vs merge SHA), and which refs are guaranteed to exist for raw.githubusercontent.com downloads?

💡 Result:

For GitHub Actions workflows triggered by pull_request events (excluding pull_request_target), github.sha points to the SHA of the merge commit (a temporary test merge of the PR head into the base branch). This is the last merge commit on the GITHUB_REF branch, which is refs/pull/<PR number>/merge [1][2][3]. The head SHA is available via github.event.pull_request.head.sha, and base SHA via github.event.pull_request.base.sha [4][5]. actions/checkout by default checks out this merge commit [6]. For raw.githubusercontent.com downloads, the following refs are guaranteed to exist for any repository (public or private, as long as the file/blob is accessible via the token/permissions): - refs/heads/<branch> for any branch (e.g., https://raw.githubusercontent.com/user/repo/refs/heads/main/file.txt) - refs/tags/<tag> for any tag - <commit-sha> directly (e.g., https://raw.githubusercontent.com/user/repo/abc123/file.txt) - refs/pull/<PR number>/head pointing to the head commit of the PR (even for fork PRs) [7][8][9] - refs/pull/<PR number>/merge pointing to the test merge commit (only if the PR is mergeable) [4][1][10] These refs allow serving raw files from branches, tags, commits, and PRs via raw.githubusercontent.com [11].

Citations:


🏁 Script executed:

cat -n .github/workflows/e2e.yml | sed -n '65,85p'

Repository: shunkakinoki/dotfiles

Length of output: 1071


🏁 Script executed:

# Check if there are any recent changes or comments about this workflow
git log --oneline -20 .github/workflows/e2e.yml 2>/dev/null | head -10

Repository: shunkakinoki/dotfiles

Length of output: 123


🏁 Script executed:

# Check if there are other uses of github.sha or github.event.pull_request.head.sha in the workflow
rg 'github\.(sha|event\.pull_request)' .github/workflows/e2e.yml

Repository: shunkakinoki/dotfiles

Length of output: 542


This step waits for the wrong ref—ensure the guarded SHA is actually available.

Line 70 checks for refs/pull/<num>/head, but line 82 downloads using ${{ github.sha }}, which on pull_request events points to the merge commit (refs/pull/<num>/merge). These are different refs. The merge ref exists only if the PR is mergeable, so the guard can pass while the SHA used by raw.githubusercontent.com is still unavailable.

Either wait for ${{ github.sha }} availability directly, or fetch using ${{ github.event.pull_request.head.sha }} consistently.

🤖 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 @.github/workflows/e2e.yml around lines 69 - 70, The guard loop is checking
the wrong ref (refs/pull/${{ github.event.pull_request.number }}/head) while the
download uses ${{ github.sha }}, which on pull_request events refers to the
merge commit; update the check to wait for the exact SHA you later use—either
test availability of ${{ github.sha }} directly or switch the download to use
${{ github.event.pull_request.head.sha }} so both the guard and fetch use the
same ref; ensure the loop command that currently references refs/pull/... is
replaced to verify the chosen SHA/ref (github.sha or
github.event.pull_request.head.sha) before proceeding to
raw.githubusercontent.com.

exit 0
fi

echo "PR head ref is not available yet; retrying in 10s (attempt ${attempt}/6)."
sleep 10
done

echo "PR head ref refs/pull/${{ github.event.pull_request.number }}/head is unavailable."
exit 1
- name: Run Install Command
run: |
curl -fsSL https://raw.githubusercontent.com/shunkakinoki/dotfiles/${{ github.sha }}/install.sh | sh
Expand Down
1 change: 1 addition & 0 deletions config/hyprland/hyprland.conf
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ binde = CTRL ALT SHIFT SUPER, minus, exec, hyprctl -j monitors | jq -r '.[0].sca
# Lock Screen
# =============================================================================
bind = CTRL ALT SHIFT SUPER, L, exec, noctalia-shell ipc call lockScreen lock
bindl = , switch:on:Lid Switch, exec, noctalia-shell ipc call lockScreen lock

# =============================================================================
# Notification Toggle
Expand Down
22 changes: 21 additions & 1 deletion config/noctalia/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
pkgs,
...
}:
let
noctaliaShell = inputs.noctalia-shell.packages.${pkgs.system}.default;
noctaliaLockBeforeSleep = pkgs.replaceVars ./lock-before-sleep.sh {
noctalia_shell = "${noctaliaShell}/bin/noctalia-shell";
sleep = "${pkgs.coreutils}/bin/sleep";
};
in
{
xdg.configFile."noctalia/colorschemes/Dracula-Custom/Dracula-Custom.json" = {
source = ./Dracula-Custom.json;
Expand All @@ -24,9 +31,21 @@
Install.WantedBy = [ "graphical-session.target" ];
};

systemd.user.services.noctalia-lock-before-sleep = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This service duplicates the functionality of the lockOnSuspend = true; setting at line 102. If the built-in setting is non-functional, consider removing it or adding a comment explaining why this manual service is necessary to avoid maintainability issues.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Document the overlap with general.lockOnSuspend: noctalia is already configured with general.lockOnSuspend = true on line 102, so it isn't obvious from the diff alone that this systemd unit is intentionally a second mechanism rather than a duplicate. The commit message explains it (lid-suspend bypasses the in-app hook), but that context is lost once merged. A one-line comment above the unit pointing to services.logind.settings.Login.HandleLidSwitch in named-hosts/matic/default.nix will prevent a future cleanup pass from deleting one of the two and silently regressing lid-close locking.

Unit = {
Description = "Lock Noctalia before system sleep";
Before = [ "sleep.target" ];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

sleep.target is a system-level target. By default, systemd --user instances do not have a sleep.target, so this service will not be triggered upon suspension unless a proxy like systemd-lock-handler is used. For a more robust implementation, a system-level service or a dedicated locker manager is typically required.

Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing graphical-session ordering: the sibling ac-idle-inhibit unit (lines 13-25) uses After = [ "graphical-session.target" ] and PartOf = [ "graphical-session.target" ], but this new unit has neither. In edge sequences where sleep.target activates while graphical-session.target is inactive, the noctalia-shell IPC call will fail silently (and the script still exits 0). Adding After = [ "graphical-session.target" ] keeps the file consistent and makes the dependency explicit; avoid PartOf here since you don't want the unit stopped mid-suspend.

};
Service = {
Type = "oneshot";
ExecStart = "${pkgs.bash}/bin/bash ${noctaliaLockBeforeSleep}";
};
Install.WantedBy = [ "sleep.target" ];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This unit will likely never run on system suspend. systemd's user-instance sleep.target is not automatically activated when the system enters sleep — there is no built-in bridge from system to per-user systemd manager. This is a long-standing systemd limitation (open RFE: systemd#40387, prior RFE #15477 closed without implementation), and the NixOS Power Management wiki only documents system-level wantedBy = [ "post-resume.target" ] for this reason. I grepped the rest of the dotfiles and there's no user-sleep@.service proxy or systemd-lock-handler configured to bridge the gap. The fix is one of:

  1. Move this to a system-level systemd.services.* (in named-hosts/matic/default.nix) with wantedBy = [ "sleep.target" ]; before = [ "sleep.target" ]; and run as your user with XDG_RUNTIME_DIR set so the IPC reaches noctalia.
  2. Add a system-level proxy unit user-sleep@.service that does systemctl --user start sleep.target — then this existing user unit fires.
  3. Have a system-level Before=sleep.target unit just call loginctl lock-session, and let noctalia react via the login1 Lock D-Bus signal (this is essentially what general.lockOnSuspend = true already does, which would make the whole new unit unnecessary if the lid path can be made to lock-session correctly).

Please verify with journalctl --user -u noctalia-lock-before-sleep after a real suspend/resume cycle before merging — I expect the unit to never have a recorded activation.

(Apologies for missing this in my initial review — I assumed modern systemd propagates user sleep.target, but the sources above show it does not.)

};

programs.noctalia-shell = {
enable = true;
package = inputs.noctalia-shell.packages.${pkgs.system}.default;
package = noctaliaShell;

settings = {
bar = {
Expand Down Expand Up @@ -60,6 +79,7 @@
{ id = "PowerProfile"; }
{ id = "Volume"; }
{ id = "Brightness"; }
{ id = "DarkMode"; }
{ id = "ControlCenter"; }
];
};
Expand Down
12 changes: 12 additions & 0 deletions config/noctalia/lock-before-sleep.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/env bash
# Lock through Noctalia before systemd lets suspend continue.
set -euo pipefail

NOCTALIA_SHELL="@noctalia_shell@"
SLEEP="@sleep@"

if ! "$NOCTALIA_SHELL" ipc call lockScreen lock; then
exit 0
fi
Comment on lines +8 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Silent lock failure allows suspend with no log; add a logger call before exit 0.

When lockScreen lock returns non-zero (Noctalia IPC down, daemon crashed mid-session, etc.) the script immediately exits with success. systemd proceeds to suspend an unprotected session, with zero visibility into the failure.

The dual Hyprland bindl reduces the risk but doesn't cover the case where the IPC is broken inside an active session (the bindl would have fired before the hook, and if that failed too, no one will know).

At minimum, journal the failure so it's auditable:

🛡️ Proposed fix: log the failed lock before gracefully exiting
+LOGGER="@logger@"
+
 if ! "$NOCTALIA_SHELL" ipc call lockScreen lock; then
+  "$LOGGER" -t noctalia-lock-before-sleep \
+    "WARNING: lockScreen IPC call failed; suspending without a lock screen"
   exit 0
 fi

Wire @logger@ in the pkgs.replaceVars call in config/noctalia/default.nix:

+logger = "${pkgs.util-linux}/bin/logger";
 noctalia_shell = ...;
 sleep = ...;
🤖 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 `@config/noctalia/lock-before-sleep.sh` around lines 8 - 10, The script
currently exits silently when NOCTALIA_SHELL ipc call lockScreen lock fails;
update the failure branch in lock-before-sleep.sh to call the injected logger
(the `@logger`@ variable provided via pkgs.replaceVars) with a clear message
including the command and its exit status before calling exit 0 so the journal
records the lock failure (refer to NOCTALIA_SHELL and the ipc call "lockScreen
lock" to find the branch to modify).


"$SLEEP" 1
8 changes: 6 additions & 2 deletions named-hosts/matic/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ import ../../hosts/nixos {
};
security.pam.services.noctalia-shell = {
fprintAuth = true;
rules.auth.fprintd.settings = {
max-tries = -1;
timeout = -1;
};
};
security.pam.services.sudo = {
fprintAuth = true;
Expand Down Expand Up @@ -246,9 +250,9 @@ import ../../hosts/nixos {

# Power button behavior - lock screen instead of shutdown
services.logind.settings.Login.HandlePowerKey = "lock";
# Suspend on lid close
# Suspend on battery lid close; on AC, Hyprland locks on the lid switch event.
services.logind.settings.Login.HandleLidSwitch = "suspend";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Battery lid-close still has a lock-vs-suspend race. With HandleLidSwitch = "suspend", systemd-logind starts the suspend chain immediately on lid close. The Hyprland bindl binding (added in this PR) async-forks noctalia-shell ipc call lockScreen lock in parallel — there’s no synchronization, so the kernel can win the race and suspend before noctalia has processed the IPC, leaving the desktop briefly visible on resume. The fallback noctalia-lock-before-sleep user unit doesn’t help here either (user-instance sleep.target isn’t bridged from system suspend).

The AC path you just added (HandleLidSwitchExternalPower = "ignore") sidesteps this because no suspend occurs. To close the gap on battery, either:

  1. Set HandleLidSwitch = "ignore" for battery too, and let a Hyprland binding sequence lock-then-suspend (exec, noctalia-shell ipc call lockScreen lock && sleep 0.5 && systemctl suspend), or
  2. Add a system-level systemd.services.lock-before-sleep ordered Before=sleep.target that calls loginctl lock-session (logind blocks suspend on the inhibitor until the unit completes; noctalia’s own login1 Lock subscription via general.lockOnSuspend = true then locks synchronously).

services.logind.settings.Login.HandleLidSwitchExternalPower = "suspend";
services.logind.settings.Login.HandleLidSwitchExternalPower = "ignore";

# Auto timezone (via geolocation)
services.geoclue2.enable = true;
Expand Down
1 change: 1 addition & 0 deletions spec/coverage_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ config/hyprland/scripts/record-screen.sh
config/hyprland/scripts/toggle-terminal.sh
config/k3s/activate.sh
config/noctalia/ac-idle-inhibit.sh
config/noctalia/lock-before-sleep.sh
config/obsidian/activate.sh
config/omp/activate.sh
config/openclaw/hydrate.sh
Expand Down
13 changes: 13 additions & 0 deletions spec/hyprland_lid_lock_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env bash
# shellcheck disable=SC2329

Describe 'config/hyprland/hyprland.conf lid lock binding'
CONFIG="$PWD/config/hyprland/hyprland.conf"

It 'locks Noctalia when Hyprland reports the lid switch closing'
When run bash -c "grep -F 'bindl = , switch:on:Lid Switch, exec, noctalia-shell ipc call lockScreen lock' '$CONFIG'"
The output should include 'switch:on:Lid Switch'
The output should include 'lockScreen lock'
End

End
17 changes: 17 additions & 0 deletions spec/matic_lid_policy_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bash
# shellcheck disable=SC2329

Describe 'named-hosts/matic/default.nix lid policy'
CONFIG="$PWD/named-hosts/matic/default.nix"

It 'keeps battery lid close as suspend'
When run bash -c "grep -F 'services.logind.settings.Login.HandleLidSwitch = \"suspend\";' '$CONFIG'"
The output should include 'HandleLidSwitch = "suspend"'
End

It 'lets Hyprland handle lid close while on AC power'
When run bash -c "grep -F 'services.logind.settings.Login.HandleLidSwitchExternalPower = \"ignore\";' '$CONFIG'"
The output should include 'HandleLidSwitchExternalPower = "ignore"'
End

End
14 changes: 14 additions & 0 deletions spec/matic_pam_fingerprint_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash
# shellcheck disable=SC2329

Describe 'named-hosts/matic/default.nix Noctalia fingerprint PAM'
CONFIG="$PWD/named-hosts/matic/default.nix"

It 'keeps lock-screen fingerprint auth from timing out'
When run bash -c "awk '/security.pam.services.noctalia-shell = \\{/{in_service=1} in_service{print} in_service && /^ \\};/{exit}' '$CONFIG'"
The output should include 'fprintAuth = true;'
The output should include 'max-tries = -1;'
The output should include 'timeout = -1;'
End

End
12 changes: 12 additions & 0 deletions spec/noctalia_bar_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/env bash
# shellcheck disable=SC2329

Describe 'config/noctalia/default.nix bar widgets'
CONFIG="$PWD/config/noctalia/default.nix"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

$PWD-based path may resolve incorrectly depending on invocation context.

CONFIG is assigned at Describe scope using $PWD, which is evaluated when ShellSpec parses/sources the spec file. If ShellSpec is invoked from a directory other than the repository root (e.g., shellspec spec/ from a subdirectory, or via a CI runner with a different $CWD), the path $PWD/config/noctalia/default.nix will be wrong and the awk pipeline will silently produce no output, causing a misleading test failure.

Consider using ShellSpec's %SPECROOT or a relative path anchored to the spec's own location, or add a guard:

🛡️ Proposed guard
+  CONFIG="$PWD/config/noctalia/default.nix"

 It 'places DarkMode immediately after Brightness on the right bar'
+  Skip if "config file not found" [ ! -f "$CONFIG" ]
   When run bash -c "awk ..."
🤖 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 `@spec/noctalia_bar_spec.sh` at line 5, CONFIG is currently set using $PWD
which can be wrong depending on how ShellSpec is invoked; change the assignment
to use ShellSpec's %SPECROOT (e.g.
CONFIG="%SPECROOT/config/noctalia/default.nix") or compute a path anchored to
the spec file (using dirname on $BASH_SOURCE or $SPEC) so it always resolves
relative to the spec, and add a guard immediately after (e.g. test -r "$CONFIG"
|| shellspec_fail "config not found: $CONFIG") so the spec fails clearly instead
of letting the downstream awk pipeline produce no output.


It 'places DarkMode immediately after Brightness on the right bar'
When run bash -c "awk '/widgets.right = \\[/{in_right=1} in_right && /id =/ { gsub(/.*id = \"|\";.*/, \"\"); print } in_right && /\\];/{exit}' '$CONFIG' | paste -sd ' ' -"
The output should include 'Brightness DarkMode ControlCenter'
End

End
24 changes: 24 additions & 0 deletions spec/noctalia_lock_before_sleep_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash
# shellcheck disable=SC2016,SC2329

Describe 'config/noctalia/lock-before-sleep.sh'
SCRIPT="$PWD/config/noctalia/lock-before-sleep.sh"

It 'uses bash strict mode'
When run bash -c "head -5 '$SCRIPT'"
The output should include 'set -euo pipefail'
End

It 'uses injected command paths'
When run bash -c "cat '$SCRIPT'"
The output should include '@noctalia_shell@'
The output should include '@sleep@'
End

It 'calls Noctalia lock IPC before sleeping'
When run bash -c "cat '$SCRIPT'"
The output should include 'ipc call lockScreen lock'
The output should include '"$SLEEP" 1'
End

End
Loading