Workflow: add Hermes review gates - #3
Conversation
Add public-safe Hermes/OpenClaw review gates, shadow promotion schema, and local shadow collector templates.
|
@codex review |
Disable shadow Telegram delivery by default and add a preview/apply/read-back hardening script for existing local cron jobs.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e820095872
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Prefer fewer promotions. | ||
| - Every promotion needs evidence paths. | ||
| - Archive/delete candidates are only candidates, never actions. | ||
| - Any `delete_candidate` must include a safer archive alternative. |
There was a problem hiding this comment.
The prompt instructs delete_candidate while the schema requires the top-level array delete_candidates; this key drift can cause model outputs to fail validation or be dropped by downstream parsers that expect the schema contract exactly. Given this repo’s review rules for prompt/schema changes, the naming should be made consistent to avoid production workflow breakage.
Useful? React with 👍 / 👎.
|
|
||
| const namesToRemove = new Set(jobsToInstall.flatMap((job) => [job.name, ...job.legacyNames])); | ||
|
|
||
| const filteredJobs = parsed.jobs.filter((job) => !namesToRemove.has(job?.name)); |
There was a problem hiding this comment.
The filter removes existing jobs solely by name, so any unrelated cron job that happens to share one of these names will be deleted and replaced. This can silently clobber another agent/profile’s configuration; the removal criteria should include the expected Hermes agent identity (or another ownership marker), not just job.name.
Useful? React with 👍 / 👎.
| <array> | ||
| <string>/bin/zsh</string> | ||
| <string>-lc</string> | ||
| <string>cd "${repo_root_xml}" && HERMES_VAULT_ROOT="${vault_root_xml}" "${node_bin_xml}" output/hermes-codex-runtime/collect-hermes-shadow.mjs --days=7 --vault-root="${vault_root_xml}" --apply</string> |
There was a problem hiding this comment.
The LaunchAgent command is executed through zsh -lc with interpolated path/env values, so shell expansions (for example $VAR/$(...)) inside those values are evaluated at runtime. If REPO_ROOT, VAULT_ROOT, or similar inputs contain shell metacharacters, this can execute unintended commands or break execution; prefer direct ProgramArguments without shell parsing or robust shell escaping.
Useful? React with 👍 / 👎.
|
|
||
| writeJson(outputFile, plan); | ||
|
|
||
| if (apply && freeze.length > 0) { |
There was a problem hiding this comment.
The --apply path immediately mutates jobs.json without checking any approval token/status, so a single accidental invocation can disable live planner/orchestrator jobs. This contradicts the repo-safe gate contract added in the same change (preview → explicit approval → apply → read-back), so apply should be blocked unless an explicit approval artifact is present and validated.
Useful? React with 👍 / 👎.
| if (!Array.isArray(content)) { | ||
| return ""; |
There was a problem hiding this comment.
parseTextParts returns an empty string whenever message.content is not an array, but OpenClaw transcripts can store message content as plain strings. In those sessions the collector drops user/assistant text, which misclassifies runs and undercounts shadow evidence days, potentially blocking promotion decisions even when valid evidence exists.
Useful? React with 👍 / 👎.
| external_writes: ["OpenClaw cron jobs.json"], | ||
| safety: { | ||
| preview_file: planPath, | ||
| approval_id: "operator-requested-shadow-stabilization", |
There was a problem hiding this comment.
The script records a fixed approval_id string and then performs writes whenever --apply is set, without verifying that a real operator approval was provided for this run. Because this job changes OpenClaw cron state, approval metadata must be supplied and validated per invocation; otherwise the gate is effectively bypassed.
Useful? React with 👍 / 👎.
| const filteredJobs = parsed.jobs.filter((job) => !namesToRemove.has(job?.name)); | ||
|
|
||
| const newJobs = jobsToInstall.map((job) => ({ | ||
| id: crypto.randomUUID(), |
There was a problem hiding this comment.
The script always recreates matching jobs with crypto.randomUUID(), which discards prior state and changes each job’s identity on every rerun. Cron scheduling uses job.id as the stable hash input for stagger offsets, so this ID churn can shift run timing unexpectedly and reset operational history (for example consecutive error tracking) even when the logical jobs are unchanged.
Useful? React with 👍 / 👎.
| parsed.jobs = nextJobs; | ||
| await fs.writeFile(jobsPath, `${JSON.stringify(parsed, null, 2)}\n`, "utf8"); | ||
| result.backupPath = backupPath; | ||
| result.read_back = JSON.parse(await fs.readFile(jobsPath, "utf8")) |
There was a problem hiding this comment.
After --apply, the script records a read_back snapshot but never compares it to expected_changes and never flips repair_required from false on mismatch/omission. That means partial or ineffective writes can still be reported as successful hardening, despite the workflow contract requiring repair-needed status when read-back is incomplete.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds a clean, public-safe Hermes/OpenClaw workflow gate set for Codex review validation:
Safety Notes
Test Plan