Skip to content

Conversation

@Coldaine
Copy link
Owner

@Coldaine Coldaine commented Jan 1, 2026

User description

Addresses bot-flagged robustness items for agent instruction mirroring + does a quick documentation consistency pass.

Changes:

  • Make hardlink scripts portable (non-GNU stat), louder on failures, and idempotent.
  • Stop suppressing hook output; use --quiet mode instead.
  • Improve docs-ci mirror drift errors with actionable hints.
  • Align docs/plans + playbook language with current policy (AGENTS.md canonical; no docs/agents.md).
  • Clean up README duplication and remove conflicting STT claims.
  • Update CI split guidance in AGENTS.md + CI architecture doc.

PR Type

Enhancement, Bug fix


Description

  • Harden agent hardlink automation with portable stat, fallback copy, and idempotency checks

  • Replace output suppression with explicit --quiet flag for better control

  • Improve docs-ci mirror drift errors with actionable hints and error context

  • Align documentation policy: AGENTS.md canonical; remove conflicting docs/agents.md references

  • Update CI architecture guidance: move cargo build and unit tests to GitHub-hosted runners

  • Simplify README: remove outdated STT claims and consolidate development setup


Diagram Walkthrough

flowchart LR
  A["Hardlink Scripts"] -->|"Add --quiet flag"| B["Quiet Mode Control"]
  A -->|"Portable stat + fallback"| C["Cross-Platform Support"]
  A -->|"Idempotency checks"| D["Robustness"]
  E["CI Validation"] -->|"Better error messages"| F["Actionable Hints"]
  G["Documentation"] -->|"AGENTS.md canonical"| H["Single Source of Truth"]
  G -->|"Remove docs/agents.md refs"| H
  I["CI Architecture"] -->|"GitHub-hosted for build/tests"| J["Optimized Parallelism"]
Loading

File Walkthrough

Relevant files
Enhancement, error handling
2 files
ensure_agent_hardlinks.sh
Harden hardlink script with portability and robustness     
+98/-6   
docs-ci.yml
Add actionable error hints for mirror drift detection       
+10/-2   
Enhancement
4 files
install_git_hardlink_hooks.sh
Add quiet mode and improve hook configuration output         
+16/-3   
post-checkout
Use explicit quiet flag instead of output redirection       
+1/-1     
post-merge
Use explicit quiet flag instead of output redirection       
+1/-1     
mise.toml
Use quiet flag for hardlink hook installation                       
+1/-1     
Documentation
9 files
copilot-instructions.md
Update CI split guidance for build and unit tests               
+1/-1     
agents.md
Update CI split guidance for build and unit tests               
+1/-1     
AGENTS.md
Update CI split guidance for build and unit tests               
+1/-1     
README.md
Simplify and remove outdated STT documentation claims       
+10/-97 
MasterDocumentationPlaybook.md
Clarify hardlink mirrors policy and remove docs/agents.md
+3/-3     
architecture.md
Restructure CI split rationale and move build to GitHub-hosted
+34/-39 
documentation-migration-mapping.md
Add override note on canonical AGENTS.md policy                   
+7/-2     
future-documentation-architecture.md
Add override note removing docs/agents.md from future state
+5/-1     
proposal-documentation-restructure.md
Add override note on canonical AGENTS.md and remove docs/agents.md
+10/-6   

Copilot AI review requested due to automatic review settings January 1, 2026 00:48
@Coldaine Coldaine merged commit 8bf6b9d into main Jan 1, 2026
5 of 6 checks passed
@Coldaine Coldaine deleted the chore/bot-fixes-doc-pass branch January 1, 2026 00:48
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@qodo-free-for-open-source-projects

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Symlink following vulnerability

Description: The ln -f command can follow symlinks and overwrite arbitrary files if $dst is a symlink
pointing outside the repository, potentially allowing an attacker to overwrite system
files or sensitive data by manipulating symlinks before running the script.
ensure_agent_hardlinks.sh [65-73]

Referred Code
  if ln -f "$src" "$dst" 2>/dev/null; then
    return 0
  fi

  # If hardlinking fails (e.g., cross-filesystem), keep content correct but warn.
  cp -f "$src" "$dst"
  echo "warning: could not hardlink $dst; copied contents instead" >&2
  return 0
}
TOCTOU race condition

Description: The script exits with error if content doesn't match after link_or_copy, but an attacker
could exploit race conditions between the copy/link operation and the cmp check to inject
malicious content into agent instruction files that are executed by AI agents.
ensure_agent_hardlinks.sh [96-99]

Referred Code
if ! cmp -s "$src" "$dst"; then
  echo "error: $dst does not match $src" >&2
  exit 1
