Repository navigation
docs(adr): accept 0038 snapshot capture source routing - #4255
Conversation
Apex SentinelVerdict: Since the last review: 0 new · 0 still open · 0 no longer raised No findings. Not in the reviewers' prompt (3 files)
Reviewed with this repository's own Sentinel configuration ( Reply under an inline comment with Earlier reviewsReview of
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
I found no code defects at cb15409, and all 9 checks pass (this docs-only diff touches no runtime job route). There are no conflicts. Before merge, please decide whether to accept the proposal now or keep it in #4205 until it is accepted. If accepted, move the migration and guard inventory to #4205 and fix the "if verified" wording for unknown codes. Not blocking, and you can take or leave these: the single-source list at 0038:112 binds WebDriver, Revyl, HarmonyOS, Linux, TV CDP and composing providers to router channels, but #4205 requirement 4 names only iOS local, iOS provider, Android, macOS and web. A clearer rule would be that owners with more than one channel route through the router, and every owner stamps identity through the capture-kit helper and declares a registry row. The file is also slightly over the ~8 KB aim in FORMAT.md, and moving the migration list to #4205 fixes that. Simplicity question: could the router and generation circuits stay only for owners with an alternate (local and Limrun iOS Simulators), with the contracts producer registry kept for every producer, and one-channel owners getting just the shared identity-stamp helper plus their registry row? That would still close the placeholder ports, split eligibility, three fallback settlers and six producer tables, without moving nine single-source providers through a planner that never chooses. #4205 requirement 4 and its completion conditions would need to name multi-channel owners only first, or the ADR's rationale would need to accept the wider scope explicitly. The open inline threads still apply for the migration checklist, unknown-code wording and one lower-priority thread on ADR timing. Two do not apply, so you can resolve them: proof reuse (proof is passed into acquire and absence is never reused across captures) and quiet time (it is already derived per capture from the served producer). |
…lan to #4205 - Route only owners with more than one channel through the router. Every producer stamps identity through capture-kit and declares a registry row. - Give unknown refusal codes generation scope without a verification condition. - Add the runner-session scope from #4241. - Derive quiet time per capture from the served producer. - Move the migration plan and the guard inventory to #4205.
Keep only the decisions, reasons, rejected alternatives, and consequences. Move the source interface, planner mechanics, and per-owner inventory to #4205. Keep the constraints that the code cannot show: unproven is not clear, cancellation is terminal, no circuit without a verified generation, and a served surface owns its identity. Amend the ADR 0004 status to point at 0038.
Limit the no-generation rule to app-generation refusals, scope surface proof to captures that need it, keep integrated checks for unproven absence, and scope the rationale to multi-source owners.
thymikee
changed the title
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
This PR is ready at fcf318e. The findings from the earlier review (#4255 (comment)) are fixed, and I found no new problems in the ADR. Not blocking, take or leave: the "One-source owners" rule says the owner stamps identity through the capture-kit helper, but that helper does not exist on main yet (#4205 step 4 adds it), so please add it to the "Open gaps" line, for example "the router, the identity-stamp helper, and the producer registry do not exist yet" (https://github.com/callstack/agent-device/blob/fcf318e/docs/adr/0038-snapshot-capture-source-routing.md#L26). All 9 checks pass, and the diff only touches docs/adr files, so no runtime job route applies. There are no conflicts. Nothing else must happen before merge. The open thread asking for a "Rules at a glance" section (#4255 (comment)) does not apply, because FORMAT.md makes sections optional and ADR 0036 has none, so it can be resolved. One question: #4205 links to a commit on this PR branch; will that link still resolve after a squash merge? |
thymikee
added
the
ready-for-human
|
Addressed at 74e1a6e:
|
Accepts ADR 0038, "Snapshot capture source routing", for #4205 as a one-page decision record. It has six sections: Status, Context, Decision, Rationale, Rejected alternatives, and Consequences.
Docs only. Codex reviewed the rewrite for meaning drift against the proposal and the code, and its fixes are applied.