diff --git a/cron/jobs.py b/cron/jobs.py index 904b8aa34626..2d3c19a08cb1 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -1261,6 +1261,7 @@ def create_job( workdir: Optional[str] = None, no_agent: bool = False, attach_to_session: Optional[bool] = None, + profile: Optional[str] = None, ) -> Dict[str, Any]: """ Create a new cron job. @@ -1426,6 +1427,7 @@ def create_job( "origin": origin, # Tracks where job was created for "origin" delivery "enabled_toolsets": normalized_toolsets, "workdir": normalized_workdir, + "profile": str(profile).strip() if isinstance(profile, str) and profile.strip() else None, } # Only persist attach_to_session when explicitly set, so existing jobs and # the common case stay byte-identical (absent key => fall back to the @@ -1531,6 +1533,11 @@ def update_job(job_id: str, updates: Dict[str, Any]) -> Optional[Dict[str, Any]] updates["workdir"] = _normalize_workdir(_wd) previous_inference_axes = _normalized_inference_axes(job) + + # Normalize profile — empty string clears the field. + if "profile" in updates: + _p = updates["profile"] + updates["profile"] = str(_p).strip() if isinstance(_p, str) and _p.strip() else None updated = _apply_skill_fields({**job, **updates}) schedule_changed = "schedule" in updates inference_fields_changed = bool( @@ -1738,8 +1745,19 @@ def mark_job_run(job_id: str, success: bool, error: Optional[str] = None, # Check if we've hit the repeat limit if times is not None and times > 0 and completed >= times: - # Remove the job (limit reached) - jobs.pop(i) + # Limit reached: retain the record as a terminal + # completion instead of popping it. Deleting the job + # here discarded the last_status / last_error / + # last_delivery_error written above — a finished + # one-shot vanished from `cronjob list` with no + # inspectable outcome, and a failed delivery was + # invisible. Mirror the terminal shape of the + # next_run_at-is-None branch below; the retention + # sweep prunes these after + # COMPLETED_ONESHOT_RETENTION_DAYS. + job["enabled"] = False + job["state"] = "completed" + job["next_run_at"] = None save_jobs(jobs) return @@ -1857,13 +1875,29 @@ def claim_dispatch(job_id: str) -> bool: return True # infinite — always dispatch completed = repeat.get("completed", 0) if completed >= times: - # Already dispatched the max number of times (e.g. a prior - # tick claimed then died before mark_job_run could remove it). - # Clean up so it stops appearing as due on every tick. + # Already dispatched the max number of times. + if job.get("last_run_at") is not None: + # A prior run completed normally (e.g. mark_job_run raced + # with this tick). Retain the terminal record — same shape + # as mark_job_run's repeat-limit branch — instead of + # deleting the job and its final status/delivery error. + job["enabled"] = False + job["state"] = "completed" + job["next_run_at"] = None + save_jobs(jobs) + logger.info( + "Job '%s': dispatch limit reached (%d/%d) — marking completed", + job.get("name", job.get("id", "?")), + completed, + times, + ) + return False + # A prior tick claimed the dispatch then died before the run + # completed (#73973) — a genuinely wedged claim. Remove it so + # it stops appearing as due, and leave an operator-visible + # diagnostic instead of vanishing silently. jobs.pop(i) save_jobs(jobs) - # If the claimed run never completed (#73973), leave an - # operator-visible diagnostic instead of vanishing silently. _write_wedged_oneshot_diagnostic(job) logger.info( "Job '%s': dispatch limit reached (%d/%d) — removing", @@ -2049,6 +2083,82 @@ def claim_job_for_fire(job_id: str, *, claim_ttl_seconds: int = 300) -> bool: return False +# Completed one-shot job records are retained in jobs.json (final status + +# delivery error stay inspectable via `cronjob list`) instead of being deleted +# at completion, then pruned by _sweep_completed_oneshots once they age out. +COMPLETED_ONESHOT_RETENTION_DAYS = 7 + + +def _completed_oneshot_retention_days() -> float: + """Resolve the completed one-shot retention window from config. + + ``cron.completed_retention_days`` (number, default + ``COMPLETED_ONESHOT_RETENTION_DAYS``). A non-positive value disables the + sweep, retaining completed one-shot records indefinitely. + """ + try: + from hermes_cli.config import load_config + cfg = load_config() or {} + cron_cfg = cfg.get("cron", {}) if isinstance(cfg, dict) else {} + return float( + cron_cfg.get( + "completed_retention_days", COMPLETED_ONESHOT_RETENTION_DAYS + ) + ) + except Exception: + return float(COMPLETED_ONESHOT_RETENTION_DAYS) + + +def _sweep_completed_oneshots(raw_jobs: List[Dict[str, Any]], now: datetime) -> bool: + """Prune terminal ``state == "completed"`` one-shot records past retention. + + Mutates *raw_jobs* in place; returns True when anything was removed (the + caller persists). Only one-shot (``schedule.kind == "once"``) records in + the terminal completed state are candidates; recurring jobs and non- + terminal one-shots are never touched. Age is measured from + ``last_run_at`` — a completed record without a parseable ``last_run_at`` + is kept (never guess a record into deletion). + """ + retention_days = _completed_oneshot_retention_days() + if retention_days <= 0: + return False + cutoff = now - timedelta(days=retention_days) + removed = False + for rj in list(raw_jobs): + try: + if rj.get("state") != "completed": + continue + schedule = rj.get("schedule") + kind = schedule.get("kind") if isinstance(schedule, dict) else None + if kind != "once": + continue + last_run = rj.get("last_run_at") + if not isinstance(last_run, str): + continue + try: + last_run_dt = _ensure_aware(datetime.fromisoformat(last_run)) + except Exception: + continue + if last_run_dt >= cutoff: + continue + raw_jobs.remove(rj) + removed = True + logger.info( + "Job '%s': pruning completed one-shot record " + "(finished %s, retention %.1f days)", + rj.get("name", rj.get("id", "?")), + last_run, + retention_days, + ) + except Exception: + logger.debug( + "Retention sweep skipped malformed job record %r", + rj.get("id", "?"), + exc_info=True, + ) + return removed + + def get_due_jobs() -> List[Dict[str, Any]]: """Get all jobs that are due to run now. @@ -2168,6 +2278,15 @@ def _get_due_jobs_locked() -> List[Dict[str, Any]]: # (derived from HERMES_CRON_TIMEOUT). See _oneshot_run_claim_ttl_seconds. _run_claim_ttl = _oneshot_run_claim_ttl_seconds() + # Retention sweep: completed one-shots are retained (so their final + # status / delivery error stay inspectable via `cronjob list`) instead of + # being deleted on completion, but they must not accumulate in jobs.json + # forever. Prune terminal one-shot records older than the retention + # window each scan. + if _sweep_completed_oneshots(raw_jobs, now): + needs_save = True + jobs = [j for j in jobs if any(rj.get("id") == j.get("id") for rj in raw_jobs)] + for job in jobs: # Per-job containment (structural guard): one malformed or # unexpected job record must never abort the whole scan. The id / diff --git a/hermes_cli/cli_commands_mixin.py b/hermes_cli/cli_commands_mixin.py index 69b5323b5616..55302cf8a65d 100644 --- a/hermes_cli/cli_commands_mixin.py +++ b/hermes_cli/cli_commands_mixin.py @@ -1446,6 +1446,7 @@ def _parse_flags(tokens): "all": False, "prompt": None, "schedule": None, + "profile": None, "positionals": [], } i = 0 @@ -1485,6 +1486,9 @@ def _parse_flags(tokens): elif token == "--schedule" and i + 1 < len(tokens): opts["schedule"] = tokens[i + 1] i += 2 + elif token == "--profile" and i + 1 < len(tokens): + opts["profile"] = tokens[i + 1] + i += 2 else: opts["positionals"].append(token) i += 1 @@ -1577,6 +1581,7 @@ def _parse_flags(tokens): deliver=opts["deliver"], repeat=opts["repeat"], skills=skills or None, + profile=opts["profile"], ) if result.get("success"): print(f"(^_^)b Created job: {result['job_id']}") @@ -1623,6 +1628,7 @@ def _parse_flags(tokens): deliver=opts["deliver"], repeat=opts["repeat"], skills=final_skills, + profile=opts["profile"], ) if result.get("success"): job = result["job"] diff --git a/hermes_cli/cron.py b/hermes_cli/cron.py index c5be7681c936..d4549fd2479f 100644 --- a/hermes_cli/cron.py +++ b/hermes_cli/cron.py @@ -163,6 +163,9 @@ def cron_list(show_all: bool = False): workdir = job.get("workdir") if workdir: print(f" Workdir: {workdir}") + profile_name = job.get("profile") + if profile_name: + print(f" Profile: {profile_name}") # Execution history last_status = job.get("last_status") @@ -352,6 +355,7 @@ def cron_create(args): model=getattr(args, "model", None), provider=getattr(args, "model_provider", None), no_agent=getattr(args, "no_agent", False) or None, + profile=getattr(args, "profile", None), ) if not result.get("success"): print(color(f"Failed to create job: {result.get('error', 'unknown error')}", Colors.RED)) @@ -368,6 +372,8 @@ def cron_create(args): print(" Mode: no-agent (script stdout delivered directly)") if job_data.get("workdir"): print(f" Workdir: {job_data['workdir']}") + if job_data.get("profile"): + print(f" Profile: {job_data['profile']}") print(f" Next run: {result['next_run_at']}") _warn_if_gateway_not_running() return 0 @@ -417,6 +423,7 @@ def cron_edit(args): model=getattr(args, "model", None), provider=getattr(args, "model_provider", None), no_agent=getattr(args, "no_agent", None), + profile=getattr(args, "profile", None), ) if not result.get("success"): print(color(f"Failed to update job: {result.get('error', 'unknown error')}", Colors.RED)) @@ -436,6 +443,8 @@ def cron_edit(args): print(" Mode: no-agent (script stdout delivered directly)") if updated.get("workdir"): print(f" Workdir: {updated['workdir']}") + if updated.get("profile"): + print(f" Profile: {updated['profile']}") return 0 diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 98874824c196..793aa4cc6b9e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -566,9 +566,25 @@ def _resolve_sudo_user_profile_env(name: str) -> str | None: return None return None + def _inside_cron_edit(index: int) -> bool: + """True once argv reaches ``cron edit`` sub-subcommand. + + ``cron edit`` has its own ``--profile`` argument that sets the job's + profile field — it does NOT mean "switch the active profile context". + So we must NOT consume ``--profile`` after ``cron edit``. + """ + try: + cron_idx = argv.index("cron", 0, index) + argv.index("edit", cron_idx + 1, index) + except ValueError: + return False + return True + # 1. Check for explicit -p / --profile flag. Historically this worked even # after the subcommand (`hermes chat -p coder`), so keep scanning broadly. - # The exception is command-argv passthrough regions such as `mcp add --args`. + # The exception is command-argv passthrough regions such as `mcp add --args` + # and ``cron edit`` which uses ``--profile`` as a job field. + # Exempt ``cron edit`` — ``--profile`` there sets the job's profile field. value_flags = { "-z", "--oneshot", "-m", "--model", @@ -586,12 +602,15 @@ def _resolve_sudo_user_profile_env(name: str) -> str | None: break if arg == "--args" and _inside_mcp_add_args(i): break - if arg in {"--profile", "-p"} and i + 1 < len(argv): + # Don't consume --profile after ``cron edit`` — it sets the job's + # profile field, not the active Hermes profile context. + if arg in {"--profile", "-p"} and i + 1 < len(argv) and not _inside_cron_edit(i): profile_name = argv[i + 1] consume = 2 profile_index = i break - if arg.startswith("--profile="): + # Also handle ``--profile=value`` form — same cron edit exception. + if arg.startswith("--profile=") and not _inside_cron_edit(i): profile_name = arg.split("=", 1)[1] consume = 1 profile_index = i diff --git a/hermes_cli/subcommands/cron.py b/hermes_cli/subcommands/cron.py index 331176116c3f..d0f0bae3c94b 100644 --- a/hermes_cli/subcommands/cron.py +++ b/hermes_cli/subcommands/cron.py @@ -68,7 +68,11 @@ def build_cron_parser(subparsers, *, cmd_cron: Callable) -> None: ) cron_create.add_argument( "--workdir", - help="Absolute path for the job to run from. Injects AGENTS.md / CLAUDE.md / .cursorrules from that directory and uses it as the cwd for terminal/file/code_exec tools. Omit to preserve old behaviour (no project context files).", + help="Absolute path for the job to run from (injects AGENTS.md etc. and sets terminal cwd). Omit to preserve old behaviour (no project context files).", + ) + cron_create.add_argument( + "--profile", + help="Assign this job to a specific profile. The job's cron data (db, output) stays in the profile's home.", ) cron_create.add_argument( "--model", @@ -160,6 +164,10 @@ def build_cron_parser(subparsers, *, cmd_cron: Callable) -> None: dest="model_provider", help="Inference provider paired with --model. Pass empty string to clear.", ) + cron_edit.add_argument( + "--profile", + help="Assign this job to a specific profile. The job's cron data (db, output) stays in the profile's home. Pass empty string to clear or omit to keep current.", + ) # lifecycle actions cron_pause = cron_subparsers.add_parser("pause", help="Pause a scheduled job") diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index e9402208e3cc..b0121daed1e0 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -323,11 +323,45 @@ def test_increments_completed(self, tmp_cron_dir): assert updated["repeat"]["completed"] == 1 assert updated["last_status"] == "ok" - def test_repeat_limit_removes_job(self, tmp_cron_dir): + def test_repeat_limit_retains_completed_record(self, tmp_cron_dir): + """A finished one-shot must stay inspectable, not vanish from the store.""" job = create_job(prompt="Once", schedule="30m", repeat=1) mark_job_run(job["id"], success=True) - # Job should be removed after hitting repeat limit - assert get_job(job["id"]) is None + updated = get_job(job["id"]) + assert updated is not None, "completed one-shot was deleted from jobs.json" + assert updated["state"] == "completed" + assert updated["enabled"] is False + assert updated["next_run_at"] is None + assert updated["last_status"] == "ok" + + def test_repeat_limit_retains_delivery_error(self, tmp_cron_dir): + """A one-shot whose delivery failed must keep the error on its record.""" + job = create_job(prompt="Once", schedule="30m", repeat=1) + mark_job_run( + job["id"], success=True, + delivery_error="platform 'telegram' not configured", + ) + updated = get_job(job["id"]) + assert updated is not None + assert updated["state"] == "completed" + assert updated["last_delivery_error"] == "platform 'telegram' not configured" + + def test_completed_oneshot_visible_in_list(self, tmp_cron_dir): + """list_jobs(include_disabled=True) surfaces the completed record.""" + job = create_job(prompt="Once", schedule="30m", repeat=1) + mark_job_run(job["id"], success=True, delivery_error="send failed: 502") + listed = {j["id"]: j for j in list_jobs(include_disabled=True)} + assert job["id"] in listed + assert listed[job["id"]]["state"] == "completed" + assert listed[job["id"]]["last_delivery_error"] == "send failed: 502" + # Default (enabled-only) listing hides it, matching paused/disabled jobs. + assert job["id"] not in {j["id"] for j in list_jobs()} + + def test_completed_oneshot_not_due(self, tmp_cron_dir): + """A retained completed one-shot must never be dispatched again.""" + job = create_job(prompt="Once", schedule="30m", repeat=1) + mark_job_run(job["id"], success=True) + assert job["id"] not in {j["id"] for j in get_due_jobs()} def test_error_status(self, tmp_cron_dir): @@ -639,9 +673,14 @@ def test_run_claim_heartbeat_keeps_long_run_claimed_past_ttl( assert get_job("slowrun") is not None # Run completes → outcome lands on a record that still exists - # (times=1 reached, so mark_job_run retires the job normally). + # (times=1 reached, so mark_job_run retires the job as a terminal + # completed record instead of deleting it). mark_job_run("slowrun", True) - assert get_job("slowrun") is None + retired = get_job("slowrun") + assert retired is not None + assert retired["state"] == "completed" + assert retired["enabled"] is False + assert retired["last_status"] == "ok" def test_heartbeat_run_claim_rejects_replaced_owner(self, tmp_cron_dir): @@ -900,12 +939,17 @@ def test_already_dispatched_oneshot_is_removed(self, tmp_cron_dir): def test_mark_job_run_does_not_double_count_preclaimed_oneshot(self, tmp_cron_dir): # Full lifecycle: claim bumps completed to times, then mark_job_run must - # NOT increment again — it recognizes the pre-claim and removes the job. + # NOT increment again — it recognizes the pre-claim and retires the job + # as a terminal completed record (retained for inspection, not re-fired). save_jobs([self._oneshot(times=1, completed=0)]) assert claim_dispatch("os1") is True assert load_jobs()[0]["repeat"]["completed"] == 1 mark_job_run("os1", success=True) - assert load_jobs() == [] # completed once, removed — not fired twice + retired = load_jobs() + assert len(retired) == 1 # completed once, retired — not fired twice + assert retired[0]["repeat"]["completed"] == 1 # no double count + assert retired[0]["state"] == "completed" + assert retired[0]["enabled"] is False def test_get_due_jobs_removes_stale_maxed_oneshot(self, tmp_cron_dir): @@ -1139,3 +1183,70 @@ def test_wrapper_semantics_unchanged(self, tmp_cron_dir): assert advance_next_run(rec_ids[0]) is True assert advance_next_run(one_ids[0]) is False assert advance_next_run("missing-id") is False + + +# ========================================================================= +# Completed one-shot retention sweep +# ========================================================================= + +class TestCompletedOneshotRetentionSweep: + """Completed one-shots are retained for inspection, then pruned by age.""" + + def _completed_oneshot(self, age_days: float): + """Create a one-shot, complete it, and backdate its last_run_at.""" + job = create_job(prompt="Once", schedule="30m", repeat=1) + mark_job_run(job["id"], success=True, delivery_error="boom") + stamp = ( + datetime.now(timezone.utc) - timedelta(days=age_days) + ).isoformat() + jobs = load_jobs() + for j in jobs: + if j["id"] == job["id"]: + j["last_run_at"] = stamp + save_jobs(jobs) + return job["id"] + + def test_sweep_prunes_old_completed_oneshot(self, tmp_cron_dir): + old_id = self._completed_oneshot(age_days=30) + get_due_jobs() # sweep runs as part of the due scan + assert get_job(old_id) is None + + def test_sweep_keeps_recent_completed_oneshot(self, tmp_cron_dir): + recent_id = self._completed_oneshot(age_days=1) + get_due_jobs() + kept = get_job(recent_id) + assert kept is not None + assert kept["state"] == "completed" + assert kept["last_delivery_error"] == "boom" + + def test_sweep_ignores_recurring_jobs(self, tmp_cron_dir): + """Old recurring jobs are never candidates, whatever their history.""" + job = create_job(prompt="Recurring", schedule="every 1h") + stamp = ( + datetime.now(timezone.utc) - timedelta(days=365) + ).isoformat() + jobs = load_jobs() + for j in jobs: + if j["id"] == job["id"]: + j["last_run_at"] = stamp + save_jobs(jobs) + get_due_jobs() + assert get_job(job["id"]) is not None + + def test_sweep_disabled_by_nonpositive_retention(self, tmp_cron_dir, monkeypatch): + monkeypatch.setattr( + "cron.jobs._completed_oneshot_retention_days", lambda: 0.0 + ) + old_id = self._completed_oneshot(age_days=30) + get_due_jobs() + assert get_job(old_id) is not None + + def test_recurring_jobs_unaffected_by_retention_change(self, tmp_cron_dir): + """A recurring job still cycles normally alongside retained one-shots.""" + recurring = create_job(prompt="Recurring", schedule="every 1h") + self._completed_oneshot(age_days=1) + mark_job_run(recurring["id"], success=True) + updated = get_job(recurring["id"]) + assert updated["enabled"] is True + assert updated["state"] == "scheduled" + assert updated["next_run_at"] is not None diff --git a/tests/hermes_cli/test_apply_profile_override.py b/tests/hermes_cli/test_apply_profile_override.py index 0eb6fc7a3984..79c1ee1be9ab 100644 --- a/tests/hermes_cli/test_apply_profile_override.py +++ b/tests/hermes_cli/test_apply_profile_override.py @@ -164,3 +164,65 @@ def test_supervised_named_profile_flag_still_wins(self, tmp_path, monkeypatch): assert result is not None assert result.endswith("coder") + +class TestCronEditProfileIsJobField: + """Issue #32045: `hermes cron edit --profile ` sets the job's + profile FIELD — it must NOT switch the active profile context (HERMES_HOME). + + Before the fix, _apply_profile_override consumed --profile after + ``cron edit``, redirected HERMES_HOME to the target profile, and + cron_edit then looked for the job in the WRONG store -> 'Job not found'. + """ + + def test_cron_edit_profile_does_not_switch_hermes_home(self, tmp_path, monkeypatch): + result = _run_apply_profile_override( + tmp_path, + monkeypatch, + hermes_home=str(tmp_path / ".hermes"), + active_profile=None, + argv=["hermes", "cron", "edit", "job123", "--profile", "trading"], + ) + # HERMES_HOME must stay at the root — cron edit's --profile is a job field. + assert result == str(tmp_path / ".hermes") + + def test_cron_edit_profile_equals_form_does_not_switch(self, tmp_path, monkeypatch): + result = _run_apply_profile_override( + tmp_path, + monkeypatch, + hermes_home=str(tmp_path / ".hermes"), + active_profile=None, + argv=["hermes", "cron", "edit", "job123", "--profile=trading"], + ) + assert result == str(tmp_path / ".hermes") + + def test_cron_edit_short_p_flag_still_switches(self, tmp_path, monkeypatch): + """-p remains the profile-context flag everywhere EXCEPT cron edit's --profile. + + A bare `-p` before the subcommand (e.g. `hermes -p coder cron edit job123`) + is the operator selecting WHICH profile's store to operate on — that must + still switch HERMES_HOME. + """ + (tmp_path / ".hermes" / "profiles" / "coder").mkdir(parents=True, exist_ok=True) + result = _run_apply_profile_override( + tmp_path, + monkeypatch, + hermes_home=str(tmp_path / ".hermes"), + active_profile=None, + argv=["hermes", "-p", "coder", "cron", "edit", "job123", "--profile", "trading"], + ) + assert result is not None + assert result.endswith("coder") + + def test_plain_chat_profile_still_switches(self, tmp_path, monkeypatch): + """Sanity: --profile on non-cron commands still switches context.""" + (tmp_path / ".hermes" / "profiles" / "coder").mkdir(parents=True, exist_ok=True) + result = _run_apply_profile_override( + tmp_path, + monkeypatch, + hermes_home=str(tmp_path / ".hermes"), + active_profile=None, + argv=["hermes", "chat", "-q", "hi", "--profile", "coder"], + ) + assert result is not None + assert result.endswith("coder") + diff --git a/tests/hermes_cli/test_cron.py b/tests/hermes_cli/test_cron.py index 60477e2f80e8..0d0398182d58 100644 --- a/tests/hermes_cli/test_cron.py +++ b/tests/hermes_cli/test_cron.py @@ -78,6 +78,41 @@ def test_edit_can_replace_and_clear_skills(self, tmp_cron_dir, capsys): out = capsys.readouterr().out assert "Updated job" in out + def test_edit_sets_profile_field(self, tmp_cron_dir, capsys): + """cron edit --profile pins the job to a profile (#32045). + + The job record must gain a 'profile' field; subsequent get_job must + return it. Regression for the CLI path that previously never passed + profile through (and the --profile flag was consumed by + _apply_profile_override as a context switch). + """ + job = create_job(prompt="Profile pin", schedule="every 1h") + assert job.get("profile") is None + + cron_command( + Namespace( + cron_command="edit", + job_id=job["id"], + schedule=None, + prompt=None, + name=None, + deliver=None, + repeat=None, + skill=None, + skills=None, + clear_skills=False, + add_skills=None, + remove_skills=None, + script=None, + workdir=None, + no_agent=None, + profile="trading", + ) + ) + updated = get_job(job["id"]) + assert updated is not None + assert updated["profile"] == "trading" + def test_create_with_multiple_skills(self, tmp_cron_dir, capsys): cron_command( Namespace( diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 44e57b5d40b6..40b1c567555a 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -239,6 +239,74 @@ def test_quoted_and_brace_paths_are_hardline_blocked(command): assert desc +# Multi-line QUOTED arguments are data, not command sequences: a newline +# inside quotes is part of the argument the shell passes to the program. +# These previously tripped the hardline floor because the flat command-start +# class treated every raw newline — even inside quotes — as a command +# boundary, blocking `hermes send` message bodies, multi-line +# `git commit -m` messages, and heredoc text that merely MENTION +# shutdown/reboot commands. +_QUOTED_NEWLINE_DATA_ALLOW = [ + # hermes send with a multi-line message body (the reported symptom) + 'hermes send -t telegram -s "spark1" "console output:\nsudo reboot\ndone"', + 'hermes send -t telegram "line1\nshutdown -h now\nline3"', + # git commit -m with a multi-line message + "git commit -m 'ops notes:\nreboot the box after the deploy'", + 'git commit -m "fix startup\nsystemctl reboot was flaky here"', + # heredoc bodies quoting dangerous strings as data + "python3 - <<'EOF'\nmsg = 'run sudo reboot later'\nprint(msg)\nEOF", + "cat > /tmp/notes.txt <<'EOF'\nremember: shutdown -h now\nEOF", + # rm hardline floor is anchored to the same class — quoted prose about it + # across a line break must stay data too + 'git commit -m "docs:\nwarn about rm -rf / in the guide"', +] + +# The masking must be strictly scoped to quoted data: real command +# boundaries around/inside those same shapes still hit the floor. +_QUOTED_NEWLINE_THREATS_BLOCK = [ + # unquoted newline is a real command separator + "echo hi\nsudo reboot", + 'echo "a"\nsudo reboot', + 'git commit -m "safe message"\nshutdown -h now', + # command substitution inside double quotes really executes + 'hermes send -t telegram "$(sudo reboot)"', + 'echo "`shutdown -h now`"', + # multi-line quoted data followed by a REAL chained command + 'hermes send "line1\nline2" && sudo reboot', + # a heredoc whose body is data, but the delivery command itself is hardline + "sudo reboot <<'EOF'\nignored\nEOF", +] + + +@pytest.mark.parametrize("command", _QUOTED_NEWLINE_DATA_ALLOW) +def test_quoted_newline_data_not_blocked(command): + """Newlines inside quoted arguments are data, not command starts.""" + is_hl, desc = detect_hardline_command(command) + assert not is_hl, ( + f"multi-line quoted data false-positived the hardline floor: " + f"{command!r} (got: {desc})" + ) + + +@pytest.mark.parametrize("command", _QUOTED_NEWLINE_THREATS_BLOCK) +def test_real_newline_separated_threats_still_blocked(command): + """Unquoted newlines / $() / backticks remain real command boundaries.""" + is_hl, desc = detect_hardline_command(command) + assert is_hl, f"real threat leaked through hardline floor: {command!r}" + assert desc + + +def test_quoted_newline_data_not_blocked_by_full_guard_chain(clean_session): + """End-to-end: the guard chain must not hardline-block a multi-line + quoted message (yolo on, so only the unconditional floor can block).""" + enable_session_yolo("hardline_test") + command = 'hermes send -t telegram "status:\nsudo reboot happened at 3am"' + result = check_all_command_guards(command, "local") + assert result["approved"], ( + f"guard chain blocked multi-line quoted data: {result.get('message')}" + ) + + # Commands that carry the literal string "rm -rf /" (or a sibling) as DATA in # another command's quoted argument — a PR title, a commit message, an echo / # printf argument. The shell never executes that text as an rm command, so the diff --git a/tools/approval.py b/tools/approval.py index 48edfa2f3afe..151e53406553 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -2004,6 +2004,54 @@ class deliberately omits — into a form the anchored hardline/dangerous return "".join(parts) +def _mask_quoted_newlines(command: str) -> str: + """Replace raw newlines inside single/double quotes with a space. + + Detection-only rewrite. A newline inside a quoted string is DATA to the + shell — part of the argument, not a command separator — yet the flat + ``_CMDPOS`` start-position class treats every raw ``\\n`` as a command + start. That made any multi-line quoted argument (``hermes send`` message + bodies, ``git commit -m`` messages, heredoc text) trip the hardline + blocklist when a data line began with e.g. ``sudo reboot``. + + Quote tracking mirrors ``_iter_shell_command_starts``: single quotes are + literal until the closing quote; inside double quotes a backslash escapes + the next character. Real command boundaries are unaffected: unquoted + newlines pass through untouched, ``$(``/backtick remain ``_CMDPOS`` + anchors independent of newlines, and ``_mark_command_starts`` still + re-inserts newlines at every genuine quote-aware command start. An + unclosed quote absorbs following newlines exactly as the shell would + (the quoted word continues across the line break), so masking them + cannot hide a runnable command. + """ + if "\n" not in command: + return command + out: list[str] = [] + quote: str | None = None + i = 0 + while i < len(command): + ch = command[i] + if quote: + if ch == "\\" and quote == '"' and i + 1 < len(command): + out.append(command[i:i + 2]) + i += 2 + continue + if ch == quote: + quote = None + out.append(" " if ch == "\n" else ch) + i += 1 + continue + if ch in ("'", '"'): + quote = ch + elif ch == "\\" and i + 1 < len(command): + out.append(command[i:i + 2]) + i += 2 + continue + out.append(ch) + i += 1 + return "".join(out) + + def _iter_shell_command_word_spans(command: str): """Yield command-position words that may be executable names.""" for command_start in _iter_shell_command_starts(command): @@ -2047,7 +2095,13 @@ def _iter_shell_command_word_spans(command: str): def _command_detection_variants(command: str): - normalized = _normalize_command_for_detection(command) + # Mask quoted newlines BEFORE normalization: normalization strips + # backslash-escapes (\" -> ") and empty-string pairs (""), which would + # corrupt quote tracking — e.g. `echo "a\""` normalizes to `echo "a` (an + # unterminated quote), so masking the normalized text could swallow a + # REAL unquoted newline separator that follows. The raw command carries + # faithful shell quote state. + normalized = _normalize_command_for_detection(_mask_quoted_newlines(command)) # Quote-aware grep parsing hides only structurally identified pattern # operands. Malformed/ambiguous input remains byte-for-byte intact. grep_safe, _ = _grep_safe_detection_variant(normalized) diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 5f1a56d25805..fe816664152c 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -586,6 +586,8 @@ def _format_job(job: Dict[str, Any]) -> Dict[str, Any]: result["enabled_toolsets"] = job["enabled_toolsets"] if job.get("workdir"): result["workdir"] = job["workdir"] + if job.get("profile"): + result["profile"] = job["profile"] return result @@ -729,6 +731,7 @@ def cronjob( no_agent: Optional[bool] = None, attach_to_session: Optional[bool] = None, task_id: str = None, + profile: Optional[str] = None, ) -> str: """Unified cron job management tool.""" del task_id # unused but kept for handler signature compatibility @@ -801,6 +804,7 @@ def cronjob( workdir=_normalize_optional_job_value(workdir), no_agent=_no_agent, attach_to_session=attach_to_session, + profile=profile, ) _notify_provider_jobs_changed_safe() _create_message = f"Cron job '{job['name']}' created." @@ -998,6 +1002,9 @@ def cronjob( success=False, ) updates["no_agent"] = target_no_agent + if profile is not None: + # Empty string clears the field; otherwise set the profile name. + updates["profile"] = profile or None if repeat is not None: # Normalize: treat 0 or negative as None (infinite) normalized_repeat = None if repeat <= 0 else repeat @@ -1127,6 +1134,10 @@ def cronjob( "type": "boolean", "description": "When True, this job becomes CONTINUABLE: the user can reply to its delivery and the agent has the brief in context instead of asking 'what is that?'. On thread-capable platforms (Telegram topics, Discord/Slack threads) a dedicated thread is opened for the job and its replies; on DM-only platforms (WhatsApp/Signal) the brief is mirrored into the origin DM session. Use this for conversational recurring jobs the user will reply to — daily briefings, reminders that kick off follow-up work. Leave unset for fire-and-forget alerts/watchdogs. Overrides the global cron.mirror_delivery config for this one job. Only the origin chat is touched (never fan-out targets); no effect when deliver='local'." }, + "profile": { + "type": "string", + "description": "Assign this job to a specific profile. The job's cron data (db, output) stays in the profile's home. Pass empty string to clear, or omit to keep current. Only meaningful on create/update." + }, }, "required": ["action"] } @@ -1184,6 +1195,7 @@ def check_cronjob_requirements() -> bool: enabled_toolsets=args.get("enabled_toolsets"), workdir=args.get("workdir"), no_agent=args.get("no_agent"), + profile=args.get("profile"), task_id=kw.get("task_id"), ), check_fn=check_cronjob_requirements, diff --git a/website/docs/user-guide/features/cron.md b/website/docs/user-guide/features/cron.md index b855f5e5ec5e..11079c110903 100644 --- a/website/docs/user-guide/features/cron.md +++ b/website/docs/user-guide/features/cron.md @@ -174,6 +174,7 @@ The `` placeholder below (and in [Lifecycle actions](#lifecycle-actions) /cron edit --skill blogwatcher --skill maps /cron edit --remove-skill blogwatcher /cron edit --clear-skills +/cron edit --profile trading ``` ### Standalone CLI @@ -185,6 +186,7 @@ hermes cron edit --skill blogwatcher --skill maps hermes cron edit --add-skill maps hermes cron edit --remove-skill blogwatcher hermes cron edit --clear-skills +hermes cron edit --profile trading ``` Notes: @@ -193,6 +195,7 @@ Notes: - `--add-skill` appends to the existing list without replacing it - `--remove-skill` removes specific attached skills - `--clear-skills` removes all attached skills +- `--profile ` pins the job to a profile: the job's cron data (db, output) stays in that profile's home. Pass an empty string (`--profile ""`) to clear the pin. On the CLI, `--profile` after `cron edit` is the **job field**, not the profile-context switch — `hermes cron edit --profile trading` edits the job in your current profile's store and pins it to `trading`, it does not switch which store is read. (Use `hermes -p cron ...` to operate on another profile's store directly.) ## Lifecycle actions