Repository navigation
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
16 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/adr/0009-apple-platform-consolidation.md">
<violation number="1" location="docs/adr/0009-apple-platform-consolidation.md:106">
P3: The visionOS discovery test reference points to the implementation module instead of its test file. Refer to `inventory-classification.test.ts` so readers can locate the claimed unit coverage.</violation>
</file>
<file name="docs/adr/0006-daemon-rpc-protocol-version.md">
<violation number="1" location="docs/adr/0006-daemon-rpc-protocol-version.md:31">
P2: State the required outcome of the comparison: an incompatible peer must be refused before sending command RPC. As written, this decision only requires comparing the values, so readers could interpret the comparison as diagnostic rather than as a pre-dispatch compatibility gate.</violation>
<violation number="2" location="docs/adr/0006-daemon-rpc-protocol-version.md:90">
P3: This overstates the released-surface gate: newly added wire declarations are accepted without a protocol bump or `compatibleChanges` entry, but this sentence says every change since the release needs one. Limit the rule to changed or removed existing declarations, and state that additions are additive if that distinction is intentional.</violation>
</file>
<file name="docs/adr/0011-interaction-guarantee-contract.md">
<violation number="1" location="docs/adr/0011-interaction-guarantee-contract.md:13">
P2: This glance rule overstates the sharing requirement: `responseIdentity` is explicitly closed through per-path capability work below. Limit the rule to shared implementations where the guarantee has a common implementation, and keep path-specific capabilities separate.</violation>
<violation number="2" location="docs/adr/0011-interaction-guarantee-contract.md:383">
P3: This says scroll keyboard probing waits for known healthy AX, but the runner probes while AX health is `unknown` too. Document the actual condition so the ADR does not mislead future policy changes.</violation>
</file>
<file name="docs/adr/0010-error-system.md">
<violation number="1" location="docs/adr/0010-error-system.md:81">
P2: This claims every error carries both diagnostic fields, but normalized errors can omit them and remote errors intentionally replace `logPath` with `logPathUnavailable`. Document these fields as conditional so consumers do not assume a readable path is always present.</violation>
</file>
<file name="docs/adr/0017-parameterized-recorded-inputs.md">
<violation number="1" location="docs/adr/0017-parameterized-recorded-inputs.md:10">
P2: The session-event presentation does not retain `${VAR}`: it redacts fill positionals as `<text>` and omits the value. Describe event data as redacted instead of claiming it uses the placeholder.</violation>
<violation number="2" location="docs/adr/0017-parameterized-recorded-inputs.md:123">
P2: This amendment retains each non-empty literal in session memory, contradicting the data-flow boundary's claim that the literal exists only on the live request path. Clarify that the literal is retained in the ephemeral, non-serialized session registry and state its security/lifetime limits consistently.</violation>
</file>
<file name="docs/adr/0014-session-ref-frame-lifetime.md">
<violation number="1" location="docs/adr/0014-session-ref-frame-lifetime.md:25">
P2: This summary overstates partial issuance: empty `find`/diff/divergence results do not activate or supersede a frame. Qualify the rule with the non-empty ref-and-epoch condition so the at-a-glance contract matches the implementation and the detailed rule below.</violation>
</file>
<file name="docs/adr/0001-provider-first-integration-scenarios.md">
<violation number="1" location="docs/adr/0001-provider-first-integration-scenarios.md:187">
P3: This list omits several exclusion buckets that the progress model reports, including cloud artifact lookup, daemon lifecycle, local contention, and Apple screenshot/private-AX options. List every bucket or label these six as examples so the ADR does not present an incomplete classification as exhaustive.</violation>
</file>
<file name="docs/adr/0002-persistent-platform-helper-sessions.md">
<violation number="1" location="docs/adr/0002-persistent-platform-helper-sessions.md:19">
P2: This rule incorrectly requires every Android touch command to use the bundled helper, excluding the provider-native injection path documented in ADR 0013. Limit it to instrumentation-backed touch injection.</violation>
</file>
<file name="docs/adr/0013-unified-gesture-plans.md">
<violation number="1" location="docs/adr/0013-unified-gesture-plans.md:129">
P3: `One-contact plans` is too broad here: target-authored drag includes source, movement, and destination-hold samples, while the next paragraph says those authored samples are preserved. Qualify this as `canonical one-contact endpoint plans` so the ADR does not contradict its drag schedule.</violation>
</file>
<file name="docs/adr/0005-ios-runner-interaction-lifecycle.md">
<violation number="1" location="docs/adr/0005-ios-runner-interaction-lifecycle.md:44">
P2: The preflight policy omits the unconditional exemptions for `activate`, `terminate`, and `targetReset`. Document these alongside the startup, readiness-probe, and recent-mutation exceptions in both the summary and Decision.</violation>
<violation number="2" location="docs/adr/0005-ios-runner-interaction-lifecycle.md:56">
P2: This allowlist is incomplete: `desktopScroll` and `gesture` also qualify for the healthy-mutation preflight skip. Add both commands so the ADR matches `RUNNER_COMMAND_TRAITS`.</violation>
</file>
<file name="docs/adr/0008-command-descriptor-registry.md">
<violation number="1" location="docs/adr/0008-command-descriptor-registry.md:61">
P2: This decision still names a `capability` descriptor facet, but the amended model and current descriptor use typed runtime declarations under `platformExecution`. Name the actual runtime-use facet so new command additions follow the accepted contract.</violation>
<violation number="2" location="docs/adr/0008-command-descriptor-registry.md:100">
P2: This consequence points to a nonexistent `descriptor.execute` field. The current extension point is the descriptor's `platformExecution` declaration, so name that property instead.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| daemon, cannot safely communicate over the HTTP RPC boundary for existing commands. Bump it for | ||
| breaking changes to: | ||
| The daemon HTTP `/health` payload advertises integer `rpcProtocolVersion` beside service and package version metadata. | ||
| Before JSON-RPC requests, remote clients compare this value. |
There was a problem hiding this comment.
P2: State the required outcome of the comparison: an incompatible peer must be refused before sending command RPC. As written, this decision only requires comparing the values, so readers could interpret the comparison as diagnostic rather than as a pre-dispatch compatibility gate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0006-daemon-rpc-protocol-version.md, line 31:
<comment>State the required outcome of the comparison: an incompatible peer must be refused before sending command RPC. As written, this decision only requires comparing the values, so readers could interpret the comparison as diagnostic rather than as a pre-dispatch compatibility gate.</comment>
<file context>
@@ -4,83 +4,94 @@
-daemon, cannot safely communicate over the HTTP RPC boundary for existing commands. Bump it for
-breaking changes to:
+The daemon HTTP `/health` payload advertises integer `rpcProtocolVersion` beside service and package version metadata.
+Before JSON-RPC requests, remote clients compare this value.
+
+Package `version` is diagnostic only and must not control compatibility.
</file context>
| Before JSON-RPC requests, remote clients compare this value. | |
| Before JSON-RPC requests, remote clients compare this value and refuse incompatible peers before sending command RPC. |
There was a problem hiding this comment.
The PR keeps the base sentence: "Remote clients compare this value before sending JSON-RPC requests" (base line 21). The base does not state a refusal outcome here, so the suggestion adds meaning. If the base is wrong, docs/adr-stale-facts (umbrella #3744) handles it.
|
|
||
| - Classify every dispatch-path × guarantee cell in the typed registry. | ||
| - Link each gap waiver to a tracking issue. Keep command-scoped claims within the path's command set. | ||
| - Give each TypeScript rule one shared implementation. Prove runner-side equivalents through golden fixtures. |
There was a problem hiding this comment.
P2: This glance rule overstates the sharing requirement: responseIdentity is explicitly closed through per-path capability work below. Limit the rule to shared implementations where the guarantee has a common implementation, and keep path-specific capabilities separate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0011-interaction-guarantee-contract.md, line 13:
<comment>This glance rule overstates the sharing requirement: `responseIdentity` is explicitly closed through per-path capability work below. Limit the rule to shared implementations where the guarantee has a common implementation, and keep path-specific capabilities separate.</comment>
<file context>
@@ -2,65 +2,71 @@
+
+- Classify every dispatch-path × guarantee cell in the typed registry.
+- Link each gap waiver to a tracking issue. Keep command-scoped claims within the path's command set.
+- Give each TypeScript rule one shared implementation. Prove runner-side equivalents through golden fixtures.
+- Prove every non-waived cell through a tagged contract scenario.
+- Delegation-on-error does not prove success-path parity.
</file context>
| - Give each TypeScript rule one shared implementation. Prove runner-side equivalents through golden fixtures. | |
| - Give each shared TypeScript rule one implementation. Prove runner-side equivalents through golden fixtures. |
There was a problem hiding this comment.
The glance bullet restates base Layer 2: "Each guarantee has exactly one home that all TS paths import" (base line 130). Narrowing it changes the frozen meaning. If the base is wrong, docs/adr-stale-facts (umbrella #3744) handles it.
| Without one, the same helper runs one-shot. | ||
| - **Apple.** iOS lowers one-contact endpoint-hold plans to the established fast-swipe synthesis profile. | ||
| It reaches the endpoint in 100 ms, holds for the planned duration, then lifts, as Maestro's XCTest driver does. | ||
| One-contact plans are linear and carry only start/end samples with the authored duration between them. |
There was a problem hiding this comment.
P3: One-contact plans is too broad here: target-authored drag includes source, movement, and destination-hold samples, while the next paragraph says those authored samples are preserved. Qualify this as canonical one-contact endpoint plans so the ADR does not contradict its drag schedule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0013-unified-gesture-plans.md, line 129:
<comment>`One-contact plans` is too broad here: target-authored drag includes source, movement, and destination-hold samples, while the next paragraph says those authored samples are preserved. Qualify this as `canonical one-contact endpoint plans` so the ADR does not contradict its drag schedule.</comment>
<file context>
@@ -4,215 +4,255 @@
+ Without one, the same helper runs one-shot.
+- **Apple.** iOS lowers one-contact endpoint-hold plans to the established fast-swipe synthesis profile.
+ It reaches the endpoint in 100 ms, holds for the planned duration, then lifts, as Maestro's XCTest driver does.
+ One-contact plans are linear and carry only start/end samples with the authored duration between them.
+ Two-contact plans convert every planned point to native orientation and send the exact arrays to the private XCTest event bridge.
+ macOS lowers one-contact plans to its drag executor.
</file context>
| One-contact plans are linear and carry only start/end samples with the authored duration between them. | |
| Canonical one-contact endpoint plans are linear and carry only start/end samples with the authored duration between them. |
There was a problem hiding this comment.
This text matches the base (59fdf008b9), so it is not drift from this PR. This PR keeps the base meaning. If the base is wrong, docs/adr-stale-facts (umbrella #3744) handles it. Base line 96: "One-contact plans are linear".
There was a problem hiding this comment.
3 issues found across 17 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/adr/0014-session-ref-frame-lifetime.md">
<violation number="1" location="docs/adr/0014-session-ref-frame-lifetime.md:28">
P3: This rule names the wrong owner for frame construction: `ref-frame-slot.ts` contains the private class and frame factories, while `ref-frame.ts` owns transitions and admission. Point the invariant at the value module so future changes follow the actual encapsulation boundary.</violation>
</file>
<file name="docs/adr/0015-direct-maestro-engine.md">
<violation number="1" location="docs/adr/0015-direct-maestro-engine.md:162">
P3: The Layer 1 outcome list omits the verifier's `we-are-lenient` classification. Add it so the ADR accurately describes every classification the conformance check can produce.</violation>
<violation number="2" location="docs/adr/0015-direct-maestro-engine.md:165">
P3: Layer 2 does not read parser-observed defaults from bytecode: the harness parses a minimal flow with `YamlCommandReader` to obtain the swipe duration. Update this description so the ADR does not misstate the conformance evidence source.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
| The frame is one value on `SessionState`. | ||
| A transition replaces the frame whole and never edits it in place. | ||
| The frame type is nominal (`#`-private fields). | ||
| Thus, no module outside `packages/daemon/src/core/ref-frame.ts` can construct a frame, edit a frame, or derive a frame from an existing frame. |
There was a problem hiding this comment.
P3: This rule names the wrong owner for frame construction: ref-frame-slot.ts contains the private class and frame factories, while ref-frame.ts owns transitions and admission. Point the invariant at the value module so future changes follow the actual encapsulation boundary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0014-session-ref-frame-lifetime.md, line 28:
<comment>This rule names the wrong owner for frame construction: `ref-frame-slot.ts` contains the private class and frame factories, while `ref-frame.ts` owns transitions and admission. Point the invariant at the value module so future changes follow the actual encapsulation boundary.</comment>
<file context>
@@ -2,64 +2,72 @@
+ The frame is one value on `SessionState`.
+ A transition replaces the frame whole and never edits it in place.
+ The frame type is nominal (`#`-private fields).
+ Thus, no module outside `packages/daemon/src/core/ref-frame.ts` can construct a frame, edit a frame, or derive a frame from an existing frame.
+ The type cannot judge a whole frame that moves unchanged, for example when code clears the field or assigns the frame of another session.
+ The field-owner gate still covers these moves.
</file context>
| Thus, no module outside `packages/daemon/src/core/ref-frame.ts` can construct a frame, edit a frame, or derive a frame from an existing frame. | |
| The frame's nominal representation is confined to `packages/daemon/src/core/ref-frame-slot.ts`; frame transitions and admission remain owned by `packages/daemon/src/core/ref-frame.ts`. |
There was a problem hiding this comment.
This text matches the base (59fdf008b9), so it is not drift from this PR. This PR keeps the base meaning. If the base is wrong, docs/adr-stale-facts (umbrella #3744) handles it. Base lines 22-23 name ref-frame.ts.
| The verifier classifies every flow as identical / both-reject / we-reject / mismatch. | ||
| Each non-identical outcome must be a declared, on-the-record divergence. | ||
| 2. Layer 2 reads upstream constants (retry cap, animation-wait threshold/timeout, erase cap) straight from the pinned bytecode. | ||
| It also reads parser-observed defaults (swipe duration) straight from the pinned bytecode. |
There was a problem hiding this comment.
P3: Layer 2 does not read parser-observed defaults from bytecode: the harness parses a minimal flow with YamlCommandReader to obtain the swipe duration. Update this description so the ADR does not misstate the conformance evidence source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0015-direct-maestro-engine.md, line 165:
<comment>Layer 2 does not read parser-observed defaults from bytecode: the harness parses a minimal flow with `YamlCommandReader` to obtain the swipe duration. Update this description so the ADR does not misstate the conformance evidence source.</comment>
<file context>
@@ -6,248 +6,256 @@ Accepted
+ The verifier classifies every flow as identical / both-reject / we-reject / mismatch.
+ Each non-identical outcome must be a declared, on-the-record divergence.
+2. Layer 2 reads upstream constants (retry cap, animation-wait threshold/timeout, erase cap) straight from the pinned bytecode.
+ It also reads parser-observed defaults (swipe duration) straight from the pinned bytecode.
+ It cross-checks each value against the live `MAESTRO_COMPATIBILITY_PRESETS` constant.
+3. Layer 3 runs a small set of app-observable differential scenarios through both engines against the real fixture app.
</file context>
| It also reads parser-observed defaults (swipe duration) straight from the pinned bytecode. | |
| It also observes parser defaults (swipe duration) by parsing a minimal flow with the pinned `YamlCommandReader`. |
There was a problem hiding this comment.
This text matches the base (59fdf008b9), so it is not drift from this PR. This PR keeps the base meaning. If the base is wrong, docs/adr-stale-facts (umbrella #3744) handles it. Base lines 141-143: "parser-observed defaults (swipe duration) straight from the pinned bytecode".
| The pinned parser is SHA-verified at regeneration. | ||
| Layer 1 captures each parse. | ||
| The deterministic verifier replays that capture against our engine. | ||
| The verifier classifies every flow as identical / both-reject / we-reject / mismatch. |
There was a problem hiding this comment.
P3: The Layer 1 outcome list omits the verifier's we-are-lenient classification. Add it so the ADR accurately describes every classification the conformance check can produce.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0015-direct-maestro-engine.md, line 162:
<comment>The Layer 1 outcome list omits the verifier's `we-are-lenient` classification. Add it so the ADR accurately describes every classification the conformance check can produce.</comment>
<file context>
@@ -6,248 +6,256 @@ Accepted
+ The pinned parser is SHA-verified at regeneration.
+ Layer 1 captures each parse.
+ The deterministic verifier replays that capture against our engine.
+ The verifier classifies every flow as identical / both-reject / we-reject / mismatch.
+ Each non-identical outcome must be a declared, on-the-record divergence.
+2. Layer 2 reads upstream constants (retry cap, animation-wait threshold/timeout, erase cap) straight from the pinned bytecode.
</file context>
| The verifier classifies every flow as identical / both-reject / we-reject / mismatch. | |
| The verifier classifies every flow as identical / both-reject / we-reject / we-are-lenient / mismatch. |
There was a problem hiding this comment.
This text matches the base (59fdf008b9), so it is not drift from this PR. This PR keeps the base meaning. If the base is wrong, docs/adr-stale-facts (umbrella #3744) handles it. Base line 140 lists four outcomes.
|
The ADR rewrite in ea9be72 has nothing that must change before merge. All 8 checks pass, and this docs-only diff touches no job route. No conflicts. Not blocking: docs/adr/0008-command-descriptor-registry.md line 15 keeps "No parity test exists", but I could not find the base "do not add one" directive. Please restore it if the base had it, and qualify the claim (for example "no byte-for-byte table parity test"). The retired "capability facet" wording also stays at lines 62 and 104. I checked only 0008, so one pass over all 16 ADRs for dropped "do not", "must" and status clauses would be useful. You can take or leave these. On the open threads: the 0008 parity-test claim, the 0008 capability facet, the 0002 fail-closed scope, the 0002 planned-touch helper and the 0007 refused commands still apply. The 0007 renew window is unchecked against the base. Please resolve 0016 line 165, which repeats the base wording, and 0007 lines 170-171, where the detail exists in the text. I did not read the other 22 open threads, so please sort each as drift from this PR or inherited from the base before you act on it. I did not compare all 16 ADRs against the base, and I checked ADR claims against code only for the one parity test file. Before merge, run the per-ADR check for dropped clauses against the base and sort the open threads. |
The base rule applies to the snapshot helper and forbids the uiautomator dump substitute. The glance bullet dropped both. #3729 (comment)
The retired gate compared derived tables byte for byte. src/__tests__/command-descriptor-parity.test.ts compares name sets. #3729 (comment)
|
Follow-up at d7b95ab. Docs only; meaning stays frozen against Commits
CI: all 8 checks pass. No conflicts. Threads (36): 3 fixed. 4 already fixed. 6 fixed on Validation: |
|
The PR is ready: I found no problems at d7b95ab, and the earlier findings from ea9be72 are fixed. The 0002 fail-closed rule now applies only to Android snapshots, and the 0002 thread is fixed at this head. CI is green on all 8 checks, and the change is docs-only. There are no conflicts. Nothing else blocks human review. Not blocking: docs/adr/0016-active-session-script-publication.md:132 lost the sentence that points to the later amendment replacing the stored flag, so you can restore that pointer or leave it. On the other review threads, the fixed one is #3729 (comment) (0002 fail-closed scope), so please resolve it. These do not apply, because they ask to change content that the base ADRs already had and this PR keeps the meaning frozen, so please resolve them: #3729 (comment) (0008 capability facet is in the base text), #3729 (comment) (descriptor.execute is in the base text), #3729 (comment) (asks for a rewording beyond the base 0006 text), #3729 (comment) (0011 glance rule is preserved from the base), #3729 (comment) (0010 diagnostic-field claim is base content), #3729 (comment) (0017 literal-retention tension is base amendment content), #3729 (comment) (0014 partial-issuance summary is base content), #3729 (comment) (0005 preflight exemptions are base content), #3729 (comment) (0005 allowlist is base content), #3729 (comment) (0017 event-presentation claim is base content). For this round I read only the changed hunks since ea9be72 and spot-checked three base passages, so I did not diff all 16 ADRs against base again. I judged the thread claims on frozen meaning and did not check each factual claim against current code. |
thymikee
added
the
ready-for-human
|
The review of d7b95ab still stands, but this PR now conflicts with main in |
thymikee
removed
the
ready-for-human
Restore the deleted interim-owner sentence and the AGENTS.md rule at the end of the 2026-06 update. Drop the Status history line, since no history stays deleted. Use "pin" for the registry tests, and restore the exact denied-surface list and the "only"/export scope in the glance rules. Leave owner drift (daemon-command-registry.ts vs registry.ts, catalog.ts) for a maintainer decision.
Restore the Status fact that the public surface stays two modes and that host AX is an acquisition route.
Restore the no-box condition on wire `hittable`, the Swift/TypeScript guard referents, the Simulator probe routing condition, and the layering gate's "only".
Rewrite the glance rules from the body: fallback disables the bridge except capture-scoped failures; restore body scope for node-count, raw transport, and facet rules.
Restore the technical term "ladder", the semantic layer ordinals, past-tense history facts, and dropped modals ("can", "never", "always", "for any node").
Restore the ADR 0005 preflight-reduction gate as a present-tense rule and link the allowlisted uptime skip to it. Scope the RUNNER_BUSY glance rules so connect-failure replay is not banned. Restore dropped causal links, "not only from", "bookkeeping", and the base occupancy and close-and-pooling scopes.
Restore "may" and the proxied-upstream condition in the ADR 0006 cache glance rule, and add the must-honor-conditional-headers rule to the glance. State the one remaining wire-compat gap. Restore "fail before it sends", "is treated as", "instead of failing clearly", and the "over bumping the whole protocol" preference.
Restore the caller-named window and the close-release case in the ADR 0007 glance rules, and fix the "It allocates" referent. Restore the full request-lock list, past-tense incident facts (#2946, orphaned allocations, expired admitted work), the dropped heartbeat-default history, and the base terms "treated as", "is refused with", "is used".
Restore "As of 2026-07" on the descriptor-registry live list; the `read` alias it names was retired later in #1877. Restore the "Parity gate retired (2026-09)" label and its reason, and the ADR ownership sentence. Mark the import-cycle inversion as a migration risk while keeping the `Input`/`Result` ownership rule. Restore "every", "never collapse", "must preserve", "is bound by", and "the remaining deferred work".
Restore "Implementation status as of 2026-08" with its Shipped / Decision-only support boundary / Retained compatibility / Deferred labels. Restore the command-atomic cutover wording and ADR 0019's term "legal injected substrate transition". Restore "no coverage is claimed" as a dated statement, not a ban, and the "registry and enforcement paths are authoritative" sentence. Fix a false causal clause for visionOS and restore base terms (OS-agnostic, net-new, seam, must not be flattened).
Restore the hint-required condition in the ADR 0010 glance, and use "pin" for help-text guarantees. Restore the scope "a user or agent can reach", "consumer surface", "most common agent failures", and "some consumers". Restore the condition on the ADR 0019 cleanup sentence, and keep the code-selection list from reading as a ban on the correct codes. Replace "subclasses" with the base term "sub-classification".
Restore ADR 0011 Migration plan step 3 as open: no fake runner reads tap-point-policy.json yet. Steps 1, 2, 4, and 5 stay deleted as complete.
Restore the dated status qualifier (implemented through Layer 3, 2026-07-04).
Restore both collapse conditions in the glance, name `RUNTIME_OPERATION_EFFECTS`, and restore "Remaining gaps".
Restore dropped conditions, "only"/"merely"/"deliberately", the causal links, and the base terms ("class of bug", "ladder", "Swift twin", "hand-rolled").
Restore the Android rule that gestures do not stop or restart the snapshot session (#1275), which the language commit dropped. Restore the codec amendment date (#1216), "only remaining callers", and "nothing left to migrate off". Make the glance rules match the body: removed-argument rejection, viewport failure code, 60 s scope, and drag resolve-versus-replay-verify. Restore dropped conditions, owners, and causal links in the Android, Apple, codec, and drag sections.
Restore ADR 0014 meaning that the STE rewrite changed: "implemented and enforced on `main`", the staged-plan pointer that code comments use, and the PR #1241 status sentence. Restore dropped conditions and quantifiers ("only ... and only with their epoch", "any", "every", "all reach the generic daemon leaf", "on that platform"), the side-effect "seam" term, "not the transition site", and the base ADR-composition and Consequences claims.
Restore technical terms that code uses: bug class, bug-class flows, settle ordering, settle-loop ordering. Restore "never authoritative" for observation frames, and the "as long as" condition on ADR 0013 internal rewrites. Restore dropped facts: the oracle replaced the hand-typed fixture, Layer 3 is a small set, documented deviations are not silent mismatches, a unit test rather than a device scenario pins percent truncation. Make the glance rules match the body: inline-settle exception, ambiguity rule under `optional`, failed-dispatch invalidation.
Restore the #1533 invariant that no lifecycle state name, transition, or entry point is added or altered, and the writer's single publication-blocked question. Restore "only" for PUBLISHED in the body and glance, plus the "shipped" status of each amendment and the implemented-on-main status. Put superseded behavior (pre-amendment close paths, the recordSession flag) back in the past tense, and keep "such as" non-exhaustive. Restore dropped qualifiers: "own recorded evidence", "re-serialize", "for the matched element", "(2026-08-02 amendment)", "leaks ... exactly like", "entirely under #1336".
Restore the absolute "no secret-to-name table" rule and fix the "until its resolution" referent (#1348). Rewrite the glance bullets as statements that match the body's conditions, including the full session-scoped echo guarantee. Turn the PR's imperatives in landmark and action modes back into descriptive statements, and restore dropped "only", "re-serialize", "the literal is redacted either way", and "rather than an authoring convention".
Restore "replay healing" and the ADR 0032 "revisit triggers" term in the index.
Restore dropped nouns that carried scope ("error handling", "semantics", "cleanup fix", "where ... enforced", "that the table refused").
Scope the waiver/evidence-gap convention to the Status section again.
The STE rewrite read as if `is` and `get` also record landmark-mode evidence. The base lists three read-only actions: `wait` in landmark mode, `is`, and `get`. Found in review round 2.
The base rule applies to the snapshot helper and forbids the uiautomator dump substitute. The glance bullet dropped both. #3729 (comment)
The retired gate compared derived tables byte for byte. src/__tests__/command-descriptor-parity.test.ts compares name sets. #3729 (comment)
The base states the rule as current. The rewrite put it in the past tense and added a sentence that made it read as replaced.
…tense src/commands/family/types.ts still imports AgentDeviceClient. That import is the reason for the retained Input/Result ownership rule.
d7b95ab to
4150630
Compare
|
Rebased onto Conflicts (2 files):
Checked every main-sourced hunk sentence-by-sentence against main: no condition, scope word, or modal changed. The 19 unresolved threads are stale-fact notes for the later fact-correction PR; untouched. Receipt: |
|
The conflict from d7b95ab is fixed, and I found no code problems in 4150630. This is a docs-only change, so there is no runtime risk. All 8 checks pass. There are no conflicts. Before this goes to human review, please fix the 0008, 0010 and 0002 statements below, which the open review threads also flag. These still need a fix: 0008:62 and line 104 still name a "capability" facet, while the amendment at line 10 replaces it with typed runtime declarations under I checked only 0002, 0006, 0008 and 0010 against the code. I did not check the other threads against code, and I did not look at the lower-priority threads. The delta diff includes upstream rebase content, so I could not isolate your changes beyond the two conflict files. These review threads still apply: 0008 descriptor.execute, 0008 capability facet, 0002 provider-native path and 0010 overstated claim, which is inherited from main. Seven more accuracy threads are unchecked against code: 0006 outcome, 0011 responseIdentity, 0017 literal retention, 0014 empty-result frames, 0005 exemptions, 0005 allowlist and 0017 redaction. None of these are runtime defects. |
|
At The maintainer assigned all 19 unresolved fact threads to After #3729 and #3731 merge, I will verify each claim against the code and include each valid correction in the stale-facts PR. Each thread stays open. I will reply with the PR link when it exists. No text changed in this round. |
|
Thanks. The items left from my review of 4150630 (the ADR 0008 facet and |
thymikee
added
the
ready-for-human
This branch has not been deployed
1 participant
Summary
Rewrite the prose of 16 ADRs (0001–0011, 0013–0017) and the index in ASD-STE100. Add the missing "Rules at a glance" sections. Meaning is frozen: where STE wording and exact meaning conflict, the base sentence wins.
The fixup commits ("restore the frozen meaning in ADR NNNN") fix the round-1 review. They restore operative content that the history commits deleted: ADR 0005 preflight gate, ADR 0011 open migration step 3, ADR 0013 gesture/snapshot-session rule, ADR 0016 #1533 invariant, ADR 0003 interim-owner sentence. ADRs 0002, 0003, 0005, and 0016 now delete no history. About 3.5 KB of completed history stays deleted in ADRs 0001, 0004, 0008, 0009, 0011, 0013, and 0014.
Discrepancies found, not changed
daemon-command-registry.tsas the trait declaration site, but traits are inlinedaemon: {…}fields inpackages/command-registry/src/registry.ts;catalog.tsholds no per-command data. Maintainer: is the inline facet an accepted change to Invariant 1? Should the ADR point atregistry.ts?contracts/fixtures/tap-point-policy.json.interaction-guarantees.ts:6("phase 3"),registry.ts:1602-1606andcommand-descriptor-ref-frame-effect.test.ts:51("ADR 0014 escape hatch").Review and validation
ea9be72adc. No finding open.check:affectedpassed,exactCommit=true, 0 checks selected (ADR-only).check:agent-guidancepassed.