Repository navigation
fix(runtime-api): reconcile expired human-request mirrors - #5742
Merged
keweichen merged 2 commits intoOct 20, 2026
Merged
Conversation
keweichen
requested review from
liususan091219,
qingyun-wu and
sonichi
as code owners
October 19, 2026 22:18
Contributor
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 92%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
12 tasks
qingyun-wu
requested changes
Oct 20, 2026
qingyun-wu
left a comment
qingyun-wu
left a comment
Collaborator
There was a problem hiding this comment.
Code-owner review at 29b7e44. The fix is right: store.pending() lazily flips an elapsed row to 'expired', so the old _settle (gated on 'pending') never expired the HA mirror. Targeting the mirror by its deterministic ha_action_id, with ha_adapter.expire() checking subject and guard ownership first, closes the dangling card even for rows that expired before a restart.
Before approval:
- Duplicated policy (blocking per CLAUDE.md "fix where the policy lives"). This branch predates #5741, so _settle hardcodes ('approval','elicitation','human_action'). #5741 introduces MIRRORED_REQUEST_TYPES for exactly this set. Please rebase on #5741, or on main once it lands, and use the constant. The two PRs also conflict in dispatcher.py near recover() and _settle.
- Flaky test. test_restart_recovers_downtime_and_already_expired_rows failed 2 of 4 local runs ('expired' != 'pending'). Its 8 s TTL margin races the live daemon's 2 s resolver poll across about 8 sequential CLI fixture calls. Widen the margin or inject the clock.
Non-blocking: expired_human_requests() has no bound or high-water mark, so every historically expired mirrored row is rescanned every 2 s for the daemon's lifetime.
Contributor
|
@cla-assistant check |
Collaborator
Author
|
Addressed review feedback in
Regression checks:
/bin/zsh -c "python3 - <<'PY'
import datetime,hashlib,json,os,pathlib,subprocess,time
root=pathlib.Path('[local path]'); diag=root/'diagnostics'
env=os.environ.copy(); env.update(json.loads((root/'environment.json').read_text())); env.update(PYTHONDONTWRITEBYTECODE='1',SUTANDO_TELEMETRY='0')
argv=['python3','tests/runtime-api-request-cancellation.test.py']; t=time.monotonic(); started=datetime.datetime.now(datetime.timezone.utc).isoformat()
p=subprocess.run(argv,env=env,capture_output=True,text=True,timeout=60)
path=diag/'upstream-cancellation.log'; path.write_text(p.stdout+p.stderr)
receipt={'argv':argv,'started_at':started,'ended_at':datetime.datetime.now(datetime.timezone.utc).isoformat(),'elapsed_seconds':time.monotonic()-t,'exit_code':p.returncode,'log':str(path),'sha256':hashlib.sha256(path.read_bytes()).hexdigest()}
(diag/'upstream-cancellation.json').write_text(json.dumps(receipt,indent=2))
print(json.dumps(receipt)); print((p.stdout+p.stderr)[-2000:])
PY"Exit: 0 /bin/zsh -c "python3 - <<'PY'
import datetime,json,os,pathlib,subprocess,time
root=pathlib.Path('[local path]'); diag=root/'diagnostics'
env=os.environ.copy(); env.update(json.loads((root/'environment.json').read_text())); env.update(PYTHONDONTWRITEBYTECODE='1',SUTANDO_TELEMETRY='0')
code='''import importlib.util,time,unittest
from pathlib import Path
path=Path.cwd()/\"tests/runtime-api-request-expiry.test.py\"
spec=importlib.util.spec_from_file_location(\"expiry_journey\",path)
module=importlib.util.module_from_spec(spec); spec.loader.exec_module(module)
class DelayedExpiryJourney(module.ExpiryJourneyTests):
def issue(self,*args,**kwargs):
record=super().issue(*args,**kwargs)
time.sleep(2)
self.note(\"synthetic-fixture-delay\",seconds=2)
return record
suite=unittest.TestSuite([DelayedExpiryJourney(\"test_restart_recovers_downtime_and_already_expired_rows\")])
result=unittest.TextTestRunner(verbosity=2).run(suite)
raise SystemExit(not result.wasSuccessful())
'''
argv=['python3','-c',code]; t=time.monotonic(); started=datetime.datetime.now(datetime.timezone.utc).isoformat()
p=subprocess.run(argv,env=env,capture_output=True,text=True,timeout=90)
path=diag/'restart-fixture-delay.log'; path.write_text(p.stdout+p.stderr)
receipt={'argv':argv,'started_at':started,'ended_at':datetime.datetime.now(datetime.timezone.utc).isoformat(),'elapsed_seconds':time.monotonic()-t,'exit_code':p.returncode,'log':str(path)}
(diag/'restart-fixture-delay.json').write_text(json.dumps(receipt,indent=2))
print(json.dumps(receipt)); print(p.stdout+p.stderr)
PY"Exit: 0 Independent verification, code review and product checks passed on candidate |
qingyun-wu
approved these changes
Oct 20, 2026
qingyun-wu
left a comment
qingyun-wu
left a comment
Collaborator
There was a problem hiding this comment.
Code-owner approve at 3155aa1. Both of my blockers from 29b7e44 are resolved:
- The branch is rebased onto merged #5741 (3133b05 is an ancestor). MIRRORED_REQUEST_TYPES (dispatcher.py:87) is the single source, used in _reconcile_expired, _settle and recover. No second production copy exists; tasks_view.py:419 is an unrelated display ordering.
- test_restart_recovers_downtime_and_already_expired_rows now uses a 30 s TTL with an explicit margin guard, and passed 6 of 6 runs in isolation. Red-first holds: with dispatcher.py, ha_adapter.py and request_store.py reverted to before the fix, it fails with 'expiry did not reconcile: … expired/pending'.
CI is green and the CLA is signed. Non-blocking follow-up still open: expired() is rescanned without a bound on every 2 s resolver tick.
keweichen
deleted the
factory/kc/recovery-71ad2538517fa594dc893e98-44df5c0ff2b8
branch
October 20, 2026 04:51
Collaborator
|
Post-merge review of #5741 (949505d) and #5742 (3155aa1). Both fixes are correct, and each new test fails when its fix lines are removed:
No blockers. These are worth follow-up issues:
Live witness: each PR body has an isolated — Sutando (Mars seat) |
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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): reconcile expired human-request mirrors
Review first:
src/runtime-api/dispatcher.py,src/runtime-api/ha_adapter.py,src/runtime-api/request_store.py,tests/runtime-api-dispatcher.test.py,tests/runtime-api-human-action.test.py,tests/runtime-api-request-expiry.test.py,tests/runtime-api-store.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): reconcile expired human-request mirrors, "request.cancel". Related results for reviewer comparison: fix(runtime-api): cancel mirrored human requirements #5741 Returned PRs were compared against current files and overlapping diffs: fix(runtime-api): cancel mirrored human requirements #5741: Related files, different patch; current-source reproduction and independent review remain required.. Title similarity alone is not equivalence; this is not exhaustive semantic coverage.
Upstream observed during evidence preparation:
0a6e182b94ebffd22c55b038c5ce8c08291b1a17. 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 changed logic and its recovery, resolver, store, reply-handler and projector callers. Expired rows remain retryable; identity checks and expiry run under the reentrant store lock. Existing terminal outcomes, answers and projection receipts are preserved. Transient mirror-write failure and subsequent successful retry were exercised.
review-scope: HEAD and parent match the supplied candidate and approved base. The reconstructed patch SHA-256 is 7ed4d82b74d46b950ce8bce0a4da3403e16945567042084f4f3acb0856414d9a, matching supplied evidence. Exactly seven allowed paths changed. Excluded cancellation, deadline, capability, schema, transport and desktop behavior remains unchanged. Git status stayed clean; git diff --check passed.
review-maintainability: The change keeps coordination in dispatcher.py, mirror ownership validation in ha_adapter.py and filtered lookup in request_store.py. It reuses HitlManager.expire and existing atomic persistence without introducing another lifecycle writer or reconciliation schema. Tests cover failure isolation, ownership guards and idempotence.
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 [local path] exit 0, during 2026-10-19 22:15:24–22:15:51 UTC. It launched unchanged python3 src/runtime-api/server.py and exercised bash bin/sutando over private Unix sockets with the supplied environment. For example, approval request --task-id task-rtapi-ed208d880e91 --action 'Local synthetic approval' --expires-in 6 returned approval-41ea4cf9abc2 pending; request get approval-41ea4cf9abc2 later returned expired. Equivalent elicitation and human-action journeys passed. Raw persisted records confirmed expiry before CLI reads. Full commands, outputs, timestamps and readiness are in monitor-journey-events.json.
monitor-repeatability: The independent witness stopped the daemon, crossed six request deadlines, and restarted python3 src/runtime-api/server.py against the same stores. Three pending runtime rows and three already-expired rows with active mirrors reconciled successfully. bash bin/sutando request get elicitation-9f0936a8f3ec returned status expired. A second restart preserved all recorded identities, revisions, deadlines, answers and projection receipts. Repeated production project() calls returned []; nine expiry edits produced zero duplicates. Commands and observations are in monitor-journey-events.json 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: The added journey test loaded the supplied environment into product subprocesses and exercised unchanged server.py and bin/sutando. At the default two-second resolver interval, approval, elicitation and human-action requests and matching requirements expired within 1.874 seconds after their deadlines. Requirements left active(); tasks stopped reporting waiting.
acceptance-2: Production projector with synthetic local delivery generated exactly one expired edit per requirement against its original target. Repeated projection passes emitted nothing further. Previous-revision owner actions were rejected without changing expired outcomes. Independently checked recorded payloads and replies.
acceptance-3: Real daemon restart reconciled all three types after downtime and already-expired runtime rows with active mirrors. Identities, deadlines and projection receipts survived; another restart added no revisions. The focused dispatcher test injected a transient HitlStore.save failure and verified isolation, periodic retry and idempotence.
acceptance-4: Unrelated pending requests, requests without explicit deadlines before adapter TTL, and previously resolved requests remained unchanged. Selected answer Beta remained retrievable after restart. Non-owner and stale replies were rejected. Task/result payloads and populated second-workspace state were preserved.
check-request-expiry-journey: python3 tests/runtime-api-request-expiry.test.py: exit 0; two journeys passed in 32.954 seconds, 2026-10-19 22:07:29–22:08:02 UTC.
check-request-domain-regressions: python3 tests/runtime-api-dispatcher.test.py: exit 0 in 1.610 seconds; expiry recovery, transient-write retry and repeated-success checks passed.
check-human-action-regressions: python3 tests/runtime-api-human-action.test.py: exit 0 in 0.287 seconds; all 16 tests passed.
check-request-store-regressions: python3 tests/runtime-api-store.test.py: exit 0 in 0.288 seconds; durable retry filtering and preservation checks passed.
coverage: Reran all four suites with python3 -m coverage run --rcfile=.coveragerc. diff-cover against 8e6ebd5 with --fail-under=95 measured 21/21 changed executable lines covered: 100%, zero missing. Commands and measurements: [local path]
runtime-evidence: All ten daemon starts across ordinary and coverage runs reached request.list readiness in 0.285–0.432 seconds and were reaped with exit 0. Commands, PIDs, timestamps and startup logs are recorded in [local path] independently checked journey receipts are in verify-runtime-summary.json.
scope: HEAD matched the pinned candidate before and after verification; tracked/untracked status remained clean. Exactly the seven allowed paths differ from the approved base. Entry points and coverage configuration are unchanged; git diff --check passed.
Test plan
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Canonical review scanner:
Verified candidate:
9fa42a7b55487802e194895527c90fb0c489f30b. Publication commit:29b7e44dab603bc81daaa7f419057c87028bb0dc. Both have tree0a056947810747162949f35b28ebe2107fe99ecd; only factory-authored commit metadata changed. Acceptance evidence refers to the verified candidate; the scanner ran on the publication tree.