Repository navigation
Conversation
There was a problem hiding this comment.
No issues found across 7 files
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Size Report
Startup median (7 runs, lower is better):
|
|
The PR is ready. I reviewed commit 49a4033 and found no blocking problems in the code. Not blocking: these notes can be taken or left. (1) The Unreleased section of CHANGELOG.md has no entry for two user-visible changes: authored and I found no conflicts. The iOS Smoke Tests job failed at "wait for the deep-link destination (5/5)" with APP_NOT_RUNNING, reason |
Address #3760 (comment). Read explicit recording from the existing action flags, retain authored provenance, add the Unreleased notes, and synchronize lock tests with queue insertion.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Addressed all four maintainer notes in eb30491:
iOS diagnosis: the original artifact screenshot shows unanswered “Open in Agent Device Tester?” confirmation; runner Validation: five focused suites, 104 passed; affected gate Head CI: lint/typecheck/package, size, native fingerprint and CodeQL passed; smoke, guards, integration, coverage and cubic pending. Not merge-ready yet. Review threads: 0 fixed / 0 already fixed / 0 rejected (none exist). |
|
This PR is ready at eb30491. The earlier review of 49a4033 was clean, and this head keeps that result: cleanup locks are now retained and healed snapshots are preserved. Not blocking: the The Smoke Tests failure looks unrelated to this change. The log I have is from run 39008383674, which is not from this head. It shows the iOS simulator deep-link destination wait stalling (APP_NOT_RUNNING, wait_capture_stalled, zero readable captures), and the hint points to a deep-link confirmation prompt holding the launch. That is a simulator prompt state. The scope-disposal reorder and Please rerun Smoke Tests on eb30491. Before merge, that run needs to be green, or show that the wait_capture_stalled prompt state comes from the environment. No conflicts are known. On the open review threads, the P2 about |
Distinguish explicit CLI/MCP/SDK corrections from project record defaults before recording and publishing repair observations. Retain authored provenance even when record is also set. Review thread: #3760 (comment)
|
Fixed the config-derived recording leak in 73faf74. Regression before fix: 1 failed / 1 passed (unwanted snapshot). CLI/MCP/SDK publication coverage after fix: 5 passed. Provider integration exercises SDK ingress and rejects an effective flag without intent. iOS: attempt 1 failed in daemon startup ( At Validation: Threads: 1 fixed/resolved, 0 already fixed, 0 rejected. New-head CI remains required. |
|
There was a problem hiding this comment.
All reported issues were addressed across 17 files (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Set HOME and USERPROFILE together for CLI and MCP publication setups. Review: #3760 (comment)
|
Pushed 81e2eba: CLI and MCP publication tests now set Threads: 1 fixed/resolved, 0 already fixed, 0 rejected. Re-read issue comments, reviews and inline threads after pushing; no unresolved threads or new maintainer action items remain. Validation: CI: at T3 rejects CI watching from a sub-agent; the parent thread must continue monitoring and diagnose any failures. The prior |
Address #3760 (comment). Read explicit recording from the existing action flags, retain authored provenance, add the Unreleased notes, and synchronize lock tests with queue insertion.
Distinguish explicit CLI/MCP/SDK corrections from project record defaults before recording and publishing repair observations. Retain authored provenance even when record is also set. Review thread: #3760 (comment)
Set HOME and USERPROFILE together for CLI and MCP publication setups. Review: #3760 (comment)
81e2eba to
d7bdb0d
Compare
|
Rebased Resolved both test conflicts against main’s history: retained its typed action-history cases and the PR’s publication-intent cases; kept provider scenarios on public runtime/HTTP results and healed-script assertions. Moved the five CLI/MCP/SDK publication cases to provider integration through the public daemon runtime, removing all three new daemon testing-export imports. Production changes are unchanged by the rebase. Validation: locked install/build, full-repository format, 120 focused tests passed. Exact-head affected receipt run-oBaplK: 9 checks / 3,570 tests passed, |
ADR 0019 requires request bindings to dispose before device/session lock release. Await the shared cleanup outcome under initial and retained execution locks, then let response finalization preserve primary failures and report cleanup failures. At base 4c1c786, six blocked-cleanup regressions fail: queued same-session or same-device requests enter on success, throw, and cancellation. With the fix all six pass; the scope and open-wait suites pass (42 tests). No request-binding disposal bound exists; no timeout was added. Device dispatch is unchanged.
ADR 0012 requires authored observations and explicit --record corrections to survive healed-script publication. Carry their typed publication intent from recording into the writer instead of stripping every snapshot. Retain ordinary diagnostic-snapshot optimization. Isolated regression at base 4c1c786: authored and explicit-record persisted snapshots and the correction-count/resume publication scenario all fail (3 regressions); all pass with the fix. Combined before: 9 failed, 43 passed; after: 52 passed. Recorder/resume focused coverage also passes (82 tests). Device dispatch is unchanged.
ADR 0012 decision 6 preserves authored observations through replay provenance. Non-observation wait and diff requests must not acquire authored intent from their false observation classification. Regression proof on 17b30bf plus tests: 2 failed, 31 passed (interactive wait/diff received authored intent). With the fix: 33 passed.
ADR 0012 decision 6 preserves the authored replay path. The initial snapshot composed by open --foreground is not a source step and must not inherit its recording intent, including a configured record flag. Regression proof on 17b30bf plus tests: 2 failed, 13 passed because composite snapshots entered session.actions. With noRecord on the synthetic capture: 15 passed; capture response remains successful.
ADR 0012 decision 6 requires authored observations to survive the entire repair publication path. Exercise a source snapshot -i through replay, close, healed-file parsing and fresh-session replay. Isolated regression: restore the writer from dacec4a (before snapshot retention) in the 17b30bf checkout plus this test. The selected case fails because the healed snapshot is missing (1 failed, 5 skipped). With retention: 1 passed, 5 skipped; full repair-recording suite: 6 passed.
ADR 0012 decision 6 scopes --record observation retention to repair-armed sessions. Preserve ordinary authoring snapshot exclusion even when project configuration supplies record=true; keep authored source observations and explicit repair corrections. Regression proof on 17b30bf plus tests: 1 failed, 1 passed, 39 skipped (ordinary record=true published a diagnostic snapshot). With the fix: 2 passed, 39 skipped. All four focused suites: 95 passed.
Address #3760 (comment). Read explicit recording from the existing action flags, retain authored provenance, add the Unreleased notes, and synchronize lock tests with queue insertion.
Distinguish explicit CLI/MCP/SDK corrections from project record defaults before recording and publishing repair observations. Retain authored provenance even when record is also set. Review thread: #3760 (comment)
Set HOME and USERPROFILE together for CLI and MCP publication setups. Review: #3760 (comment)
d7bdb0d to
a46bcdc
Compare
|
Rebased Resolved Quiesce remains compatible: a request stays in flight through binding cleanup, so quiesce returns Validation: locked install/build, full-repository format, 180 focused tests passed. Exact-head receipt run-0VMDJP: 9 checks / 3,601 tests passed, Review threads: 0 newly fixed / 0 already-fixed actions / 0 rejected; both existing threads remain resolved. |
Summary
--recordcorrections in healed scripts (ADR 0012). Exclude diagnostic and foreground composite captures.record: truedefaults; SDK per-call recording stays explicit.Validation
a46bcdce611309463f5cc91749f9cc8efaac6402, rebased onto0563666b09a09b2de039507307d38239dabee272.pnpm format; 180 focused tests passed. Publication tests use default CLI/MCP ensured clients and SDK HTTP transport against the public daemon runtime; HOME/USERPROFILE remain isolated.run-0VMDJP,exactCommit=true,checkoutChanged=false; nine checks and 3,601 tests passed, seven parked/GitHub-owned skips. Self-reviewed; no independent review or live device run for this rebase.daemon-busywithout acquiring its execution lock.d7bdb0d70e: guards, integration, coverage, lint/package, docs, preview, size, native fingerprint, CodeQL and two smoke jobs passed; two smoke jobs queued, cache matrix skipped, cubic neutral. Current-head CI, including native smoke, remains required.