fix: restore agent mirror flexibility #337
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
There was a problem hiding this comment.
Pull request overview
This PR implements a resilient agent-instruction mirror strategy with a three-tier fallback mechanism (hardlink → symlink → copy) to handle cross-filesystem scenarios, while restoring Quick Start documentation and aligning CI architecture docs with current practice.
Key Changes:
- Adds
--require-hardlinkflag for strict mode (fails if hardlink not possible) vs. best-effort mode (accepts symlink/copy if content matches) - Implements Python-based relative path calculation for symlink fallback when hardlinks fail
- Restores Quick Start commands to README while deferring to critical-action-plan.md for backend status details
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/ensure_agent_hardlinks.sh | Renames link_or_copy() to link_or_symlink_or_copy() with three-tier fallback; adds --require-hardlink flag for conditional hardlink enforcement |
| scripts/install_git_hardlink_hooks.sh | Extends argument parsing to support --require-hardlink and passes flags through to ensure_agent_hardlinks.sh via array |
| docs/dev/CI/architecture.md | Updates self-hosted runner comment to clarify it's for hardware/display tests only, removes outdated build step |
| docs/MasterDocumentationPlaybook.md | Updates agent mirror documentation to reflect "hardlink preferred; symlink/copy acceptable" policy |
| README.md | Restores Quick Start section with just run/tui commands and file-scoped Cargo commands for local feedback |
| rel="$(python - <<'PY' | ||
| import os | ||
| import sys | ||
| src = sys.argv[1] | ||
| dst = sys.argv[2] | ||
| print(os.path.relpath(src, os.path.dirname(dst))) | ||
| PY | ||
| "$src" "$dst" 2>/dev/null || true)" |
There was a problem hiding this comment.
The Python heredoc is missing the arguments to the Python interpreter. The command should pass arguments to the Python script, but line 89 starts with "$src" "$dst" which will be interpreted as shell commands rather than arguments to Python.
The Python script expects sys.argv[1] and sys.argv[2], but these won't be available. This should be:
rel="$(python - "$src" "$dst" <<'PY'This ensures the arguments are passed to Python correctly.
| rel="$(python - <<'PY' | |
| import os | |
| import sys | |
| src = sys.argv[1] | |
| dst = sys.argv[2] | |
| print(os.path.relpath(src, os.path.dirname(dst))) | |
| PY | |
| "$src" "$dst" 2>/dev/null || true)" | |
| rel="$( | |
| python - "$src" "$dst" <<'PY' 2>/dev/null | |
| import os | |
| import sys | |
| src = sys.argv[1] | |
| dst = sys.argv[2] | |
| print(os.path.relpath(src, os.path.dirname(dst))) | |
| PY | |
| )" || true |
| if [[ "$quiet" -eq 1 ]]; then ensure_args+=(--quiet); fi | ||
| if [[ "$require_hardlink" -eq 1 ]]; then ensure_args+=(--require-hardlink); fi | ||
|
|
||
| "$repo_root/scripts/ensure_agent_hardlinks.sh" "${ensure_args[@]}" |
There was a problem hiding this comment.
The array expansion "${ensure_args[@]}" will fail with an "unbound variable" error when the array is empty due to set -euo pipefail. When neither --quiet nor --require-hardlink is set, ensure_args will be empty, and accessing it with @ will trigger the error.
Use this pattern instead:
"$repo_root/scripts/ensure_agent_hardlinks.sh" ${ensure_args[@]+"${ensure_args[@]}"}This safely expands to nothing when the array is empty.
| "$repo_root/scripts/ensure_agent_hardlinks.sh" "${ensure_args[@]}" | |
| "$repo_root/scripts/ensure_agent_hardlinks.sh" ${ensure_args[@]+"${ensure_args[@]}"} |
| echo "warning: could not hardlink $dst; created symlink instead" >&2 | ||
| return 0 | ||
| fi | ||
| fi | ||
|
|
||
| # 3) Last resort: copy contents. | ||
| cp -f "$src" "$dst" | ||
| echo "warning: could not hardlink $dst; copied contents instead" >&2 | ||
| echo "warning: could not hardlink or symlink $dst; copied contents instead" >&2 | ||
| return 0 |
There was a problem hiding this comment.
The warning message for symlink fallback (line 93) writes to stderr, but the successful hardlink case (line 76) doesn't produce any output. However, the final "AGENTS mirrors hardlinked" message (lines 148-150) may be misleading when symlinks or copies are used instead.
Consider checking if symlinks or copies were used and adjusting the final message accordingly, or emit warnings through the say() function so they respect the --quiet flag. Currently, fallback warnings always appear even with --quiet, which may be unintended.
User description
Implements (instead of redacting) a resilient agent-instruction mirror strategy and restores a usable Quick Start.
Commands:
PR Type
Enhancement, Bug fix
Description
Implements resilient three-tier agent mirror strategy: hardlink → symlink → copy
Adds
--require-hardlinkflag for strict mode; default allows symlink/copy fallbacksRestores Quick Start section to README with current backend status reference
Updates documentation to reflect flexible mirroring approach across tools
Removes outdated CI architecture claim about self-hosted being "THE build"
Diagram Walkthrough
flowchart LR A["Agent Mirror Request"] --> B{"Hardlink\nPossible?"} B -->|Yes| C["Create Hardlink"] B -->|No| D{"Symlink\nPossible?"} D -->|Yes| E["Create Symlink"] D -->|No| F["Copy Contents"] C --> G{"--require-hardlink\nSet?"} E --> G F --> G G -->|Yes| H{"Hardlinked?"} G -->|No| I["Success with Warning"] H -->|Yes| I H -->|No| J["Exit with Error"]File Walkthrough
ensure_agent_hardlinks.sh
Add symlink fallback and strict mode flagscripts/ensure_agent_hardlinks.sh
--require-hardlinkflag to enforce strict hardlink-only modelink_or_copytolink_or_symlink_or_copyforclarity
warnings
flags
install_git_hardlink_hooks.sh
Support require-hardlink flag propagationscripts/install_git_hardlink_hooks.sh
--quietand--require-hardlinkflags--require-hardlinkflag toensure_agent_hardlinks.shcallREADME.md
Restore Quick Start section with commandsREADME.md
truth
MasterDocumentationPlaybook.md
Document flexible agent mirroring strategydocs/MasterDocumentationPlaybook.md
preferred; symlink/copy acceptable"
strategies
architecture.md
Remove outdated self-hosted CI architecture claimdocs/dev/CI/architecture.md