Skip to content

feat: [HIMMEL-1640][HIMMEL-1643][HIMMEL-1644][HIMMEL-1645][HIMMEL-1648] leg-6/7 batch: graphify freshness+refresh, arm/suite hardening, critic env pinning - #559

Open
yotamleo wants to merge 2 commits into
mainfrom
feat/public-batch-leg7-20260809
Open

feat: [HIMMEL-1640][HIMMEL-1643][HIMMEL-1644][HIMMEL-1645][HIMMEL-1648] leg-6/7 batch: graphify freshness+refresh, arm/suite hardening, critic env pinning#559
yotamleo wants to merge 2 commits into
mainfrom
feat/public-batch-leg7-20260809

Conversation

@yotamleo

@yotamleo yotamleo commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

Leg-6/7 batch re-projection of private #1628–#1642 (private range 591eebee..c87cee76, 15 commits). All changes were CR-cleared privately (critic panel + codex adversarial per PR; findings fixed or premise-verified-and-deferred to tickets); the public re-projection is byte-verified by the ship pipeline.

Highlights:

  • HIMMEL-1643 — graphify freshness SessionStart advisory (three-state, silent-when-fresh) + wire-hook-bash install-when-missing.
  • HIMMEL-1644/graph-refresh one-shot operator refresh with a hardened vault preflight: salus/PHI hard-refuse mirroring the fence classifier, symlink-resolved classification, configured-identity containment (sentinels ≠ identity), canonical-path consumption, drive-relative refusal.
  • HIMMEL-1645GRAPHIFY_API_TIMEOUT knob; the raised 900s ceiling is scoped to the contending claude-cli backend only.
  • HIMMEL-1640 — wire-session-telemetry ownership-by-verb + --off byte-reversibility; avail supersession identity (head, model); strict frontmatter-ticket parser (both delimiters required, hard parse error on unclosed frontmatter, line-1-anchored opener with BOM strip).
  • HIMMEL-1641 — GLM worker end conditions (done_escalated terminal state, killed-by-caller finalize).
  • HIMMEL-1637/1606test-arm-resume --only/--list section filter + worker-census shield; real-arm prune coverage.
  • HIMMEL-1287 — cmd escaping routed through cadence_cmd_escape (nested-cmd/c proxy site keeps caret escaping).
  • HIMMEL-1646/1648 — critic credential loading: artifact-critic .env parity, then both loaders pinned to script-root resolution (lockstep).
  • dependabot minor-and-patch group bump (#1638).

Tickets: HIMMEL-1640, HIMMEL-1641, HIMMEL-1643, HIMMEL-1646, HIMMEL-1637, HIMMEL-1606, HIMMEL-1287, HIMMEL-1645, HIMMEL-1644, HIMMEL-1648

Summary by CodeRabbit

  • New Features

    • Added a command to refresh Luna and Himmel graphs with corpus selection, vault overrides, dry-run support, safety checks, and status reporting.
    • Added session-start advisories when graph data is stale or unavailable, including refresh guidance.
    • Added safer hook installation and repair for supported session events.
  • Improvements

    • Improved handling and reporting of interrupted or push-blocked background runs.
    • Added clearer diagnostics for commits without an upstream branch.
    • Improved credential loading and availability recovery across review workflows.
    • Improved graph extraction timeout handling while preserving environment settings.
  • Bug Fixes

    • Corrected graph refresh and session telemetry edge cases.

…6][HIMMEL-1637][HIMMEL-1287][HIMMEL-1645][HIMMEL-1644][HIMMEL-1648] leg-6/7 batch: graphify freshness+refresh, arm/suite hardening, critic env pinning

Batch re-projection of private #1628-#1642 (range 591eebee..c87cee76):

