merge queue: checking #818 on main (25b1257), stacked on #821 - #896
Closed
mergify[bot] wants to merge 17 commits into
Closed
mergify[bot] wants to merge 17 commits into
mergify[bot] wants to merge 17 commits into
Conversation
…nt event waits use one blocking listener and an explicit callback contract. Review lane: behavior Review unit: product-skill Safety invariant: Each wait owns only its subscription and receipts; it never mutates a producer or another wait, and unsupported delivery remains explicit. Effectiveness measurement: Real subprocess/socket tests assert concurrent routing and zero repeated status queries; actual harness/CI delivery is separately evidenced or blocked. Slice rationale: This slice has one review unit and one claim; source adapters and policy selection remain separately reviewable. Architectural effect: Create a reusable skill-owned blocking event-consumer boundary. Goal: Concurrent event waits use one blocking listener and an explicit callback contract. Motivation: Replace repeated bespoke listeners and status polling with reusable event-driven waits. Alternative considerations: Keep the hardcoded one-off (fails reuse); use background polling (violates no-poll instruction); add a universal daemon (unneeded lifecycle complexity). Chosen per-wait process must survive the concurrency spike. Implementation details: Create product/skills/event-wait/ with SKILL.md, a small Python stdlib runner and tests, plus references/examples needed by the package. Review unit product-skill only; docs/ecosystem.md inventory row may ride along. Do not edit corpus principle or engine hook policy in this slice. One lightweight blocking process per wait; no generic broker/daemon. Caller supplies a validated JSON spec with unique wait_id, exact source identity, exact subject selector, absolute deadline, receipt destination and declared wake ownership. Do not hardcode a workflow ID, home path, repo, CLI, secret or harness session. Require distinct private receipt paths and exclusive per-ID ownership (no check-then-create race). Separate event_received from wake_delivered; process output by itself is not proof chat resumed. One terminal receipt per invocation, isolated cancellation, cleanup only own resources. Bounded frame/record/output sizes, redacted errors, total monotonic deadline including continuous unrelated traffic, nonzero errors for malformed stream/EOF/timeout. Do not turn source failure into target failure or success. Do not promise exactly-once delivery across crashes. Source interfaces: implement reusable JSON event stream consumption and configurable framed-JSON Unix socket subscription; channel, subject fields, terminal statuses, request envelope and response mapping are validated source configuration, not arbitrary expressions. Subscribe/connect before at most one optional initial snapshot, buffer/reconcile in-flight events and deduplicate event identity. No repeated snapshot queries, timers that query status, hidden polling commands, reconnect rescan loop or pretending a synthetic local emitter is live CI. Keep protocol examples under the package. Invoker-specific CLI changes are outside scope: a socket specification can describe this source without invoking an external project CLI. Read the actual Invoker envelope shape when preparing a local example; generic SKILL.md must remain consumer-neutral per create-skill. Harness recipes: use actual available native background completion where documented. Claude asyncRewake is an optional explicit running-session integration; distinguish its wake exit code from job outcome, bounded timeout and ordinary async no-idle-wake behavior. Cursor local SDK background-subagent follow-up is scoped to SDK-owned runs. Codex current tool-level subagent notifications are scoped to a process started/awaited by that child; do not assume shell process handles transfer across agents or that public Codex CLI always exposes notify_on_output. If no supported wake mechanism, report session_wake_unsupported instead of silently polling or falsely saying armed. Distinguish source_ready and callback_ready acknowledgments. Before broad implementation, run a disposable spike through this contract: two processes subscribe to a local producer and complete in reverse order without cross-delivery. Keep the finding, not a second production implementation. Then ship tests for at least 20 concurrent waits, unrelated events, duplicates, wrong target, completion before attach via snapshot, completion during snapshot, immediate completion before armed, timeout under continuous traffic, cancellation A leaves B alive, malformed/oversize/truncated frames, EOF/disconnect, reused wait ID/path, read-only sources, and callback unsupported/failure. Count producer status requests to prove at most one initial request and no post-start polling. Tests use actual sockets/subprocesses, not implementation-mirroring mocks. A synthetic source proves protocol only. Run the new focused test suite, python3 scripts/check_skill_file_refs.py, python3 scripts/check_skill_test_coverage.py, and three-harness install checks using ./install.sh in an isolated temporary home. Verify all three links resolve to the same skill and the CLI executes via each. Do not relink the user's other live skills from an ephemeral worktree. Prepare a retained source path/install instruction for this skill after review; user-home activation only if the repository's normal safe installer can target this skill without modifying unrelated links. Run make-pr preflight on the actual diff. If actual harness access is available, exercise one real synthetic-source-to-idle-parent callback and capture the parent result; otherwise name this explicit live-path blocker and do not advertise that harness as tested. Non-goals: No unrelated repo edits, new public receiver, remote webhook mutation, global hook setting changes, merges or deployments. Files: product/skills/event-wait/SKILL.md, product/skills/event-wait/scripts/wait_event.py, product/skills/event-wait/tests/test_wait_event.py, docs/ecosystem.md Change types: Create reusable skill code, direct tests, and package documentation. Layer: domain Feature state: active Acceptance criteria: Focused tests and applicable repo preflight must finish with exit code 0 and real output; at least two concurrent independent waits prove routing. Unsupported live sources are reported explicitly rather than scored as passing. Verify: Execute the scope-specific tests described above, inspect real receipts, and run repository make-pr preflight for product-skill before PR publication. Exit code: 1
…for status Adds a reusable skill that waits for one event from a push source with one blocking process per wait, replacing hand-rolled listeners and status-polling loops. The runner attaches to the source first, prints an `armed` acknowledgment the caller must read before triggering the work, sends at most one optional snapshot request, then blocks. A live event is matched the moment it arrives, including while the snapshot reply is in flight, so a completion inside that window is never parked behind a reply that may never come. Nothing asks the producer again: the tests count producer-side requests and assert zero bytes reach a source that declares no snapshot. Two sources ship: a configurable framed-JSON Unix socket (2/4/8-byte big- or little-endian length prefix, or lines) and a read-only JSON stream from a child process, file or fifo. Every selector is a literal field path plus a literal value, so an untrusted producer payload can never become a command, and the wake command is taken from the spec alone. `wait_id` and `receipt_path` are both taken with an exclusive create, leaving no check-then-create gap, and the receipt directory must be private. Each wait cleans up only its own claim. The receipt keeps `event_received` and `wake_delivered` apart, because process output is not proof a chat resumed. With no supported wake mechanism the runner reports `session_wake_unsupported` rather than polling or claiming an unproven path; `source_ready` and `callback_ready` are separate acknowledgments, and a wake with no readiness probe is reported as unproven, not ready. Verified with 35 tests over real sockets and subprocesses, including 20 concurrent waits completing in reverse order without cross-delivery, cancellation isolation, malformed/oversize/truncated frames, disconnects, reuse conflicts and callback failure. Installed into an isolated temporary home and executed through all three harness links. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nt event waits use one blocking listener and an explicit callback contract. Review lane: behavior Review unit: product-skill Safety invariant: Each wait owns only its subscription and receipts; it never mutates a producer or another wait, and unsupported delivery remains explicit. Effectiveness measurement: Real subprocess/socket tests assert concurrent routing and zero repeated status queries; actual harness/CI delivery is separately evidenced or blocked. Slice rationale: This slice has one review unit and one claim; source adapters and policy selection remain separately reviewable. Architectural effect: Create a reusable skill-owned blocking event-consumer boundary. Goal: Concurrent event waits use one blocking listener and an explicit callback contract. Motivation: Replace repeated bespoke listeners and status polling with reusable event-driven waits. Alternative considerations: Keep the hardcoded one-off (fails reuse); use background polling (violates no-poll instruction); add a universal daemon (unneeded lifecycle complexity). Chosen per-wait process must survive the concurrency spike. Implementation details: Create product/skills/event-wait/ with SKILL.md, a small Python stdlib runner and tests, plus references/examples needed by the package. Review unit product-skill only; docs/ecosystem.md inventory row may ride along. Do not edit corpus principle or engine hook policy in this slice. One lightweight blocking process per wait; no generic broker/daemon. Caller supplies a validated JSON spec with unique wait_id, exact source identity, exact subject selector, absolute deadline, receipt destination and declared wake ownership. Do not hardcode a workflow ID, home path, repo, CLI, secret or harness session. Require distinct private receipt paths and exclusive per-ID ownership (no check-then-create race). Separate event_received from wake_delivered; process output by itself is not proof chat resumed. One terminal receipt per invocation, isolated cancellation, cleanup only own resources. Bounded frame/record/output sizes, redacted errors, total monotonic deadline including continuous unrelated traffic, nonzero errors for malformed stream/EOF/timeout. Do not turn source failure into target failure or success. Do not promise exactly-once delivery across crashes. Source interfaces: implement reusable JSON event stream consumption and configurable framed-JSON Unix socket subscription; channel, subject fields, terminal statuses, request envelope and response mapping are validated source configuration, not arbitrary expressions. Subscribe/connect before at most one optional initial snapshot, buffer/reconcile in-flight events and deduplicate event identity. No repeated snapshot queries, timers that query status, hidden polling commands, reconnect rescan loop or pretending a synthetic local emitter is live CI. Keep protocol examples under the package. Invoker-specific CLI changes are outside scope: a socket specification can describe this source without invoking an external project CLI. Read the actual Invoker envelope shape when preparing a local example; generic SKILL.md must remain consumer-neutral per create-skill. Harness recipes: use actual available native background completion where documented. Claude asyncRewake is an optional explicit running-session integration; distinguish its wake exit code from job outcome, bounded timeout and ordinary async no-idle-wake behavior. Cursor local SDK background-subagent follow-up is scoped to SDK-owned runs. Codex current tool-level subagent notifications are scoped to a process started/awaited by that child; do not assume shell process handles transfer across agents or that public Codex CLI always exposes notify_on_output. If no supported wake mechanism, report session_wake_unsupported instead of silently polling or falsely saying armed. Distinguish source_ready and callback_ready acknowledgments. Before broad implementation, run a disposable spike through this contract: two processes subscribe to a local producer and complete in reverse order without cross-delivery. Keep the finding, not a second production implementation. Then ship tests for at least 20 concurrent waits, unrelated events, duplicates, wrong target, completion before attach via snapshot, completion during snapshot, immediate completion before armed, timeout under continuous traffic, cancellation A leaves B alive, malformed/oversize/truncated frames, EOF/disconnect, reused wait ID/path, read-only sources, and callback unsupported/failure. Count producer status requests to prove at most one initial request and no post-start polling. Tests use actual sockets/subprocesses, not implementation-mirroring mocks. A synthetic source proves protocol only. Run the new focused test suite, python3 scripts/check_skill_file_refs.py, python3 scripts/check_skill_test_coverage.py, and three-harness install checks using ./install.sh in an isolated temporary home. Verify all three links resolve to the same skill and the CLI executes via each. Do not relink the user's other live skills from an ephemeral worktree. Prepare a retained source path/install instruction for this skill after review; user-home activation only if the repository's normal safe installer can target this skill without modifying unrelated links. Run make-pr preflight on the actual diff. If actual harness access is available, exercise one real synthetic-source-to-idle-parent callback and capture the parent result; otherwise name this explicit live-path blocker and do not advertise that harness as tested. Non-goals: No unrelated repo edits, new public receiver, remote webhook mutation, global hook setting changes, merges or deployments. Files: product/skills/event-wait/SKILL.md, product/skills/event-wait/scripts/wait_event.py, product/skills/event-wait/tests/test_wait_event.py, docs/ecosystem.md Change types: Create reusable skill code, direct tests, and package documentation. Layer: domain Feature state: active Acceptance criteria: Focused tests and applicable repo preflight must finish with exit code 0 and real output; at least two concurrent independent waits prove routing. Unsupported live sources are reported explicitly rather than scored as passing. Verify: Execute the scope-specific tests described above, inspect real receipts, and run repository make-pr preflight for product-skill before PR publication. Exit code: 0
… This worktree has no ephemeral handoff artifacts. Review lane: proof Safety invariant: This gate reads file names and never deletes or commits files. Effectiveness measurement: Fail explicitly if an ephemeral handoff path remains in the worktree inventory. Slice rationale: Terminal read-only artifact gate required for this implementation workflow. Architectural effect: Prevent transient planning artifacts from entering publication. Goal: Check handoff absence. Motivation: Keep PR contents reviewable. Alternative considerations: Automatic deletion rejected because it could remove caller work. Implementation details: Inspect git tracked/untracked file names and fail on known ephemeral artifacts. Non-goals: No mutations or global inventory reads. Layer: domain Feature state: active Acceptance criteria: HANDOFF_ARTIFACT_CHECK: PASS and exit code 0. Exit code: 0
…-aed59aae6-e4580479 — Review claim: This worktree has no ephemeral handoff artifacts. Review lane: proof Safety invariant: This gate reads file names and never deletes or commits files. Effectiveness measurement: Fail explicitly if an ephemeral handoff path remains in the worktree inventory. Slice rationale: Terminal read-only artifact gate required for this implementation workflow. Architectural effect: Prevent transient planning artifacts from entering publication. Goal: Check handoff absence. Motivation: Keep PR contents reviewable. Alternative considerations: Automatic deletion rejected because it could remove caller work. Implementation details: Inspect git tracked/untracked file names and fail on known ephemeral artifacts. Non-goals: No mutations or global inventory reads. Layer: domain Feature state: active Acceptance criteria: HANDOFF_ARTIFACT_CHECK: PASS and exit code 0.
is_duplicate recorded every envelope-matching event id before anything checked the subject. On a shared channel a neighbour's event id landed in the seen set, so when this wait's own completion arrived under an id that was only ever unique within a subject, it was skipped as a duplicate and the wait timed out on a job that had already finished. Unrelated traffic also evicted real entries through the seen-id cap. The subject check now happens first, for both stream events and the snapshot response, so only our own subject's ids are ever recorded. Test: RoutingTests.test_another_subjects_event_id_never_masks_our_completion fails on the parent commit with outcome 'timeout' and passes here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… its head did not move Exit code: 0
…ead PRRT_kwDOT3uYWs6kyhIt after PR head changes Exit code: 0
…c-259dcb40 — Resolve bot review thread PRRT_kwDOT3uYWs6kyhIt after PR head changes
… test; Solution: Repair PR #821: failed_checks: test;
… the person The runner's per-run table counted wrong-check-reflect as "silent 2,582, spoke 0" on Claude. That number cannot tell a mute hook from a working one: the Stop hook never prints, it queues a background judge job, and the verdict arrives on a later PostToolUse. The live cache showed the real losses were downstream: 39 hit verdicts for this hook sat undelivered, 25 of them on jobs queued with an empty transcript path, which no drain can ever find. Each stage now writes one catstack.hook_event.v1 row with a reason: - judge_skipped (wrong-check-reflect): stop_hook_active, gate_off, empty_reply, already_prompted, user_asked_reflect, judge_child, bad_payload - judge_queued (llm-judge, every hook): transcript or no_transcript - judge_finished (llm-judge): hit, clean or unchecked Delivery was already recorded by drain. Rows join on the job id. A job queued with no transcript also prints catstack-hook-error, so the runner classifies the run as caught_error and hook-health surfaces it on the next prompt instead of the verdict vanishing. report.py --judge prints the per-hook funnel plus leak counts (no_transcript, stuck, undelivered, undelivered_hits) older than --grace (default 1h); --check exits 1 on any leak. Stage rows carry no rule_id, so the existing rule table ignores them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Change-Id: Ic6eeb8c30dabc90c4d11270e98ef4d1a2eecddfd
main landed this same skill through #790, so every event-wait file came back as an add/add conflict. #790 carried the lazy Decoder.feed() change and a SKILL.md paragraph this branch never had, and it logs the OSError when a test producer fails to hang up instead of swallowing it. This branch carries three fixes #790 never saw. Resolved by taking main's file as the base, then re-applying only what is unique here, so neither side loses work: - _body_path() accepts an explicit [] again. references/framed-socket-source.md documents [] for a source whose records are already the body, and dig() already reads it that way; only the validator rejected it. - is_subject() now gates is_duplicate(). Event ids are unique per subject, not per channel, so a neighbour's event could otherwise record an id ours would reuse and mask our own completion. - The wake command's stdout goes to stderr. stdout carries the armed record and the receipt, and a chatty wake would be parsed as a malformed record. SKILL.md took main's copy unchanged: diffing the two sides showed main's is a strict superset, so this branch had nothing to re-apply there. Each of the three fixes was re-checked by reverting it alone and watching its test fail: 'timeout' != 'matched' for the subject gate, a JSONDecodeError off the record stream for the wake, and a spec_type SpecError for the empty body_path. Both sides' tests are kept, including the two pairs that cover the same ordering guarantee through different error codes (invalid_json here, frame_too_large on main). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…air-bot-thread-pr-791-4d21848
…ub reports a merge conflict against main; Exit code: 0
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
1 participant
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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#818 is queued for merge on branch main (25b1257).
Stacked behind 1 pull request queued ahead of this batch, not part of it. These checks run on a tip that also carries its commits, so a failure here can come from it as much as from #818.
Queued ahead of this batch:
This pull request has been created by Mergify to speculatively check the mergeability of #818.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
admin-bypassfor merge:check-success = lintcheck-success = testcheck-success = validateRequired conditions to stay in the queue:
-draftbase=mainlabel=admin-bypass