Merge unstable: Fix duplicate modules, add shell env tests

lib/fish/deployer.fish

@@ -45,17 +45,26 @@ function deployer-deploy-module
         fedpunk-config-init
     end
 
-    # Normalize git URLs to module names for consistent config storage
-    # git@gitlab.com:org/thinkpad-fans.git -> thinkpad-fans
-    set -l module_name "$module_ref"
-    if module-ref-is-url "$module_ref"
-        # Extract repo name from URL (same logic as external-module-get-storage-path)
-        set module_name (string replace -r '\.git$' '' "$module_ref")
-        set module_name (string replace -r '^.*[/:]' '' "$module_name")
+    # Normalize module reference before storing
+    # - URLs: Keep as-is (fixes duplicate entry bug)
+    # - Paths: Convert to absolute (fixes relative path resolution)
+    # - Names: Keep as-is
+    set -l config_ref "$module_ref"
+
+    # Check if it's a path (contains / but not a URL)
+    if string match -q '*/*' "$module_ref"; and not module-ref-is-url "$module_ref"
+        # It's a path - convert to absolute
+        set -l expanded_path (string replace -r '^~' "$HOME" "$module_ref")
+
+        # If not already absolute, make it absolute relative to PWD
+        if not string match -q '/*' "$expanded_path"
+            set expanded_path (realpath "$expanded_path" 2>/dev/null; or echo "$PWD/$expanded_path")
+        end
+
+        set config_ref "$expanded_path"
     end
 
-    # Add normalized module name to config
-    fedpunk-config-add-module "$module_name"
+    fedpunk-config-add-module "$config_ref"
 
     # Use existing fedpunk-module deploy (already handles local + git)
     if fedpunk-module deploy "$module_ref"

test/ci/test-env-login-vs-nonlogin-shells.sh

@@ -0,0 +1,192 @@
+#!/bin/bash
+# Regression test for env vars only available in login shells
+#
+# This test reproduces the issue where:
+# 1. User deploys a module with environment variables
+# 2. Env vars are NOT available in current shell
+# 3. Env vars are NOT available in new non-login shell (bash from bash)
+# 4. Env vars ARE available in new login shell (ssh, new terminal)
+#
+# Expected behavior: Env vars should be available everywhere
+# Current behavior: Only in login shells (bug)
+
+set -e
+
+echo ""
+echo "========================================="
+echo "Login vs Non-Login Shell Env Test"
+echo "========================================="
+echo "Reproducing the 'had to run bash again' bug"
+echo ""
+
+# Setup test environment
+TEST_DIR=$(mktemp -d -t fedpunk-shell-test-XXXXXX)
+trap "rm -rf $TEST_DIR" EXIT
+
+echo "Test environment: $TEST_DIR"
+echo ""
+
+# Override HOME for isolated testing
+export HOME="$TEST_DIR/home"
+mkdir -p "$HOME/.config/fedpunk/profile.d"
+mkdir -p "$TEST_DIR/etc/profile.d"
+
+# Simulate module deployment generating env config
+# Use unique var name to avoid pollution from current shell
+UNIQUE_VAR="FEDPUNK_TEST_$$_$(date +%s)"
+cat > "$HOME/.config/fedpunk/profile.d/fedpunk-env.sh" <<EOF
+#!/bin/sh
+# Auto-generated by fedpunk - DO NOT EDIT
+export ${UNIQUE_VAR}="deployed_value"
+export FEDPUNK_TEST_API="https://api.test.com"
+EOF
+
+echo "Simulated module deployment - generated env config at:"
+echo "  $HOME/.config/fedpunk/profile.d/fedpunk-env.sh"
+echo ""
+
+# Simulate system /etc/profile.d/fedpunk.sh
+cat > "$TEST_DIR/etc/profile.d/fedpunk.sh" <<'EOF'
+#!/bin/sh
+# Fedpunk environment variables
+export FEDPUNK_SYSTEM=/usr/share/fedpunk
+export FEDPUNK_USER=$HOME/.local/share/fedpunk
+
+# Auto-load user module environment variables
+if [ -f "$HOME/.config/fedpunk/profile.d/fedpunk-env.sh" ]; then
+    . "$HOME/.config/fedpunk/profile.d/fedpunk-env.sh"
+fi
+EOF
+
+echo "Created simulated /etc/profile.d/fedpunk.sh"
+echo ""
+
+#
+# Test 1: Current shell does NOT have env vars (expected)
+#
+echo "=== Test 1: Current shell after deployment ==="
+echo "This simulates: user just ran 'fedpunk module deploy'"
+echo ""
+
+if [ -z "$FEDPUNK_TEST_API" ]; then
+    echo "  ✓ EXPECTED: FEDPUNK_TEST_API not set in current shell"
+    echo "    (User would need to manually source the config)"
+else
+    echo "  ✗ UNEXPECTED: FEDPUNK_TEST_API is set: $FEDPUNK_TEST_API"
+fi
+
+echo ""
+
+#
+# Test 2: Non-login shell does NOT have env vars (BUG)
+#
+echo "=== Test 2: New non-login shell (bash from bash) ==="
+echo "This simulates: user runs 'bash' from their current bash shell"
+echo ""
+
+# Non-login interactive shell (like running 'bash' from bash)
+NONLOGIN_VALUE=$(bash -c "
+export HOME='$HOME'
+echo \$FEDPUNK_TEST_API
+")
+
+if [ -z "$NONLOGIN_VALUE" ]; then
+    echo "  ❌ BUG REPRODUCED: FEDPUNK_TEST_API not set in non-login shell"
+    echo "    /etc/profile.d/ is NOT sourced for non-login shells"
+    echo "    This is the bug the user experienced!"
+    BUG_REPRODUCED=1
+else
+    echo "  ✓ UNEXPECTED: FEDPUNK_TEST_API is set: $NONLOGIN_VALUE"
+    echo "    (Test environment might be different from real world)"
+fi
+
+echo ""
+
+#
+# Test 3: Login shell HAS env vars (works but requires new login)
+#
+echo "=== Test 3: New login shell (ssh, new terminal) ==="
+echo "This simulates: user SSH'ing into container or opening new terminal"
+echo ""
+
+# Login shell (sources /etc/profile which sources /etc/profile.d/*)
+LOGIN_VALUE=$(bash --login -c "
+export HOME='$HOME'
+# Manually source profile.d to simulate login shell behavior
+. '$TEST_DIR/etc/profile.d/fedpunk.sh'
+echo \$FEDPUNK_TEST_API
+")
+
+if [ "$LOGIN_VALUE" = "https://api.test.com" ]; then
+    echo "  ✓ WORKS: FEDPUNK_TEST_API set in login shell"
+    echo "    Value: $LOGIN_VALUE"
+    echo "    But user had to start a NEW login shell to get it!"
+else
+    echo "  ✗ FAIL: FEDPUNK_TEST_API not set even in login shell"
+    echo "    Expected: 1"
+    echo "    Got: $LOGIN_VALUE"
+    exit 1
+fi
+
+echo ""
+
+#
+# Test 4: User's workaround - running 'bash' might create login shell
+#
+echo "=== Test 4: User's workaround behavior ==="
+echo "When user ran 'bash' again, they might have gotten lucky with:"
+echo "  - Container configuration that forces login shells"
+echo "  - .bashrc that sources /etc/profile"
+echo "  - bash --login being their default"
+echo ""
+
+# Try various bash invocations
+echo "Testing different bash invocations:"
+
+# Standard non-login
+STANDARD=$(bash -c "export HOME='$HOME'; echo \$FEDPUNK_TEST_API")
+echo "  bash -c: '$STANDARD' (empty = not set)"
+
+# Interactive non-login
+INTERACTIVE=$(bash --init-file /dev/null -i -c "export HOME='$HOME'; echo \$FEDPUNK_TEST_API" 2>/dev/null || echo "")
+echo "  bash -i: '$INTERACTIVE' (empty = not set)"
+
+# Login shell
+LOGIN=$(bash --login -c "export HOME='$HOME'; . '$TEST_DIR/etc/profile.d/fedpunk.sh'; echo \$FEDPUNK_TEST_API")
+echo "  bash --login: '$LOGIN' (should be 'https://api.test.com')"
+
+echo ""
+
+#
+# Summary
+#
+echo "========================================="
+if [ -n "$BUG_REPRODUCED" ]; then
+    echo "BUG REPRODUCED!"
+else
+    echo "Bug not reproduced (environment differs)"
+fi
+echo "========================================="
+echo ""
+echo "Issue Summary:"
+echo "  ❌ Current shell: Env vars NOT available after deployment"
+echo "  ❌ Non-login shell (bash from bash): Env vars NOT available"
+echo "  ✅ Login shell (new terminal/ssh): Env vars available"
+echo ""
+echo "Real-world impact:"
+echo "  - User deploys module"
+echo "  - Tries to use env vars → NOT AVAILABLE"
+echo "  - Runs 'bash' → STILL NOT AVAILABLE (unless login shell)"
+echo "  - Has to start new terminal/ssh → Finally available"
+echo ""
+echo "Root cause:"
+echo "  /etc/profile.d/ only sourced for LOGIN shells, not:"
+echo "    - Current shell (where deployment happens)"
+echo "    - Non-login interactive shells (bash from bash)"
+echo "    - Subshells"
+echo ""
+echo "Solutions needed:"
+echo "  1. Source config in current shell during deployment"
+echo "  2. Add to /etc/bash.bashrc for non-login interactive shells"
+echo "  3. Add to /etc/zshrc for zsh"
+echo ""

test/ci/test-external-module-params-no-duplicate.sh

@@ -112,16 +112,14 @@ echo ""
 #
 # Test 2: Simulate the bug scenario - deployer-deploy-module with params
 #
-echo "=== Test 2: Simulate deployer-deploy-module bug scenario ==="
+echo "=== Test 2: Deploy module with parameters (testing fix) ==="
 
 # Initialize config
 run_fish "fedpunk-config-init" 2>&1 || true
 
-# Simulate the buggy behavior:
-# Step 1: deployer-deploy-module adds NORMALIZED NAME (line 58)
-NORMALIZED_NAME="test-params-ext-module"
-run_fish "fedpunk-config-add-module '$NORMALIZED_NAME'" 2>&1 || true
-echo "  Step 1: Added normalized name to config: $NORMALIZED_NAME"
+# Step 1: deployer-deploy-module adds module URL (FIXED behavior)
+run_fish "fedpunk-config-add-module '$TEST_MODULE_URL'" 2>&1 || true
+echo "  Step 1: Added module URL to config: $TEST_MODULE_URL"
 
 # Step 2: param-save-to-config tries to find using URL (not name)
 # This is what happens when fedpunk-module deploy calls param-prompt-required
@@ -131,7 +129,7 @@ param-save-to-config '$TEST_MODULE_URL' 'auth_mode' 'enabled'
 " 2>&1 || true
 
 echo "  Step 2: Saved params using URL: $TEST_MODULE_URL"
-echo "  (This is where the duplicate gets created if bug exists)"
+echo "  (Should update existing entry, not create duplicate)"
 echo ""
 
 #