Repository navigation
fix(runtime-api): preserve runtime API task priority through queue consumption - #5505
Conversation
sonichi
left a comment
There was a problem hiding this comment.
Verified at 10d9ea9 by running the diff, not just reading it.
- Copied the new test file onto the parent commit (7ac30fc) and ran it: 5 failures, including the exact described symptom —
parse_priority_from_filereads backnormalfor a task actually submitted withpriority=loworpriority=urgent. At head, all 67 tests pass. - Read the fix:
_submission_textpreviously wrotetask: {text}\nas the THIRD line, beforesource/channel_id/user_id/access_tier/priority. Per this repo's own documented header-parser contract (CLAUDE.md "Chat-path task tracking": "task:must be the LAST header... a task written withtask:earlier routes nowhere and sorts asnormalhowever it declares itself"), every real header aftertask:was being swallowed into the body by the strict parser — this PR movestask:to actually be last, matching the documented contract. Clean, well-targeted fix of a real silent-priority-loss bug, not a new invention. _check_record's widenedrecord_keys in (expected_keys, task_last_keys)check is the right backward-compatibility move: an idempotency receipt published under the OLD ordering (before this fix) still validates as the same submission, so an in-flight retry across the deploy doesn't spuriously see "already bound" or duplicate.test_pre_change_receipts_preserve_identity_and_reject_conflictscovers the legacy-ordering compatibility directly (8 state/instance combinations), and the negative cases (different task text/priority/actor/scope) still correctly raise "already bound".- No regressions:
tests/runtime-api-dispatcher.test.pypasses at head.review-checks.shon the diff: clean. - Coordination note, not a blocker: this diff touches
tests/runtime-api-tasks-view.test.py, same file as #5490 (already approved, not yet merged). I merged #5490's head into this branch locally as a check — it auto-merges cleanly, different regions of the file, so no rebase conflict expected either order.
APPROVE.
qingyun-wu
left a comment
There was a problem hiding this comment.
Approve at 10d9ea9. I reviewed this independently in an isolated worktree. No blockers.
The bug exists on main (base 7ac30fc). _submission_text wrote task: third. parse_priority_from_text stops at the first task:, so it never saw priority:. Every runtime-API submission and cancel signal read back as normal. The head writes task: last, so the declared tier now reaches task_queue.py and delivery/task_dispatch.py through sort_tasks_by_priority.
Before/after evidence. I ran the same script against both trees. It submits low@mtime100, normal@101, urgent@102 through submit and submit_idempotent, then checks a default submit, a cancel, and a forged body.
BASE 7ac30fc:
submit low -> normal; idempotent low -> normal
submit normal -> normal; idempotent normal -> normal
submit urgent -> normal; idempotent urgent -> normal
default(no priority) -> normal
cancel signal -> normal
forged body urgent, declared low -> normal
queue order (low@100,normal@101,urgent@102): ['low', 'normal', 'urgent']
header order: ['id', 'envelope_hmac', 'timestamp', 'task', 'source', 'channel_id', 'user_id', 'access_tier', 'priority']
HEAD 10d9ea9:
submit low -> low; idempotent low -> low
submit normal -> normal; idempotent normal -> normal
submit urgent -> urgent; idempotent urgent -> urgent
default(no priority) -> normal
cancel signal -> urgent
forged body urgent, declared low -> low
queue order (low@100,normal@101,urgent@102): ['urgent', 'normal', 'low']
header order: ['id', 'envelope_hmac', 'timestamp', 'source', 'channel_id', 'user_id', 'access_tier', 'priority', 'task']
lowstayslowend to end and is not normalized tonormal.- Normal and default behavior is unchanged. With no priority,
dispatcher.py:306-308defaults to"normal", which matchesdefault_priority_for_source("runtime-api"). task:is now the last header, as CLAUDE.md requires. The HMAC stamp is still inserted afterid:.
The PR's new tests fail on base and pass on head:
- base with the head test file:
Ran 67 tests ... FAILED (failures=5). The failures aretest_cancel_writes_cancel_instruction_signaland 4 subtests oftest_queue_priority_roundtrip_order_and_body_confinement. - head:
Ran 67 tests ... OK.
Mutations at head. Each one was caught by tests/runtime-api-tasks-view.test.py:
- M1: invalid priority passes and is written as
urgent. Caught:FAIL test_submit_rejects_empty_and_bad_priority. - M2:
lowis written asnormal. Caught:FAIL test_queue_priority_roundtrip... (priority='low')plustest_details_roundtrip. - M3: the legacy (task-third) receipt order is no longer accepted. Caught: 8
ERROR test_pre_change_receipts_preserve_identity_and_reject_conflictssubtests (pending, claimed, archived and result-only, each with and without an instance). - M4: the cancel signal is written as
normal. Caught:FAIL test_cancel_writes_cancel_instruction_signal.
Invalid input and authorization.
_validated_submissionstill rejects anything outside urgent|normal|low.- The dispatcher maps that
ValueErrorto JSON-RPC -32602. An invalid value cannot become urgent. On the read side,task_prioritystill fails open tonormal. - A forged
priority: urgentin the body stays confined, because_one_linekeeps the text on one line and the reader stops attask:. - The tier is unchanged. The socket is same-user 0600,
access_tier: owneris fixed by the writer, and the actor comes fromresolve_actor_idon the daemon side, not from params. An owner choosing urgent is within policy. - Transport/dispatcher boundaries are unchanged. The fix stays in
tasks_view.py, behind the dispatcher.
Worst-case disruption.
- Runtime-API tasks submitted as
urgentnow really do jump ahead of voice-tier tasks. That is the intent, and only a same-user owner caller can trigger it. - Receipts and queued files written before the upgrade keep the old order. They are accepted by
_check_record(record_keys in (expected_keys, task_last_keys)) and keep reading asnormal, which is harmless. - Rollback is a clean revert.
Other tests run at head, all exit 0: task-priority*.test.py (3 files), task-queue*.test.py, task-envelope*.test.py, gateway-task-envelope-golden, bridges-envelope-stamp, health*task-queue* (3 files), and every runtime-api-*.test.py (dispatcher, e2e, server, push, store and others).
Hardcoded-path scan: git diff origin/main...HEAD | bash scripts/review-checks.sh gives review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean). A manual grep of the added lines for /Users/, /home/, ~/. and /usr/bin/ found nothing.
Nits (non-blocking):
task_last_keyssits next to the still-namedexpected_keyslegacy list. Renaming it tolegacy_keys, or adding a 1-line comment that the task-third order is pre-change receipts only, would make it clearer which order is canonical.- The PR body checklist is left unchecked even though the evidence below it covers most items.
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
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): preserve runtime API task priority through queue consumption
Review first:
src/runtime-api/tasks_view.py,tests/runtime-api-tasks-view.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 Preserve runtime API task priority through queue consumption. Related results for reviewer comparison: feat(bee-channel): Bee wearable as inbound-only channel — SSE watcher onto the broker's /v1/ingest hop #2706, feat(runtime-api): add idempotent task submission #4488, docs(pool): worker-pool design v1 — step 1 of staging #3604 (no router process) #3860, fix(watcher): leave an in-flight routed claim to the next watcher's sweep instead of announcing it #4826, fix(watcher): scope automatic task-event-handler resolution to core only #4503, feat(mentra-bridge): track WIP bridge + wire to DeviceSession registry #740 No close title duplicate was detected; this is not a claim of exhaustive semantic coverage.
Current upstream:
7ac30fc87f6281404b48ae8e0d4f3837c571d68b. Current main equals the tested source base; required GitHub checks still run on the PR.Diff review: - review-correctness: Inspected the writer, receipt validation, safe and lenient parsers, envelope stamping and cancellation flow. Moving task: after metadata exposes priority to the unchanged safe reader. Receipt validation accepts precisely the legacy and corrected known-header orders while preserving canonical-byte equality and submission-field comparisons.
review-scope: HEAD matches the exact candidate commit; working tree is clean. Base-to-candidate diff changes only the two allowed paths. SHA-256 is 71a43c417fbbec7e43431d7dc37a005f4a22756e40d288c6c3d81f2bf2ec151a. Shared parsing, adapters, claiming, execution and cancellation methods are unchanged.
review-maintainability: The production change is small and retains the existing shared submission writer. Compatibility derives the corrected order from the legacy order without duplicating publication logic. Tests use production readers and independently constructed legacy fixtures. Added lines contain no hardcoded host paths; git diff --check passed.
Documentation: No standalone documentation files changed. The reviewer should assess the code and contract comments for documentation completeness.
Live-path verification: - monitor-user-journey: Executed python3 -B - via heredoc using TasksView and temporary task/result directories. Ordinary and keyed submissions preserved tiers; production sorting returned indices [2, 1, 3, 0] (urgent, normal FIFO, low). Details preserved instance metadata and body injection stayed confined. Cancellation produced an urgent signal without changing the original task.
monitor-repeatability: The same Python journey reopened TasksView and retried a keyed submission: identity, file names and bytes remained unchanged. Two retries against an explicitly constructed legacy receipt retained its identity and bytes; conflicting text raised ValueError without publication. Reading a completed result and cancelling twice preserved all files. A separate python3 -B - run independently repeated reopened retry and completed cancellation, exiting 0.
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:
/bin/zsh -lc 'python3 -B tests/task-priority-stop-at-task-delimiter.test.py'Exit: 0
Exit: 1
After
acceptance-1: Ordinary and idempotent submissions preserved urgent, normal and low through the production queue priority reader.
acceptance-2: Controlled-mtime checks confirmed urgent precedes normal precedes low, with FIFO ordering for equal priorities.
acceptance-3: Cancellation signals parsed as urgent. Completed-task cancellation returned false and left the task-file set unchanged.
acceptance-4: Legacy-receipt tests passed for pending, claimed, archived and result-only states, with and without instance metadata. Retries preserved identity and receipt bytes without duplicate publication; conflicting text, priority, actor and instance were rejected.
acceptance-5: Ownership, instance metadata, confinement and independent-submission tests passed. Embedded priority text could not override declared priority in either submission path.
check-priority-roundtrip: Executed the frozen Python program via stdin; exit 0, output PASS.
check-task-view-regressions: python3 -B tests/runtime-api-tasks-view.test.py exited 0: Ran 67 tests in 0.324s; OK.
check-priority-boundary: python3 -B tests/task-priority-stop-at-task-delimiter.test.py exited 0; all six parser boundary cases passed.
Test plan
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Canonical review scanner:
Verified candidate:
cb4c4012ec38c8f3a4f3eb82ea9aeb64bdb770de. Publication commit:10d9ea902c8b15d73e397f9782b46b149100cbb2. Both have tree9ce834687d2389b6411a7acecc5fd5507505744f; only factory-authored commit metadata changed. Acceptance evidence refers to the verified candidate; the scanner ran on the publication tree.