Repository navigation
feat(agent-runs): include cron-driven workflows in list - #43
Conversation
…gent_runs Audit log only records operator-initiated triggers (REST POST → audit.Log.Append). Scheduled cron runs of mctl-agents-daily, mctl-agents-shepherd, and other CronWorkflow-spawned Workflows were invisible to mctl_list_recent_agent_runs. During the 2026-05-07 ESO- incident triage this produced a false "agent system idle" reading — the operator saw last activity 6 days ago, while the daily cron had in fact fired every morning. See plan ~/.claude/plans/mctl-agents-daily-cron-visibility.md. Implementation: - New Executor.ListCronAgentRuns(ns, prefix, since): list workflows with label workflows.argoproj.io/cron-workflow + name-prefix filter, map Argo phase to audit-log status vocabulary, drop runs older than `since`. - WorkflowExecutor interface extended; fakeExecutor in tests gains cronAgentRuns slice for deterministic merge tests. - ListAgentRuns handler merges operator + cron entries, sorts by timestamp desc (Z-suffixed RFC3339 sorts lexicographically), caps at 10. Each item carries `source: "operator" | "cron"` so the caller can distinguish. - MCP tool description updated to document the merge and the new source field. - Test pins the merge order: cron-newest first, operator-oldest last. Degrades gracefully on cluster API failure — logs warn and falls back to audit-only view rather than failing the whole response.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41b6d4ddc5
ℹ️ 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".
| ti, _ := items[i]["timestamp"].(string) | ||
| tj, _ := items[j]["timestamp"].(string) |
There was a problem hiding this comment.
Sort agent runs using actual timestamp type
The new merge-sort logic compares timestamp as string, but operator entries are added with "timestamp": e.Timestamp (a time.Time). In this comparator, type assertions for those rows fail and default to "", so operator-triggered runs are always treated as oldest whenever cron items are present. This can hide recent manual runs from the top 10 and breaks the endpoint’s “newest-first” guarantee.
Useful? React with 👍 / 👎.
Codex P1 on PR #43: audit.Entry.Timestamp is a time.Time, so when ListAgentRuns stuffed it into the items map as `"timestamp": e.Timestamp` and the merge-sort comparator did `items[i]["timestamp"].(string)`, the type assertion failed for operator entries, defaulted to "", and operator runs always sorted last regardless of how recent. This silently hid recent manual triggers from the top-10 response whenever any cron items were present. Format e.Timestamp to RFC3339 (UTC) at insert time so cron + operator items share one timestamp shape and the lexicographic compare is correct.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Why
Audit log is operator-only, so `mctl_list_recent_agent_runs` was blind to cron-driven runs. During 2026-05-07 ESO-incident triage this produced a false "agent system idle" reading — last activity looked 6 days old while `mctl-agents-daily` had fired every morning. See plan `~/.claude/plans/mctl-agents-daily-cron-visibility.md`.
Test plan