fi
Command injection via error output

Description: Error messages containing user-controlled paths or filenames are output without
sanitization, which could allow injection of GitHub Actions workflow commands through
specially crafted branch names or file paths (e.g., ::set-output name=x::value).
docs-ci.yml [44-53]

Referred Code
if ! diff -u AGENTS.md .github/copilot-instructions.md; then
  echo "::error::Agent instruction mirror drift: .github/copilot-instructions.md differs from AGENTS.md"
  echo "hint: run ./scripts/ensure_agent_hardlinks.sh (or mise run prepare)" >&2
  exit 1
fi
if ! diff -u AGENTS.md .kilocode/rules/agents.md; then
  echo "::error::Agent instruction mirror drift: .kilocode/rules/agents.md differs from AGENTS.md"
  echo "hint: run ./scripts/ensure_agent_hardlinks.sh (or mise run prepare)" >&2
  exit 1
fi
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Quiet flag bug: The call "$repo_root/scripts/ensure_agent_hardlinks.sh" ${quiet:+--quiet} always
expands --quiet because quiet is set to 0/1 (non-empty), preventing intended non-quiet
behavior and masking expected output in error/diagnostic scenarios.

Referred Code
"$repo_root/scripts/ensure_agent_hardlinks.sh" ${quiet:+--quiet}

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix contradictory hardlink fallback logic

Fix the contradictory logic in ensure_agent_hardlinks.sh where the fallback to
copying a file still results in an error because a hardlink is expected.

