Skip to content

feat(dashboard): surface recovery state in task view (#1043)#1119

Open
shaun0927 wants to merge 2 commits into
feat/865-dashboard-ledger-viewsfrom
feat/1043-dashboard-recovery
Open

feat(dashboard): surface recovery state in task view (#1043)#1119
shaun0927 wants to merge 2 commits into
feat/865-dashboard-ledger-viewsfrom
feat/1043-dashboard-recovery

Conversation

@shaun0927
Copy link
Copy Markdown
Owner

@shaun0927 shaun0927 commented May 12, 2026

Progress / Review status

Auto-refreshed 2026-05-13 — owner comments cleaned up to reduce review noise.

Field Value
Branch feat/1043-dashboard-recoveryfeat/865-dashboard-ledger-views
Draft no
CI
Mergeable ✅ MERGEABLE
Review decision
Codex (latest) 💡 suggestions posted
Other reviewers (latest) chatgpt-codex-connector: commented
Head a6a60e4 — Compose dashboard recovery tests with ledger refresh tests
Commits 2

Owner comment cleanup: 0 issue + 0 inline review comments deleted. Outstanding feedback from automated/external reviewers above is unchanged.


Summary

Closes #1043.
Stacked on #955 / feat/865-dashboard-ledger-views; merge #911 then #955 first. This PR reads #1039/#1040 metadata defensively from disk so merge order stays flexible.

Direction / duplicate check before implementation

Success criteria

  • NEEDS_HELP TaskRuns render distinctly in the task dashboard recovery panel.
  • Details include reason, resume hint, cursor, latest checkpoint id, and evidence refs when metadata exists.
  • Active handoffs show handoff id, linked run id, reason, timeout remaining, and before URL/title.
  • Missing TaskRun/handoff metadata returns empty state and does not throw.
  • Reader is read-only across repeated render/snapshot loops.
  • 80x24, 120x40, and 200x60 render tests pass.

Verification performed

  • npm test -- --runTestsByPath tests/dashboard/views/ledger-views.test.ts --runInBand
  • npm run build
  • npm run lint:changed

Real OpenChrome verification after merge

  1. Launch OpenChrome with dashboard enabled.
  2. Start a TaskRun and navigate to https://the-internet.herokuapp.com/login.
  3. Call oc_task_run_needs_help with reason Manual login required before continuing, a current cursor, and a resume hint.
  4. Open the dashboard task view and verify the recovery panel shows NEEDS_HELP, the exact reason, cursor URL, resume hint, and evidence refs.
  5. Call oc_handoff_start for the same run with a safe before snapshot.
  6. Verify the recovery panel shows the active handoff id and timeout countdown within the normal dashboard tick cadence.
  7. Finish the handoff and resume the TaskRun; verify stale active handoff metadata no longer appears once the handoff status is no longer ACTIVE.
  8. Resize terminal to 80x24, 120x40, and 200x60; verify no broken wrapping artifacts.

Out of scope

  • Starting/finishing/cancelling/resuming from the dashboard.
  • Web dashboard or noVNC.
  • Inline screenshots or raw cookies/storage/typed text.

Surface read-only TaskRun NEEDS_HELP metadata and active handoff context in the terminal task view so operators can see why automation paused and how to resume without digging through JSON files.

Constraint: Stack on #955 dashboard task view and read #1039/#1040 metadata defensively from disk because those branches may merge in a different order.
Rejected: Dashboard resume/cancel controls | recovery state must remain read-only and MCP tools own mutation.
Confidence: high
Scope-risk: moderate
Directive: Keep dashboard recovery readers best-effort and non-throwing; never add writes or hotkey mutations to this pane.
Tested: npm test -- --runTestsByPath tests/dashboard/views/ledger-views.test.ts --runInBand
Tested: npm run build
Tested: npm run lint:changed
Co-authored-by: OmX <omx@oh-my-codex.dev>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18f70a1fdf

ℹ️ 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".

async function readTaskRuns(root: string, limit: number): Promise<TaskRunRecovery[]> {
const entries = await readdirSafe(root);
const rows: TaskRunRecovery[] = [];
for (const entry of entries.slice(0, limit)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply TaskRun limit after timestamp ordering

The loop truncates entries before any ordering, then sorts only the truncated subset by requested_at. Because fs.readdir does not provide a reliable recency order, dashboards with more than limit run folders can omit the most recent NEEDS_HELP runs entirely, so operators may not see current recovery state. Collect all readable rows first, sort by requested_at, and only then apply the limit.

Useful? React with 👍 / 👎.

async function readHandoffs(root: string, limit: number): Promise<HandoffRecovery[]> {
const entries = await readdirSafe(root);
const rows: HandoffRecovery[] = [];
for (const entry of entries.slice(0, limit)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply handoff limit after expiration ordering

This code also slices directory entries before sorting by expires_at, so only an arbitrary subset is ranked. In environments with many handoff records, active or soon-expiring handoffs can be dropped before sorting and never shown in the recovery panel, which defeats the new visibility feature. Read all candidate rows, sort them, then apply limit.

Useful? React with 👍 / 👎.

Resolve the stacked dashboard test conflict by preserving both recovery-pane read-only coverage and ledger refresh timer coverage.

Constraint: PR #1119 targets the dashboard-ledger-views branch, not develop, and only the shared ledger-views test file conflicted.
Rejected: Taking either side wholesale | that would drop either recovery state coverage or dashboard refresh guard coverage.
Confidence: high
Scope-risk: narrow
Directive: Keep dashboard stack conflict fixes test-only unless source conflicts require owner review.
Tested: npx jest tests/dashboard/views/ledger-views.test.ts --runInBand --forceExit; git diff --check for touched file.
Not-tested: Full GitHub Actions matrix.
Co-authored-by: OmX <omx@oh-my-codex.dev>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6a60e47a1

ℹ️ 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".

Comment on lines +97 to 99
const recoveryLines = this.renderRecovery(data.recovery, width);
const visibleRows = Math.max(0, size.rows - 7 - recoveryLines.length);
for (const t of data.tasks.slice(0, visibleRows)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Render recovery pane even when no task rows are present

Callers can provide recovery metadata independently of the task ledger (for example, when TaskRun/handoff files exist but tasks is empty), but renderRecovery(...) is only reached inside the else branch for non-empty tasks. In that state the view shows only the empty-task hint and silently drops NEEDS_HELP/ACTIVE recovery signals, which defeats the operator visibility this feature adds.

Useful? React with 👍 / 👎.

truncate(` cursor=${run.current_cursor || '—'} resume=${run.resume_hint || '—'}`, width - 4),
width,
));
const evidence = (run.evidence || []).slice(0, 3).map(item => `${item.kind}:${item.ref}`).join(', ') || '—';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard evidence entries before dereferencing fields

The reader casts meta.last_evidence to typed objects without validating each element, and rendering unconditionally accesses item.kind/item.ref. If the on-disk JSON contains null/undefined/non-object entries (e.g., partial or malformed metadata), this line throws and can break dashboard rendering instead of degrading gracefully.

Useful? React with 👍 / 👎.

Repository owner deleted a comment from gemini-code-assist Bot May 13, 2026
Repository owner deleted a comment from qodo-code-review Bot May 13, 2026
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