Repository navigation
fix(runtime-api): reflect fresh task engagement in task status - #5632
liususan091219 merged 1 commit into
Conversation
sonichi
left a comment
There was a problem hiding this comment.
Independently verified, not just reading the pasted evidence:
- Bug confirmed on main.
TasksView.status()only checked".claimed-" in live.name; a task that's genuinely RUNNING with real tool activity but hasn't hit the claim-rename convention read back aspending. Reproduced this directly: checked out the new test file against the parent commit (8c23230) with the oldtasks_view.py, and 3/5 subtests failed exactly as expected (['pending','pending','pending'] != ['in_progress','in_progress','in_progress']). At the PR head, all 5 pass. That's a real regression test, not a happy-path tautology. - Single source of truth, so the fix's reach is wider than the one-line diff suggests.
details(),list_tasks()and the cancel path all callself.status(task_id)["state"]— no duplicated claim-check anywhere else needed fixing. - Traced the exception path. The broad
except (UnicodeError, TypeError, OverflowError)around the new call is exactly sized to whatin_progress()/engaged_at()can raise on adversarial on-disk JSON:UnicodeDecodeError(⊂UnicodeError) from a truncated/invalid-utf8 line read underencoding="utf-8",OverflowErrorfrommath.isfinite()on a too-large int timestamp,TypeErrorfrom an unhashablekindfield (list/dict) in thenot in _ENGAGED_KINDScheck. All three map to the test'struncated-utf8/invalid-utf8/overflow-row/overflow-snapshot/list-kind/object-kindcontrols. Fail-closed direction (falls back topending, never crashes a read or blocks cancellation) is the right default for this kind of malformed-data path. - Workspace scoping verified:
self.tasks_dir.parentis the right workspace root forin_progress(), and the isolation subtest (engagement recorded in a different workspace doesn't flip the owned task) backs that. - review-checks clean on my own run against the merge-base (not just the PR's claim):
PASS (hardcoded-paths + root-artifacts + prose-cap clean). - CI green, CLA passing, scope is exactly
tasks_view.py+ a new test file, no public state vocabulary or cancellation/delivery semantics touched.
Ready to merge once required CI/CLA/approvals are satisfied — merge itself needs the owner's word, not something I'll do on a collaborator ask alone.
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 92%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
liususan091219
left a comment
There was a problem hiding this comment.
Ready to approve.
Independent second look at head 68a424cba. sonichi's approval covers the same head.
What I ran (fresh checkout of 68a424cba, with SUTANDO_WORKSPACE_DIR and SUTANDO_TASKS_DIR unset):
tests/runtime-api-task-engagement.test.py: 5/5 passedtests/runtime-api-tasks-view.test.py: 73/73 passedtests/activity-bus-in-progress.test.py: 10/10 passed
What I read:
- The policy is delegated, not copied.
TasksView.statuscalls the existingactivity_bus.in_progress(src/activity_bus.py:317) and adds no freshness rule of its own. The result, HITL and ownership checks still come first, and a claimed task short-circuits before the activity read. - The workspace argument is correct for the shipped wiring.
self.tasks_dir.parentis the workspace becauseserver.py:150buildsTasksView(Path(state_dir).parent / "tasks", …). - The exception list matches what the callee can still raise.
in_progressalready swallowsOSError/ValueErroron the snapshot.engaged_at(activity_bus.py:289) only catchesOSErroraround the file, so a bad-UTF-8 line escapes asUnicodeDecodeError, and the newexcept (UnicodeError, …)is what stops that from failing a status read.OverflowErrorcoversfloat()of a huge integer stamp.
Non-blocking:
- Status reads now scan the whole activity log.
list_taskscallsstatus()once per task (tasks_view.py:374), and each call now reads all ofstate/agent-activity.jsonl. On one live host that file is 400 lines (~200 KB), so the cost is negligible today. If the log is ever left to grow without bound,listgets linear in tasks × log size. - The live-path proof is harness-only. The body's evidence is real daemon/CLI runs against a scratch socket, not a running core. A round trip on a real install is owed, but per this repo's notes that is named here rather than blocking.
No hardcoded host paths in the added lines.
— Sutando (susans-MacBook-Pro)
qingyun-wu
left a comment
There was a problem hiding this comment.
Approve at 68a424c (code owner for /src/runtime-api/).
- It asks the owner, not the files. An unclaimed live task now consults
activity_bus.in_progress(...), the module that owns activity state, instead of re-reading activity files. Onlyworkingrows and runtime events count as engagement. - "Fresh" is bounded. Engagement counts only while the newest stamp is under
IN_PROGRESS_MAX_AGE_S(1800 s), so a crashed core shows a task as in progress for at most 30 minutes before it falls back to pending. Future-dated or non-finite stamps are rejected. - No new exposure. The response is still
{taskId, state}; the traversal and ownership checks run before the new lookup, andserver.pyanddispatcher.pyare untouched. A ready result still reports done, and the waiting states still take precedence.
Verified (reviewer agent, fresh worktree): tests/runtime-api-task-engagement.test.py 5/5 at head, and 3 failures with tasks_view.py reverted to the merge-base. runtime-api-tasks-view 73/73, activity-bus-in-progress 10/10, and the dispatcher, e2e and server suites pass. CI is green. I read the source change myself.
Follow-up worth an issue (not this PR's bug): the caller's except (UnicodeError, TypeError, OverflowError) is covering for activity_bus. One invalid byte in state/agent-activity.jsonl makes engaged_at raise UnicodeDecodeError while reading the file, so engagement disappears for every task at once, and the in-progress CLI that check-pending-tasks.sh calls exits with a traceback. The fix belongs in activity_bus (decode with errors="replace" or skip per line), after which this catch can go. Today's behaviour falls back to pending, the same as before this PR, so it isn't a regression.
Nit: in_progress reads the whole activity file per call, and list calls status per task. Fine at today's size; worth an index if the file grows.
Priority
Level: medium
Reason: Owner-approved reliability improvement.
Closes
N/A — owner-approved work; no public issue is claimed as closed.
Summary
fix(runtime-api): reflect fresh task engagement in task status
Review first:
src/runtime-api/tasks_view.py,tests/runtime-api-task-engagement.test.py.Checklist
gh pr list --repo sonichi/sutando --search "closes #N")git log -1 --format='%ae'shows a GH-mapped email, not*.localornoreply@anthropic.com)upstream/main(or feature isn't already covered)branches:filter can silently exclude a stacked PR)resolve_workspace/resolveWorkspace/claude_home_pathhelpers (seeCLAUDE.md"Workspace contract"; enforced by thelint-workspace-resolution,lint-claude-home-path, andworkspace-leak-checkCI checks)Contribution observations (unchecked items remain reviewer decisions):
Duplicate search: Searched PRs (all states) and open issues using fix(runtime-api): reflect fresh task engagement in task status, "activity_bus.in_progress". Related results for reviewer comparison: fix(core): a task the Stop hook hands to the core inline is recorded as read #5302, fix(stop-hook): a guest session owes nothing, a task in progress does not block, and the same queue cannot block a turn end forever #4863 Returned PRs were compared against current files and overlapping diffs: fix(core): a task the Stop hook hands to the core inline is recorded as read #5302: Different changed files.; fix(stop-hook): a guest session owes nothing, a task in progress does not block, and the same queue cannot block a turn end forever #4863: Different changed files.. Title similarity alone is not equivalence; this is not exhaustive semantic coverage.
Upstream observed during evidence preparation:
9d70abfa2b0af2a1305eba644c7677eeca2dd4e6. Main advanced after the recorded source base. Local evidence verifies the exact PR candidate, not its integration with newer main. PR checks, independent review and branch maintenance govern merge.Diff review: - review-correctness: Inspected TasksView.status, its callers, activity_bus.in_progress and production activity writers. Ownership, ready-result and HITL checks precede engagement lookup; claimed tasks short-circuit it. Additional real-socket observations confirmed assigned-task engagement, incomplete-result handling, cancellation without target mutation and exclusion of activity lacking a live task file. Evidence: diagnostics/review-adjacent-states.json.
review-scope: The diff changes only src/runtime-api/tasks_view.py and tests/runtime-api-task-engagement.test.py. Engagement freshness, activity writers, watcher behavior, cancellation protocol, archive lookup and public state vocabulary remain unchanged.
review-maintainability: The implementation delegates to the existing engagement policy using self.tasks_dir.parent without duplicating freshness logic. Targeted exception handling protects reads from malformed activity. The integration harness uses production writers, actual daemon/CLI transport, bounded readiness and subprocess calls, and registered cleanup with terminate/wait and kill/wait fallback.
Documentation: No standalone documentation files changed. The reviewer should assess the code and contract comments for documentation completeness.
Live-path verification: - monitor-user-journey: Ran python3 -B [local path] at 2026-10-16T18:59:04Z–18:59:16Z: exit 0, 113 real CLI calls. Started python3 src/runtime-api/server.py on private monitor-journey-3/api.sock. Commands through src/runtime-cli/sutando-runtime.py included task submit, status, details, list and get-result. Production activity writers changed both unclaimed tasks from pending to in_progress across all three read surfaces; atomic result publication produced done and returned 'Observed local completion'. Exact commands, responses and writer actions: monitor-journey.jsonl.
monitor-repeatability: Repeated task status/details/list for task-rtapi-2e31eba5e4ad and task-rtapi-c48983f0fe2f twice: every response showed in_progress. Terminated and waited for daemon PID 80545, then reran python3 src/runtime-api/server.py as PID 80805 without refreshing engagement. The same commands still showed in_progress; after publication, repeated reads showed done and unchanged result text. Readiness took 0.298 and 0.308 seconds. Evidence: monitor-journey.jsonl and monitor-journey-summary.json.
These observations used the isolated candidate; no production deployment was performed.
Rollback: revert this PR. Production behavior after deployment remains to be observed.
keweichen; CLA status must be observed on the PR.Before / after evidence
Before
Recorded command/output on the pinned source:
Exit: 0
Exit: 0
After
acceptance-1: Real server.py and runtime-cli over private Unix sockets reported in_progress consistently in status, details and list for fresh production activity-row and runtime-event engagement. Task filenames, file identities, modification times and activity-content hashes remained unchanged.
acceptance-2: Absent activity, delivery-only RUNNING, processing/thinking, row-only, stale/future engagement, invalid timestamps and malformed records remained pending. Claimed tasks remained in_progress, including with malformed activity. Cancellation regression checks also passed.
acceptance-3: All three HITL waiting states retained precedence over engagement; ready results then produced done. Foreign tasks/results remained inaccessible and unlisted. Engagement confined to another workspace left the owned task pending. Archive lookup remained supported.
acceptance-4: Both engagement paths survived restart from daemon PID 35017 to 35087 at 2026-10-16T18:50:05.998866Z, then transitioned to done after result publication. Every product subprocess inherited the supplied scratch environment. Readiness used real socket RPC with a 10-second bound. Across ordinary and coverage runs, all 12 daemons exited and were reaped; maximum readiness was 0.673 seconds and all sockets were removed. Startup commands, timestamps and lifecycle receipts: [local path]
check-task-engagement-integration: python3 tests/runtime-api-task-engagement.test.py — 5 tests passed, exit 0, 17.233 seconds; started 2026-10-16T18:49:50.316391Z. Log: [local path]
check-task-view-regressions: python3 tests/runtime-api-tasks-view.test.py — 73 tests passed, exit 0, 1.845 seconds. Log: [local path]
check-engagement-policy-controls: python3 tests/activity-bus-in-progress.test.py — 10 tests passed, exit 0, 0.198 seconds. Log: [local path]
coverage: Ran python3 -m coverage run --rcfile=.coveragerc tests/runtime-api-task-engagement.test.py with subprocess coverage enabled, then combined data and generated XML. diff-cover [local path] --compare-branch=8c2323052d779facfe08199e3908827d1c69a371 --fail-under=95 passed: 5 changed executable lines, 0 missing, 100% coverage. The product's unmeasured-file check reported no omissions.
candidate-identity: HEAD matches the supplied candidate; its merge base with the approved source is 8c23230. Only the two allowed paths differ. Final git status was clean at 2026-10-16T18:52:17Z. Source, engagement policy, writers and watcher behavior were not edited during verification.
Test plan
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Canonical review scanner:
Verified candidate:
ac722ba7eabed5c1f1caf3ebefffc940dcfa3267. Publication commit:68a424cbabae0f64f471aa876d8b8146c0621f74. Both have treec99fc4c9b8ca514eba6b8fb36a8a2b2cceb274a7; only factory-authored commit metadata changed. Acceptance evidence refers to the verified candidate; the scanner ran on the publication tree.