Repository navigation
feat(renderer): report forwardFrames delivery state via forwardStatus - #85
Merged
Merged
Conversation
The delivery state of a forwardFrames registration was unobservable. Per-frame
defects were dropped by a `.catch(() => {})` in handlePaint, and a refused
registration was returned as an inert handle indistinguishable from a live one.
A forward could therefore die and stay dead while paint, the native sender, the
preview window and `droppedReason` all stayed healthy — the only symptom being a
monitor window that silently went black.
- Add the `forwardStatus` event and `FrameForwardOptions.onStatus`. Both report
state *changes* only — first success, first failure, a change of failure
reason, and recovery — so a stuck target cannot emit 30 identical events a
second (same dedupe stance as `frameDropped`).
- Add `FrameForward.active`, backed by set membership, so a refused
registration, a `dispose()` and a destroyed target are all visible to the
caller.
- Route a rejected forward to `send-failed`: the import half is already
Result-wrapped by the primitive, so only the send half can reject.
- Swallow throws from either observer. Both run inside a promise continuation
where an escaping throw becomes an unhandled rejection, and neither may break
forwarding; the `"error"` event is deliberately not used, since it throws
when nobody is listening — exactly the case a diagnostic channel must survive.
- Update API.md and three skills, which documented the now-false claims that
forward defects are discarded and that registration is silent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TErgwokxkbfxexfUNeuMoW
Baseline: two agents were handed the real incident (some forwarded previews dead for the session, Syphon unaffected, empty logs) without a skill. Both opened with an invented hypothesis — one proposed unbounded in-flight promise pile-up and wanted to patch `handlePaint` with a pending counter before reading anything the library already publishes. Neither mentioned `FrameForward.active`, neither considered that an idle source emits no paint at all, and neither used the frozen-frame vs blank-placeholder distinction that splits the search space in half. The skill closes exactly those gaps: read `active` and `forwardStatus` first and let the outcome table name the layer, confirm the source is actually painting before blaming the forward path, and read the symptom (frozen vs blank) to pick a side. It also carries the measured warning that probing the paint loop hides this bug — instrumentation moved a real reproduction from 3-in-5 launches to 0-in-22. With the skill, a re-run reasoned from Syphon's health to "only forwarding died", checked `active`, planned around all four outcomes, and explicitly declined to add a temporary paint-loop probe. Also cross-links from the two neighbouring skills and lists the skill in both READMEs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TErgwokxkbfxexfUNeuMoW
The second verification run found the loophole: it checked the consumer's installed `.d.mts`, saw that the pinned 0.14.0 exposes neither `FrameForward.active` nor `forwardStatus`, and concluded the skill's primary diagnostic was unavailable — then spent its plan designing a substitute out of `frameDropped` / `error` and hand-rolled call-site logging, none of which can observe the forward path at all. State the version gate explicitly: detect it from the installed types rather than the checked-out source, treat the upgrade as the fix rather than a prerequisite, and say plainly that the fallback seams are coarse and blind to forwarding — so nobody invests in the substitute again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TErgwokxkbfxexfUNeuMoW
Verifying a library change against the consumer that motivated it currently requires merging and releasing first — the publish job only runs on a release-please release, and it publishes through npm OIDC trusted publishing, which cannot be driven from a branch. That ordering is backwards for a change whose whole purpose is to make a consumer-side failure observable. pkg.pr.new serves an installable tarball per commit from its own host, so no version is bumped and nothing reaches the npm registry. Only the renderer package is published: core and the native addon are unchanged by renderer-only work, and leaving native out keeps the Rust matrix off the PR path. `--pnpm` is what makes the single-package publish coherent — `pnpm pack` rewrites the `workspace:*` dependency on core to its concrete version so the preview resolves it from npm, where `npm pack` would leave the protocol literal and produce an uninstallable tarball. Requires the pkg.pr.new GitHub App on the repository; the workflow itself needs no token, since the app posts the preview URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TErgwokxkbfxexfUNeuMoW
naporin0624
force-pushed
the
feat/forward-status-observability
branch
from
August 14, 2026 09:57
3da6a1c to
c247354
Compare
commit: |
Adding a preview-release workflow put a fourth unpinned, over-permissioned workflow into the repository, so fix the class rather than the instance: zizmor reported 50 findings across the four, 29 of them high severity. - Pin all 26 action references to commit SHAs with the tag in a comment. A mutable tag is a supply-chain hole: whoever can move `v4` runs code in a job that, until this commit, held write access to the repository. - Give every checkout `persist-credentials: false`. Nothing here pushes over git, so leaving the token in `.git/config` only widened what a compromised build step could reach. - Move publish.yml's `contents: write` / `pull-requests: write` / `id-token: write` off the workflow and onto the two jobs that need them: release-please opens the release PR and cuts the release, publish holds the npm OIDC credential. The native build matrix, which runs the most third-party code, drops to read-only. ci.yml and platform-smoke.yml declare `contents: read`. - Keep it from regressing with a Workflow Audit job, itself pinned so the ruleset cannot drift under a green check. The one remaining finding is suppressed inline: zizmor reads pkg-pr-new's "publish" as a registry upload, but that step has no credential to move to trusted publishing — the GitHub App authorizes it and nothing reaches npm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TErgwokxkbfxexfUNeuMoW
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.
なぜ
Cannelloni で「プレビューだけが無言で死ぬ」事故を実機計測で確認した。3 時間のセッションでも、5 回の起動中 3 回でも再現した状態は次のとおり:
paintframeDropped/errorsharedTexture.sendSharedTexturepaint も native sender も preview も
droppedReasonも全部健全なまま、監視ウィンドウだけが黙って黒くなる。切り分けようにも materials が無い:ForwardDefectはhandlePaintの.catch(() => {})で捨てられている調査には計測コードの注入が必要で、しかも計装するとレースが消えた (計装前 3/5 失敗 → 計装後 22/22 成功)。ライブラリが状態を報告しない限り、この故障は現場で追えない。
なにを
forwardStatusイベント +FrameForwardOptions.onStatusどちらも 状態遷移だけ を通知する — 初回成功 / 初回失敗 / 失敗理由の変化 / 復帰。30fps で同じ状態を垂れ流さない (
frameDroppedと同じ dedupe 方針)。遷移だけを出すので、ログを素直に読むだけで停止区間になる:
{ ok: false }の後に{ ok: true }が無ければ、そのターゲットはその時刻から今も暗い。FrameForward.active登録が成立したかどうか。set のメンバーシップを見る getter なので、登録拒否・
dispose()・target の後からの破壊をすべて反映する。設計上の判断
"error"イベントは使わない。 購読者ゼロだと throw するため、まさに「誰も見ていない」状況を生き延びる必要がある診断チャネルには不適。console.errorに落とす)。send-failedとして報告。 import 側は primitive が Result で包んでいるので、reject し得るのは send 側だけ。テスト
新規 9 件 (renderer 201 件 / core 33 件すべてパス)。dedupe・復帰・理由変化・reject 経路・in-flight dispose・
activeの 4 状態を個別に固定した。typecheckクリーン、lint0 errors、oxfmt済み。ドキュメント
docs/API.mdと 3 つの skill に「defect は driver が捨てる」「登録は無言」と書いてあり、この PR で嘘になるため全部更新した。🤖 Generated with Claude Code
https://claude.ai/code/session_01TErgwokxkbfxexfUNeuMoW
追記:
diagnosing-dead-frame-forwardsskill同じ事故に次に当たった人が 2 手で層を特定できるよう、診断 skill を TDD で追加した。
RED (skill 無しのベースライン, 2 エージェント): どちらも根拠の薄い仮説から始めた。片方は in-flight promise の積み上がりを疑い、ライブラリが既に公開している信号を読む前に
handlePaintへ pending カウンタを足すと提案した。両者ともFrameForward.activeに触れず、「アイドルなソースはそもそも paint を出さない」を考慮せず、「フリーズ画像 / 空白プレースホルダ」の弁別も使わなかった。GREEN (skill 有り): Syphon が生きている事実から「死んだのは転送だけ」と推論し、
activeを確認し、4 つの結果すべてに分岐を用意し、paint ループへの一時プローブ追加を明示的に見送った。skill には計測済みの警告も入れた — 計装するとこのバグは隠れる (実測: 再現率 3/5 → 0/22)。