- HIMMEL-1640: wire-session-telemetry ownership-by-verb + --off
  byte-reversibility (#1628), ownedIndex splice guard (#1629), avail
  supersession identity = (head, model) (#1632), strict frontmatter-ticket
  parser (both delimiters, hard error on unclosed, line-1 anchored) (#1640)
- HIMMEL-1641: GLM worker end conditions - done_escalated terminal state,
  killed-by-caller finalize, verify-return push-block hint (#1631)
- HIMMEL-1643: graphify freshness SessionStart advisory + wire-hook-bash
  install-when-missing (#1633)
- HIMMEL-1646: artifact-critic loads .env credentials like critic-panel (#1634)
- HIMMEL-1637: test-arm-resume --only/--list filter + worker-census shield (#1635)
- HIMMEL-1606: T_PRUNE_REAL real-arm prune coverage (#1636)
- HIMMEL-1287: cmd escapes routed through cadence_cmd_escape; nested-cmd/c
  proxy site keeps caret escaping (#1637)
- HIMMEL-1645: refresh-graph-map GRAPHIFY_API_TIMEOUT knob, 900s ceiling
  scoped to claude-cli (#1639)
- HIMMEL-1644: /graph-refresh one-shot operator refresh + hardened vault
  preflight (salus/PHI, symlink, configured-identity) (#1641)
- HIMMEL-1648: critic credential load_dotenv pinned to script-root
  resolution, critic-panel + artifact-critic lockstep (#1642)
- dependabot minor-and-patch bump (#1638)

Platforms tested: windows
Security reviewed: ad-hoc
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e2371680-e115-4fc8-9f14-b61ea81b7e13

📥 Commits

Reviewing files that changed from the base of the PR and between 4b26d7d and 8eedb2d.

📒 Files selected for processing (2)
  • scripts/cr/test-critic-panel-registry.sh
  • scripts/graphify/test-graph-refresh.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/graphify/test-graph-refresh.sh

📝 Walkthrough

Walkthrough

This PR adds graph refresh tooling, configurable extraction timeouts, worker lifecycle states, credential and ledger updates, handover and hook hardening, telemetry ownership changes, and related tests and documentation.

Changes

Graph refresh workflow

Layer / File(s) Summary
Graph refresh wrapper and safety preflight
scripts/graphify/graph-refresh.sh
Adds guarded serial Luna and Himmel refreshes, vault validation, dry-run support, failure handling, and cadence reporting.
Backend-scoped extraction timeout
scripts/graphify/refresh-graph-map.sh, scripts/graphify/test-refresh-graph-map.sh
Adds backend-specific defaults and validates GRAPHIFY_API_TIMEOUT for update runs.
Graph refresh command contract and tests
.claude/commands/graph-refresh.md, docs/commands-catalog.md, scripts/graphify/test-graph-refresh.sh
Documents the command and tests parsing, safety checks, execution order, failures, and symlink handling.

GLM worker lifecycle

Layer / File(s) Summary
Worker completion and terminal statuses
docs/glm-offload.md, scripts/lanes/await-glm-worker.sh, scripts/lanes/verify-return.mjs, scripts/lanes/tests/verify-return.test.mjs
Documents parent-owned completion, adds terminal statuses, and reports no-upstream hints.
Worker timeout and termination finalization
scripts/telegram/spawn-glm.ts, scripts/telegram/spawn-glm.test.ts
Records caller termination and reclassifies eligible clean-tree timeouts with blocked push escalations.

Critic credentials and availability ledger

Layer / File(s) Summary
Primary-checkout credential loading
scripts/cr/artifact-critic.sh, scripts/cr/critic-panel.sh, scripts/cr/test-artifact-critic.sh, scripts/cr/test-critic-panel.sh, scripts/cr/test-critic-panel-registry.sh
Loads matching GLM credentials from the primary checkout while preserving process environment values.
Cross-artifact availability supersession
scripts/cr/ledger-append.sh, scripts/cr/test-ledger-append.sh, scripts/cr/test-clear-cr-marker.sh
Deduplicates availability records by (head, model) and supports cross-artifact recovery records.

Handover arm and test selection

Layer / File(s) Summary
Handover parsing and Windows command escaping
scripts/handover/arm-resume.sh
Uses shared command escaping and requires closed document-start YAML frontmatter.
Selectable handover regression coverage
scripts/handover/test-arm-resume.sh
Adds --only and --list selection and gates the existing regression groups.

Graph freshness and hook wiring

Layer / File(s) Summary
Graph freshness advisory hook
scripts/hooks/graphify-freshness-advisory.sh, scripts/hooks/test-graphify-freshness-advisory.sh
Adds primary-checkout graph freshness checks with stale, absent, and fail-open outcomes.
Owned hook installation and validation
scripts/hooks/wire-hook-bash.mjs, scripts/hooks/wire-hook-bash.test.mjs
Installs missing owned hooks at anchored positions and validates declared insertions.

Session telemetry hook ownership

Layer / File(s) Summary
Telemetry ownership and cleanup
scripts/observability/wire-session-telemetry-hooks.mjs, scripts/observability/wire-session-telemetry-hooks.test.mjs, scripts/observability/provisioning/alerting/rules.yaml
Recognizes command variants and removes event keys only after owned hooks are removed.

Package dependency update

Layer / File(s) Summary
Package dependency update
marketplace/plugins/obsidian-triage/tools/package.json
Updates js-yaml from 5.2.2 to 5.2.3.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant graph-refresh.sh
  participant refresh-graph-map.sh
  participant graphify
  Operator->>graph-refresh.sh: Select corpora and vault
  graph-refresh.sh->>refresh-graph-map.sh: Invoke canonical refresh
  refresh-graph-map.sh->>graphify: Run graphify --update
  graphify-->>refresh-graph-map.sh: Return refresh result
  refresh-graph-map.sh-->>graph-refresh.sh: Return corpus result
  graph-refresh.sh-->>Operator: Report refresh and cadence status
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.16% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the batch changes, including graphify refresh, test hardening, and critic environment updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/public-batch-leg7-20260809

Comment @coderabbitai help to get the list of available commands.

@yotamleo

yotamleo commented Aug 8, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (4)
scripts/graphify/test-graph-refresh.sh (2)

374-376: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assert that the ratified run actually fired the luna leg.

The ratified case checks rc 0 only. rc 0 does not prove the leg ran. Test 15 asserts both rc and the runner log for the same shape. Add the same log assertion here.

♻️ Proposed test strengthening
 rc=0; out=$(TEST_LUNA_ROOT="$OTHER_VAULT" run_refresh luna --vault "$OTHER_VAULT" 2>&1) || rc=$?
 assert_rc "ratified via LUNA_VAULT_PATH -> rc 0" 0 "$rc"
+assert_contains "ratified run fired the luna leg" "--name luna" "$(sed -n '1p' "$RUNNER_LOG")"
+assert_eq "ratified run fired exactly one leg" "1" "$(wc -l < "$RUNNER_LOG" | tr -d ' ')"
🤖 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 `@scripts/graphify/test-graph-refresh.sh` around lines 374 - 376, Strengthen
the ratified run in test-graph-refresh.sh by adding the same runner-log
assertion used by Test 15 after the existing rc 0 check. Verify that the luna
leg actually fired, reusing the established log assertion and expected
invocation shape.

15-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extend the Covers: list to Tests 10-15.

The header lists nine cases. The file now runs fifteen, including the preflight cases (salus marker, phi-roots, root/HOME, containment, drive-relative, sentinels, symlink refuse, symlink accept). A reader uses this list as the coverage contract.

🤖 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 `@scripts/graphify/test-graph-refresh.sh` around lines 15 - 31, Extend the
top-level “Covers:” list in the test script from nine entries to fifteen,
documenting the preflight cases for salus marker, phi-roots, root/HOME,
containment, drive-relative paths, sentinels, symlink refusal, and symlink
acceptance. Keep the existing cases unchanged and ensure the list accurately
matches all tests executed by the script.
scripts/hooks/test-graphify-freshness-advisory.sh (1)

52-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

T4 records no result when rc is non-zero.

Line 55 guards the case with [ "$rc" -eq 0 ] &&. If the hook exits non-zero, the case never runs. The test then calls neither pass nor fail, so T4 disappears from the counters and the suite still reports success. The other tests (T1, T3a, T3b, T5) always record a result.

Split the two assertions like T2 and T3a do.

♻️ Proposed change
-[ "$rc" -eq 0 ] && case "$out" in *absent*|*ABSENT*|*"not trustworthy"*) pass "T4 corrupt reported";; *) fail "T4 corrupt silent: $out";; esac
+[ "$rc" -eq 0 ] || fail "T4 corrupt rc=$rc"
+case "$out" in *absent*|*ABSENT*|*"not trustworthy"*) pass "T4 corrupt reported";; *) fail "T4 corrupt silent: $out";; esac
🤖 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 `@scripts/hooks/test-graphify-freshness-advisory.sh` around lines 52 - 55,
Update T4 in the test script to record a result for every hook exit status:
separate the return-code assertion from the output-banner case assertion,
following the pattern used by T2 and T3a. Ensure a non-zero rc calls fail rather
than bypassing both pass and fail, while preserving the existing
corrupt-manifest output checks.
scripts/hooks/wire-hook-bash.mjs (1)

212-263: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: anchor the SessionStart array lookup to the hooks object.

Line 213 takes the first textual match of /"SessionStart"\s*:\s*\[/ anywhere in the file. A "SessionStart" key that appears before hooks.SessionStart (for example inside another top-level object) would select the wrong span. The downstream re-parse, inventory re-check, and the two security gates make a wrong span fail loudly instead of corrupting the file, so this is a robustness point, not a defect.

If you keep the textual approach, consider locating "hooks" first and searching for "SessionStart" after that index.

🤖 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 `@scripts/hooks/wire-hook-bash.mjs` around lines 212 - 263, Update
installMissingEntries so the SessionStart array lookup is scoped to the hooks
object rather than matching the first "SessionStart" key anywhere in the
settings text. Locate the hooks object boundary first, then search for the
SessionStart array within that scope while preserving the existing missing-key
and balance validation.
🤖 Prompt for all review comments with 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.

Inline comments:
In @.claude/commands/graph-refresh.md:
- Line 33: Update the --vault documentation near the usage examples to state
that the vault must be at or below the resolved luna root, and that an external
vault requires setting LUNA_VAULT_PATH to ratify it. Apply the same
clarification to the corresponding repeated entry.

In `@scripts/cr/artifact-critic.sh`:
- Around line 54-63: Update the case condition in artifact-critic.sh to match
only the explicitly supported Z.ai/GLM model or slug identities, rather than
arbitrary substrings; preserve credential loading for valid lanes and prevent it
for values such as “notglm”. In scripts/cr/test-artifact-critic.sh, add coverage
for a non-GLM model or slug containing “glm” or “zai” and assert that
GLM_API_KEY remains unset.

In `@scripts/graphify/graph-refresh.sh`:
- Around line 301-304: Update the HOME comparison in the preflight check around
_normalize and preflight_refuse to compare trailing-slash-stripped paths
case-insensitively by applying _lc to both home_canon and ap. Preserve the
existing refusal when the normalized vault path equals the operator HOME.

In `@scripts/graphify/test-graph-refresh.sh`:
- Around line 440-452: Update the Test 15 setup around GOOD_REAL/GOOD_LINK to
configure TEST_LUNA_ROOT for the resolved target before invoking run_refresh,
matching the established pattern used by Tests 8 and 12b. Ensure the symlinked
vault remains within the configured luna root so preflight_vault accepts it and
the existing rc=0 and single-leg assertions remain valid.

In `@scripts/handover/arm-resume.sh`:
- Around line 1127-1131: Update the _fm_rc handling in arm-resume so every
nonzero awk status is treated as a fatal frontmatter read/parse failure, while
preserving the existing specific message for status 3. Exit before extracting
_raw or continuing with an empty _fm, ensuring _ho_ticket_strict cannot remain
empty after any awk failure.
- Line 1126: Update the awk program in the frontmatter extraction assignment to
construct the UTF-8 BOM with sprintf("%c%c%c",239,187,191) before processing,
then remove the non-portable \x BOM regex and use the constructed value for the
first-line substitution. Preserve the existing frontmatter parsing and
error-status behavior.

In `@scripts/handover/test-arm-resume.sh`:
- Around line 3695-3696: Replace the non-portable `touch -d '1 day ago'`
invocation for `SIB_RECENT_1606` with a computed timestamp passed to `touch -t`,
preserving the file’s intended age of one day across macOS, Linux, and Windows
Git Bash.

In `@scripts/hooks/graphify-freshness-advisory.sh`:
- Around line 43-45: Validate budget in the hook before invoking
check-graph-freshness.sh: accept only an integer GRAPHIFY_STALENESS_MAX_AGE_DAYS
value and reset invalid or non-integer input to the existing default of 2. Apply
this validation to the budget setup and the corresponding checker invocation
path so invalid configuration remains fail-open and does not populate $out with
usage errors.

In `@scripts/observability/wire-session-telemetry-hooks.mjs`:
- Around line 102-110: Update isWriterCommand and isOurs to recognize only
actual writer invocations using the expected bun or bun run argument form and
the settings-project writer path, rather than any command containing the path
suffix. Preserve verb matching while excluding commands such as echo that merely
contain the writer path and verb as text, and add a regression case covering
that foreign command.

---

Nitpick comments:
In `@scripts/graphify/test-graph-refresh.sh`:
- Around line 374-376: Strengthen the ratified run in test-graph-refresh.sh by
adding the same runner-log assertion used by Test 15 after the existing rc 0
check. Verify that the luna leg actually fired, reusing the established log
assertion and expected invocation shape.
- Around line 15-31: Extend the top-level “Covers:” list in the test script from
nine entries to fifteen, documenting the preflight cases for salus marker,
phi-roots, root/HOME, containment, drive-relative paths, sentinels, symlink
refusal, and symlink acceptance. Keep the existing cases unchanged and ensure
the list accurately matches all tests executed by the script.

In `@scripts/hooks/test-graphify-freshness-advisory.sh`:
- Around line 52-55: Update T4 in the test script to record a result for every
hook exit status: separate the return-code assertion from the output-banner case
assertion, following the pattern used by T2 and T3a. Ensure a non-zero rc calls
fail rather than bypassing both pass and fail, while preserving the existing
corrupt-manifest output checks.

In `@scripts/hooks/wire-hook-bash.mjs`:
- Around line 212-263: Update installMissingEntries so the SessionStart array
lookup is scoped to the hooks object rather than matching the first
"SessionStart" key anywhere in the settings text. Locate the hooks object
boundary first, then search for the SessionStart array within that scope while
preserving the existing missing-key and balance validation.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e6e8d9e8-3d8f-475b-9faa-dce63dbacfdf

📥 Commits

Reviewing files that changed from the base of the PR and between 829c41c and 4b26d7d.

⛔ Files ignored due to path filters (1)
  • marketplace/plugins/obsidian-triage/tools/package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json
📒 Files selected for processing (29)
  • .claude/commands/graph-refresh.md
  • docs/commands-catalog.md
  • docs/glm-offload.md
  • marketplace/plugins/obsidian-triage/tools/package.json
  • scripts/cr/artifact-critic.sh
  • scripts/cr/critic-panel.sh
  • scripts/cr/ledger-append.sh
  • scripts/cr/test-artifact-critic.sh
  • scripts/cr/test-clear-cr-marker.sh
  • scripts/cr/test-critic-panel.sh
  • scripts/cr/test-ledger-append.sh
  • scripts/graphify/graph-refresh.sh
  • scripts/graphify/refresh-graph-map.sh
  • scripts/graphify/test-graph-refresh.sh
  • scripts/graphify/test-refresh-graph-map.sh
  • scripts/handover/arm-resume.sh
  • scripts/handover/test-arm-resume.sh
  • scripts/hooks/graphify-freshness-advisory.sh
  • scripts/hooks/test-graphify-freshness-advisory.sh
  • scripts/hooks/wire-hook-bash.mjs
  • scripts/hooks/wire-hook-bash.test.mjs
  • scripts/lanes/await-glm-worker.sh
  • scripts/lanes/tests/verify-return.test.mjs
  • scripts/lanes/verify-return.mjs
  • scripts/observability/provisioning/alerting/rules.yaml
  • scripts/observability/wire-session-telemetry-hooks.mjs
  • scripts/observability/wire-session-telemetry-hooks.test.mjs
  • scripts/telegram/spawn-glm.test.ts
  • scripts/telegram/spawn-glm.ts

- `/graph-refresh luna` — refresh only the luna graph.
- `/graph-refresh himmel` — refresh only the himmel graph.
- `/graph-refresh both --dry-run` — preview the exact `refresh-graph-map.sh` invocations without running them.
- `/graph-refresh --vault /path/to/vault` — override the vault root (default: `$LUNA_VAULT_PATH` if set, else `<user-profile>/Documents/luna`).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the configured-root containment rule for --vault.

graph-refresh.sh refuses a luna-leg vault that is not at or under the resolved configured luna root (LUNA_VAULT_PATH, else <home>/Documents/luna). See preflight_vault step (e). The example on Line 33 therefore fails with rc=2 for any path outside that root, which is the common case for an arbitrary /path/to/vault. An operator following this line gets a refusal with no explanation here.

State that a vault outside the configured root must be ratified by setting LUNA_VAULT_PATH.

📝 Proposed doc fix
-- `/graph-refresh --vault /path/to/vault` — override the vault root (default: `$LUNA_VAULT_PATH` if set, else `<user-profile>/Documents/luna`).
+- `/graph-refresh --vault /path/to/vault` — override the vault root (default: `$LUNA_VAULT_PATH` if set, else `<user-profile>/Documents/luna`). On the luna leg the override must resolve at/under the configured luna root; set `LUNA_VAULT_PATH` to ratify a different location.
-- `LUNA_VAULT_PATH=<path>` — default vault root when `--vault` is not passed.
+- `LUNA_VAULT_PATH=<path>` — default vault root when `--vault` is not passed, and the configured root the luna leg's `--vault` must sit at/under.

Also applies to: 46-46

🤖 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 @.claude/commands/graph-refresh.md at line 33, Update the --vault
documentation near the usage examples to state that the vault must be at or
below the resolved luna root, and that an external vault requires setting
LUNA_VAULT_PATH to ratify it. Apply the same clarification to the corresponding
repeated entry.

Comment thread scripts/cr/artifact-critic.sh
Comment on lines +301 to +304
home_canon="$(_normalize "$(_abs "$(resolve_user_home)")")"
if [ "$ap" = "$home_canon" ]; then
preflight_refuse "vault is the operator HOME ($ap)"
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Compare the HOME path case-insensitively.

Every other classification in this preflight lowercases both sides (_under_root calls _lc). This check does a raw string compare. _normalize preserves the drive-letter case, so C:/Users/x and c:/Users/x are unequal here while they name the same directory. On Windows the two forms are both reachable: resolve_user_home returns the cygpath -u form, and --vault can be typed in the drive-rooted form. The HOME refusal then does not fire.

Reuse _lc for both sides, and compare the trailing-slash-stripped forms.

🛡️ Proposed fix
-    home_canon="$(_normalize "$(_abs "$(resolve_user_home)")")"
-    if [ "$ap" = "$home_canon" ]; then
+    home_canon="$(_lc "$(_normalize "$(_abs "$(resolve_user_home)")")")"
+    if [ "$(_lc "$ap")" = "$home_canon" ]; then
         preflight_refuse "vault is the operator HOME ($ap)"
     fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
home_canon="$(_normalize "$(_abs "$(resolve_user_home)")")"
if [ "$ap" = "$home_canon" ]; then
preflight_refuse "vault is the operator HOME ($ap)"
fi
home_canon="$(_lc "$(_normalize "$(_abs "$(resolve_user_home)")")")"
if [ "$(_lc "$ap")" = "$home_canon" ]; then
preflight_refuse "vault is the operator HOME ($ap)"
fi
🤖 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 `@scripts/graphify/graph-refresh.sh` around lines 301 - 304, Update the HOME
comparison in the preflight check around _normalize and preflight_refuse to
compare trailing-slash-stripped paths case-insensitively by applying _lc to both
home_canon and ap. Preserve the existing refusal when the normalized vault path
equals the operator HOME.

Comment thread scripts/graphify/test-graph-refresh.sh
Comment thread scripts/handover/arm-resume.sh
Comment thread scripts/handover/arm-resume.sh
Comment on lines +3695 to +3696
touch -t 200001010000 "$SIB_STALE_1606" # >7 days old -- must be pruned
touch -d '1 day ago' "$SIB_RECENT_1606" # 1 day old -- must survive

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Replace touch -d '1 day ago' with a portable form.

touch -d with a relative date string is a GNU coreutils extension. BSD touch on macOS rejects -d '1 day ago', so SIB_RECENT_1606 keeps the current time or the command fails. The suite targets macOS, Linux, and Windows Git Bash.

touch -t with a computed stamp works on all three.

🛠️ Portable timestamp
 touch -t 200001010000 "$SIB_STALE_1606"   # >7 days old -- must be pruned
-touch -d '1 day ago' "$SIB_RECENT_1606"   # 1 day old -- must survive
+# 1 day old -- must survive. `touch -d '1 day ago'` is GNU-only; compute the
+# stamp with python3 (already required by future_time) and pass it to -t.
+touch -t "$(python3 -c 'import datetime;print((datetime.datetime.now()-datetime.timedelta(days=1)).strftime("%Y%m%d%H%M"))')" \
+    "$SIB_RECENT_1606"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
touch -t 200001010000 "$SIB_STALE_1606" # >7 days old -- must be pruned
touch -d '1 day ago' "$SIB_RECENT_1606" # 1 day old -- must survive
touch -t 200001010000 "$SIB_STALE_1606" # >7 days old -- must be pruned
# 1 day old -- must survive. `touch -d '1 day ago'` is GNU-only; compute the
# stamp with python3 (already required by future_time) and pass it to -t.
touch -t "$(python3 -c 'import datetime;print((datetime.datetime.now()-datetime.timedelta(days=1)).strftime("%Y%m%d%H%M"))')" \
"$SIB_RECENT_1606"
🤖 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 `@scripts/handover/test-arm-resume.sh` around lines 3695 - 3696, Replace the
non-portable `touch -d '1 day ago'` invocation for `SIB_RECENT_1606` with a
computed timestamp passed to `touch -t`, preserving the file’s intended age of
one day across macOS, Linux, and Windows Git Bash.

Source: Path instructions

Comment on lines +43 to +45
# 2 days = tolerate one missed daily run (pairs with the qmd advisory's 36h
# philosophy on the checker's day-granular interface).
budget="${GRAPHIFY_STALENESS_MAX_AGE_DAYS:-2}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate GRAPHIFY_STALENESS_MAX_AGE_DAYS before you pass it to the checker.

check-graph-freshness.sh uses exit code 1 for both "stale" and "usage error". It rejects a non-integer --max-age-days with exit 1. If an operator exports a non-numeric GRAPHIFY_STALENESS_MAX_AGE_DAYS, the hook maps that usage error to state="STALE" and prints the banner in every session, even when the graph is fresh. The checker's usage text also lands in $out.

Validate the budget in the hook and fall back to the default. The hook's own misconfiguration then stays in the fail-open class.

🛠️ Proposed fix
 budget="${GRAPHIFY_STALENESS_MAX_AGE_DAYS:-2}"
+case "$budget" in
+    ''|*[!0-9]*) budget=2 ;;   # our own bad input -> default, never a false STALE banner
+esac

Also applies to: 60-68

🤖 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 `@scripts/hooks/graphify-freshness-advisory.sh` around lines 43 - 45, Validate
budget in the hook before invoking check-graph-freshness.sh: accept only an
integer GRAPHIFY_STALENESS_MAX_AGE_DAYS value and reset invalid or non-integer
input to the existing default of 2. Apply this validation to the budget setup
and the corresponding checker invocation path so invalid configuration remains
fail-open and does not populate $out with usage errors.

Comment thread scripts/observability/wire-session-telemetry-hooks.mjs
…pinned .env registry fixture + ratified symlink-accept vault

Cures the shell-unit (ubuntu-latest) red on this PR: the
test-critic-panel-registry.sh T1 fixture now plants its .env in a fake
himmel root (the HIMMEL-1648 script-root pinning correctly stopped reading
the cwd copy), and the test-graph-refresh.sh symlink-ACCEPT case (Linux-only;
skips on Windows) ratifies the symlinked vault as the configured root via
TEST_LUNA_ROOT per the HIMMEL-1644 r3 containment; the symlink-REFUSE case
pins the salus message. No assertion weakened. Private #1643.

Platforms tested: windows
Security reviewed: ad-hoc
@yotamleo

yotamleo commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant