Scope test-only constants with `#[cfg(test)]` instead of `#[allow(dead_code)]`
Intent: Four `PREPARED_OUTPUT_FULFILLMENT_SSH_*` constants are only consumed inside `#[cfg(test)]` blocks. The previous approach silenced the dead-code lint with `#[allow(dead_code)]`, which hid the fact that these symbols serve no purpose in production builds. Replacing the annotation with `#[cfg(test)]` makes the intent explicit: the constants exist exclusively for tests and are compiled out of release artifacts entirely.
Affected files: crates/jerichoci/src/run.rs
Evidence
@@ -101,15 +101,15 @@ const PREPARED_OUTPUT_FULFILLMENT_SSH_BINARY_ENV: &str =
-#[allow(dead_code)]
+#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_HOST_ENV: &str = "PIKACI_PREPARED_OUTPUT_FULFILL_SSH_HOST";
-#[allow(dead_code)]
+#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_REMOTE_LAUNCHER_BINARY_ENV: &str =
-#[allow(dead_code)]
+#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_REMOTE_HELPER_BINARY_ENV: &str =
-#[allow(dead_code)]
+#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_REMOTE_WORK_DIR_ENV: &str =
Why #[cfg(test)] over #[allow(dead_code)]
Both annotations suppress the compiler warning, but they carry very different semantics:
| Annotation | Compiled into release? | Communicates intent? |
|---|
#[allow(dead_code)] | Yes | No — only says "ignore this warning" |
#[cfg(test)] | No | Yes — marks the item as test-only |
#[allow(dead_code)] is a blanket suppression that can mask legitimate issues (e.g., a constant that should be used in production but was accidentally orphaned). #[cfg(test)] is a precise gate: the constant will not exist outside cargo test, so any accidental production reference becomes a compile error rather than a silent dead symbol.
Affected constants
All four constants follow the same pattern and live at crates/jerichoci/src/run.rs:104-113:
#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_HOST_ENV: &str = "PIKACI_PREPARED_OUTPUT_FULFILL_SSH_HOST";
#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_REMOTE_LAUNCHER_BINARY_ENV: &str = …;
#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_REMOTE_HELPER_BINARY_ENV: &str = …;
#[cfg(test)]
const PREPARED_OUTPUT_FULFILLMENT_SSH_REMOTE_WORK_DIR_ENV: &str = …;
The two constants directly above them (_SSH_BINARY_ENV and _SSH_NIX_BINARY_ENV) remain unconditional because they are used in non-test code paths.
Fix stale documentation: relative link and legacy fallback removal
Intent: The Apple-remote-access doc contained a hard-coded absolute path (`/Users/justin/code/pika/worktrees/pikaci-mac/.github/pikaci-apple.env`) that only resolved on one developer's machine. It also referenced a legacy secret file (`secrets/pikaci-apple.env.age`) and a migration fallback that no longer applies. This step corrects the link to a portable repository-relative path and removes the obsolete fallback mention.
Affected files: docs/pikaci-apple-remote-access.md
Evidence
@@ -74,8 +74,8 @@
-- Non-secret Apple CI config is checked into [.github/pikaci-apple.env](/Users/justin/code/pika/worktrees/pikaci-mac/.github/pikaci-apple.env).
-- Apple CI secrets are checked into `secrets/pikaci-apple.sops.yaml` (with legacy fallback to `secrets/pikaci-apple.env.age` during migration).
+- Non-secret Apple CI config is checked into [.github/pikaci-apple.env](../.github/pikaci-apple.env).
+- Apple CI secrets are checked into `secrets/pikaci-apple.sops.yaml`.
Two small but meaningful documentation hygiene fixes in docs/pikaci-apple-remote-access.md:
-
Absolute path → relative path. The Markdown link target changes from an absolute filesystem path that was specific to one contributor's worktree layout to the relative path ../.github/pikaci-apple.env. Because the doc lives in docs/, ../ correctly resolves to the repository root, making the link work for every clone.
-
Legacy fallback sentence removed. The parenthetical "(with legacy fallback to secrets/pikaci-apple.env.age during migration)" described a transitional state that has concluded. Keeping it would mislead readers into thinking the .env.age file is still consulted at runtime. The sentence now simply states the current single source of truth: secrets/pikaci-apple.sops.yaml.
Remove unused variable assignment `ssh_binary_defaulted` in shell script
Intent: The `--ssh-binary` option handler set `ssh_binary_defaulted=0` after capturing the user-supplied binary path. This variable was never read anywhere else in the script — it was likely a remnant of a removed code path that once distinguished a default SSH binary from an explicitly provided one. Deleting the assignment eliminates dead shell code.
Affected files: scripts/pikaci-apple-remote.sh
Evidence
@@ -128,7 +128,6 @@ while [[ $# -gt 0 ]]; do
--ssh-binary)
ssh_binary="${2:?missing value for --ssh-binary}"
- ssh_binary_defaulted=0
shift 2
Inside the argument-parsing while loop at scripts/pikaci-apple-remote.sh:129, the --ssh-binary branch previously ran:
ssh_binary="${2:?missing value for --ssh-binary}"
ssh_binary_defaulted=0 # ← removed
shift 2
ssh_binary_defaulted is not referenced by any conditional, function, or export in the script. Searching for the identifier confirms zero reads. The variable was likely part of a guard like if (( ssh_binary_defaulted )); then … that was removed in an earlier change without cleaning up this write. Removing it keeps the option handler minimal and avoids confusing future readers about a flag that controls nothing.
Remove unused `remote_q` helper function from shell script
Intent: The `remote_q` function provided shell-safe single-quote escaping for a string argument. It was defined inside `run_locked_body` but never called, making it pure dead code. Removing it reduces cognitive overhead and eliminates a potential source of confusion about the script's quoting strategy.
Affected files: scripts/pikaci-apple-remote.sh
Evidence
@@ -487,10 +486,6 @@ run_locked_body() {
- remote_q() {
- printf "'%s'" "${1//\'/\'\"\'\"\'}"
- }
-
ensure_mirror() {
At scripts/pikaci-apple-remote.sh:489 (pre-patch line numbers), the function:
remote_q() {
printf "'%s'" "${1//\'/\'\"\'\"\'}"
}
wrapped a value in single quotes while escaping embedded single quotes using the '"'"' idiom. This is a standard POSIX shell-quoting pattern, but remote_q was never invoked — grep -n 'remote_q' returns only the definition itself.
The function lived inside run_locked_body, so it was re-declared on every invocation of that function, adding unnecessary overhead (however slight). Its removal leaves ensure_mirror as the first nested function, which is the actual next logical step in the lock body.