scripts/ensure_agent_hardlinks.sh [57-106]

 link_or_copy() {
   local dst="$1"
 
   # Remove symlinks explicitly; ln -f replaces regular files but not all symlinks reliably.
   if [[ -L "$dst" ]]; then
     rm -f "$dst"
   fi
 
   if ln -f "$src" "$dst" 2>/dev/null; then
-    return 0
+    return 0 # 0 for hardlinked
   fi
 
   # If hardlinking fails (e.g., cross-filesystem), keep content correct but warn.
   cp -f "$src" "$dst"
   echo "warning: could not hardlink $dst; copied contents instead" >&2
-  return 0
+  return 1 # 1 for copied
 }
 
 is_hardlinked_pair() {
   local a="$1"
   local b="$2"
 
   local ia ib
   if ia="$(inode_of "$a")" && ib="$(inode_of "$b")"; then
     [[ "$ia" == "$ib" ]]
     return $?
   fi
 
   # Fallback: inode via ls -i (portable enough for typical *nix).
   ia="$(ls -di "$a" 2>/dev/null | awk '{print $1}')" || return 1
   ib="$(ls -di "$b" 2>/dev/null | awk '{print $1}')" || return 1
-  [[ "$ia" == "$ib" ]]
+  [[ -n "$ia" && "$ia" == "$ib" ]]
 }
 
 ensure_pair() {
   local dst="$1"
 
-  link_or_copy "$dst"
-
-  if ! cmp -s "$src" "$dst"; then
-    echo "error: $dst does not match $src" >&2
-    exit 1
+  if ! link_or_copy "$dst"; then
+    # This block runs if hardlinking failed and a copy was made.
+    # The copy is acceptable, but we should verify it.
+    if ! cmp -s "$src" "$dst"; then
+      echo "error: $dst was copied but does not match $src" >&2
+      exit 1
+    fi
+    # The copy is valid, so we don't exit with an error.
+    return 0
   fi
 
+  # This block runs if hardlinking succeeded.
   if ! is_hardlinked_pair "$src" "$dst"; then
     echo "error: $dst is not hardlinked to $src (same contents, different inode)" >&2
     echo "hint: ensure both files are on the same filesystem; rerun scripts/ensure_agent_hardlinks.sh" >&2
     exit 2
   fi
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical logic flaw in the new ensure_agent_hardlinks.sh script where the copy fallback mechanism is rendered useless by a subsequent hardlink check, causing the script to fail.

High
  • More

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix hardlink check after copy fallback

Modify link_or_copy to return a status indicating whether it created a hardlink
or fell back to a copy. Then, update ensure_pair to only enforce the hardlink
check when a hardlink was successfully created.

scripts/ensure_agent_hardlinks.sh [91-106]

+link_or_copy() {
+  local dst="$1"
+
+  # Remove symlinks explicitly; ln -f replaces regular files but not all symlinks reliably.
+  if [[ -L "$dst" ]]; then
+    rm -f "$dst"
+  fi
+
+  if ln -f "$src" "$dst" 2>/dev/null; then
+    return 0 # 0 for hardlink success
+  fi
+
+  # If hardlinking fails (e.g., cross-filesystem), keep content correct but warn.
+  cp -f "$src" "$dst"
+  echo "warning: could not hardlink $dst; copied contents instead" >&2
+  return 1 # 1 for copy fallback
+}
+
+# ...
+
 ensure_pair() {
   local dst="$1"
+  local link_status=0
 
-  link_or_copy "$dst"
+  link_or_copy "$dst" || link_status=$?
 
   if ! cmp -s "$src" "$dst"; then
     echo "error: $dst does not match $src" >&2
     exit 1
   fi
 
-  if ! is_hardlinked_pair "$src" "$dst"; then
-    echo "error: $dst is not hardlinked to $src (same contents, different inode)" >&2
-    echo "hint: ensure both files are on the same filesystem; rerun scripts/ensure_agent_hardlinks.sh" >&2
-    exit 2
+  if [[ "$link_status" -eq 0 ]]; then # Only enforce hardlink if link_or_copy succeeded in linking
+    if ! is_hardlinked_pair "$src" "$dst"; then
+      echo "error: $dst is not hardlinked to $src (same contents, different inode)" >&2
+      echo "hint: ensure both files are on the same filesystem; rerun scripts/ensure_agent_hardlinks.sh" >&2
+      exit 2
+    fi
   fi
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant logic bug where the script would error out even when its copy-fallback mechanism is correctly triggered, making the script fail in valid cross-filesystem scenarios.

Medium
Correctly pass the quiet flag

Correct the logic for passing the --quiet flag to the sub-script. Use an array
to build arguments conditionally, ensuring the flag is only added when $quiet is
1.

scripts/install_git_hardlink_hooks.sh [38]

-"$repo_root/scripts/ensure_agent_hardlinks.sh" ${quiet:+--quiet}
+args=()
+if [[ "$quiet" -eq 1 ]]; then
+  args+=(--quiet)
+fi
+"$repo_root/scripts/ensure_agent_hardlinks.sh" "${args[@]}"
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a bug where the --quiet flag is always passed to a sub-script due to incorrect shell parameter expansion, and provides a correct fix.

Medium
General
Avoid redundant stat command calls

Improve script performance by detecting the stat command variant (GNU vs. BSD)
once at startup. Store the correct format flags in variables to avoid redundant
stat calls within the inode_of and link_count_of functions.

scripts/ensure_agent_hardlinks.sh [31-55]

+# ... (script start)
+
+# Detect stat format once
+if stat -c '%i' . >/dev/null 2>&1; then
+  STAT_INODE_FMT='%i'
+  STAT_LINKS_FMT='%h'
+elif stat -f '%i' . >/dev/null 2>&1; then
+  STAT_INODE_FMT='%i'
+  STAT_LINKS_FMT='%l'
+else
+  STAT_INODE_FMT=''
+  STAT_LINKS_FMT=''
+fi
+
+say() {
+# ...
+
 inode_of() {
   local path="$1"
-  if stat -c '%i' "$path" >/dev/null 2>&1; then
-    stat -c '%i' "$path"
-    return 0
-  fi
-  if stat -f '%i' "$path" >/dev/null 2>&1; then
-    stat -f '%i' "$path"
-    return 0
+  if [[ -n "$STAT_INODE_FMT" ]]; then
+    stat -f "$STAT_INODE_FMT" "$path" 2>/dev/null || stat -c "$STAT_INODE_FMT" "$path" 2>/dev/null
+    return $?
   fi
   return 1
 }
 
 link_count_of() {
   local path="$1"
-  if stat -c '%h' "$path" >/dev/null 2>&1; then
-    stat -c '%h' "$path"
-    return 0
-  fi
-  if stat -f '%l' "$path" >/dev/null 2>&1; then
-    stat -f '%l' "$path"
-    return 0
+  if [[ -n "$STAT_LINKS_FMT" ]]; then
+    stat -f "$STAT_LINKS_FMT" "$path" 2>/dev/null || stat -c "$STAT_LINKS_FMT" "$path" 2>/dev/null
+    return $?
   fi
   return 1
 }
 
+# ... (rest of script)
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion offers a valid micro-optimization to reduce redundant stat calls, improving script efficiency and portability, but the performance impact is likely negligible for this use case.

Low
  • More

@kiloconnect
Copy link

kiloconnect bot commented Jan 1, 2026

✅ No Issues Found

15 files reviewed | Confidence: 95% | Recommendation: Approved

Review Details

Files Changed:

  • Scripts: ensure_agent_hardlinks.sh, install_git_hardlink_hooks.sh, .githooks/post-checkout, .githooks/post-merge, mise.toml
  • CI/CD: .github/workflows/docs-ci.yml
  • Documentation: AGENTS.md, .github/copilot-instructions.md, .kilocode/rules/agents.md, README.md, docs/MasterDocumentationPlaybook.md, docs/dev/CI/architecture.md, docs/plans/documentation-migration-mapping.md, docs/plans/future-documentation-architecture.md, docs/plans/proposal-documentation-restructure.md

Checked: Script robustness, portability, error handling, documentation consistency, CI configuration

Summary: This PR successfully hardens agent hardlink automation with significant improvements to portability, robustness, and user experience. All changes are improvements with no issues identified.

Key Improvements:

  1. Script Hardening: Portable stat commands (GNU/BSD), fallback copying for cross-filesystem scenarios, idempotency validation
  2. Better UX: Explicit --quiet flag replacing output redirection, actionable error messages with hints
  3. Documentation: CI split updates, simplified README, clarified AGENTS.md canonical policy
  4. CI: Enhanced error messages in docs validation workflow

PR Status: ✅ Already merged successfully on 2026-01-01

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the agent instruction mirroring automation and improves documentation consistency across the repository. The changes focus on making hardlink management scripts more robust, portable, and informative while aligning documentation with the current policy that AGENTS.md is the canonical source at the repo root.

Key Changes:

  • Enhanced hardlink scripts with portable stat detection, better error messages, and --quiet mode support
  • Updated CI split guidance to reflect that builds and unit tests run on GitHub-hosted runners (only hardware tests on self-hosted)
  • Consolidated README to remove duplication and outdated STT claims

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scripts/ensure_agent_hardlinks.sh Major refactor: adds portable stat detection (GNU/BSD), hardlink verification, copy fallback, and actionable error messages
scripts/install_git_hardlink_hooks.sh Adds --quiet mode, idempotency checks, and improved status reporting
mise.toml Updates to use --quiet flag instead of output redirection
.githooks/post-merge Switches from output redirection to --quiet flag
.githooks/post-checkout Switches from output redirection to --quiet flag
docs/dev/CI/architecture.md Updates CI split rationale: GitHub-hosted now handles builds+tests, self-hosted only for hardware tests
AGENTS.md Updates CI split table to reflect new architecture
.github/copilot-instructions.md Mirror of AGENTS.md CI split update
.kilocode/rules/agents.md Mirror of AGENTS.md CI split update
.github/workflows/docs-ci.yml Improves mirror drift error messages with actionable hints
docs/plans/documentation/proposal-documentation-restructure.md Adds current-state override notes clarifying AGENTS.md policy
docs/plans/documentation/future-documentation-architecture.md Adds current-state override notes clarifying AGENTS.md policy
docs/plans/documentation-migration-mapping.md Adds current-state override notes clarifying AGENTS.md policy
docs/MasterDocumentationPlaybook.md Clarifies that agent mirrors should be hardlinked (preferred) rather than symlinked
README.md Major simplification: removes 100+ lines of duplicated/outdated content, consolidates to minimal entrypoint

Comment on lines +102 to +104
echo "error: $dst is not hardlinked to $src (same contents, different inode)" >&2
echo "hint: ensure both files are on the same filesystem; rerun scripts/ensure_agent_hardlinks.sh" >&2
exit 2
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The script exits with status 2 if hardlinking succeeds but verification fails. However, if hardlinking initially fails in link_or_copy and falls back to copying (line 70), the subsequent verification at line 101 will always fail because the files won't be hardlinked. This creates an inconsistent behavior where the script warns about copying but then exits with an error.

Consider either:

  1. Tracking whether a copy fallback occurred and skipping the hardlink verification for those cases
  2. Making the hardlink verification a warning instead of an error when content matches
  3. Exiting early after copy fallback with appropriate status
Suggested change
echo "error: $dst is not hardlinked to $src (same contents, different inode)" >&2
echo "hint: ensure both files are on the same filesystem; rerun scripts/ensure_agent_hardlinks.sh" >&2
exit 2
echo "warning: $dst is not hardlinked to $src (same contents, different inode)" >&2
echo "hint: ensure both files are on the same filesystem; rerun scripts/ensure_agent_hardlinks.sh if you require hardlinks" >&2

Copilot uses AI. Check for mistakes.

echo "Enabled repo hooks via core.hooksPath=.githooks"
"$repo_root/scripts/ensure_agent_hardlinks.sh"
"$repo_root/scripts/ensure_agent_hardlinks.sh" ${quiet:+--quiet}
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The parameter expansion ${quiet:+--quiet} is used without quotes. While this works when the variable expands to nothing or a single word, it's safer to quote it. If the expansion ever included spaces or special characters, this could cause unexpected word splitting.

Use "${quiet:+--quiet}" instead for proper quoting.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants