Skip to content

[2122 by GPT-5.6 Sol] Integrate native Mac/Linux artifact lifecycle with current desktop transport - #2125

Merged
integry merged 5 commits into
1950-epic-cross-platform-dskfrom
2122/gpt-5.6-sol-integrate-native-mac-linu-20260904-2105-al1
Sep 6, 2026
Merged

[2122 by GPT-5.6 Sol] Integrate native Mac/Linux artifact lifecycle with current desktop transport#2125
integry merged 5 commits into
1950-epic-cross-platform-dskfrom
2122/gpt-5.6-sol-integrate-native-mac-linu-20260904-2105-al1

Conversation

@propr-dev

@propr-dev propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

AI Implementation Summary

Closes #2122

Branch: 2122/gpt-5.6-sol-integrate-native-mac-linu-20260904-2105-al1
Commits: ✅ Changes committed (6c35f78)


AI Processing Completed

Execution Details:

Summary:
Implemented issue #2122.

Key changes:

  • Added acknowledgement-bound, exactly-once deep-link delivery across main/IPC/preload/renderer, including bounded shutdown draining and confirmation-only Connect candidates.
  • Integrated native DEB/RPM/ZIP and DMG/ZIP lifecycle validation for Linux/macOS x64 and arm64, covering launch, protocol dispatch, relaunch, ProfileStore v3 persistence, secure-storage evidence, hashes, and cleanup.
  • Reused the existing macOS disposable signing identity/keychain for copied test apps while leaving staged artifacts unchanged.
  • Added Linux protocol MIME registration and CI lifecycle gates.
  • Updated the desktop runbook with installation, usage, removal, and unsigned-build limitations.
  • Added regression coverage throughout the lifecycle, IPC, preload, shutdown, workflow, and renderer layers.

Validation:

  • Desktop typecheck: passed
  • Desktop suite: 521 passed, 25 skipped
  • Renderer suite: 625 passed
  • Focused lifecycle/IPC tests: 116 passed
  • Prepared repository suite: first 60/392 files passed before stopping the long serial run
  • Shell syntax and git diff --check: passed

No visual preview was generated because the changes affect lifecycle validation and transport acknowledgements rather than visible UI.

Detailed Logs:

  • Conversation: 154 messages
  • Session: null

Log files stored at:

  • conversation: /tmp/claude-logs/issue-2122-2026-09-04T21-42-47-798Z-conversation.json
  • output: /tmp/claude-logs/issue-2122-2026-09-04T21-42-47-798Z-output.txt
Latest Conversation Messages

This PR was created automatically by ProPR after processing issue #2122.


💡 Need changes?

Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!

Closes #2083

…fecycle with

Implemented by ProPR AI using gpt-5.6-sol model.

Implementation completed successfully.
@integry

integry commented Sep 4, 2026

Copy link
Copy Markdown
Owner

/ultrafix goal=8 max=1

@propr-dev propr-dev Bot added the ultrafix label Sep 4, 2026
@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Ultrafix loop started (goal: 8/10, max cycles: 1)

First action: /review

💡 Tip: Remove the ultrafix label from this PR to stop further ultrafix cycles.

@integry

integry commented Sep 4, 2026

Copy link
Copy Markdown
Owner

/review

@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@integry

integry commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Concrete native Linux CI blocker on head 6c35f78: both unsigned Linux jobs fail only in the newly integrated native artifact lifecycle, after packaging/staging passes. Run33922345710, x64 job101183283494 and ARM64 job101183283411. x64 error: NativeLifecycleOperationFailure [stage:FIRST_INITIAL_EVIDENCE] [milestone:IDENTITY] [result:FAILED_EXIT], test-native-artifact-lifecycle.mjs:1443. All four ordinary packaged Connect journeys pass. Please diagnose the actual child exit and correct the smallest native launch/authorization/evidence integration defect, preserving identity assertions and all artifact formats. Do not bypass the gate or weaken tests. Ensure failure diagnostics stay secret-safe and useful. Run the relevant regressions and real native Mac/Linux CI. Also inspect the current merged result, not a raw stale head: refs/pull/2125/merge already correctly retains #2124 rollback try/finally and the #2076 release profile; no redundant rewrite is needed for those. Leave the PR open; the in-progress review may provide additional concrete findings.

@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR adds acknowledgement-bound deep-link delivery and comprehensive native macOS/Linux artifact lifecycle validation. The native validation is thorough, but the central deep-link delivery path has three correctness issues involving buffering and failure recovery. The PR is not ready to merge and needs focused, significant changes.

Acknowledgements are semantically bound — Main validates the delivery ID, original URL, normalized target, consumption kind, and sending webContents.

Native lifecycle coverage is extensive — The gate verifies architecture, artifact immutability, profile persistence, secure-storage behavior, protocol dispatch, and cleanup across each staged format.

Shutdown integration is explicit — Deep-link admission is closed and included in the bounded service-drain sequence.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F1: 🔴 Send exceptions wedge delivery

  • Required behavior: Changed deep-link delivery must propagate failures and reach an idle state; a window-destruction race must not leave acknowledgement and shutdown draining permanently stuck.
  • Evidence: apps/desktop/src/deep-link-delivery.ts:159webContents.send() occurs after active and its timer are installed but before the try/catch/finally that clears them. A synchronous send exception therefore rejects the unobserved drain() call while leaving active set, the timer running, and whenIdle() unable to resolve.
  • Minimum fix: Move the send into the guarded delivery block and ensure every synchronous send failure clears the timer and active record, invokes the failure policy, and settles idle waiters.

F2: 🔴 Buffered renderer links cannot be acknowledged

  • Required behavior: A link buffered across the renderer subscription boundary must still produce the acknowledgement associated with its eventual consumption.
  • Evidence: propr-ui/src/desktop-deep-link.ts:75receive() queues a link but returns null when no consumer is installed, while subscribe() later invokes the consumer and discards its returned consumption. apps/desktop/src/preload-bridge.ts:47 acknowledges only the synchronous result of the original listener call, so that eventual consumption can never acknowledge the main-process delivery.
  • Minimum fix: Preserve acknowledgement completion across renderer buffering, such as by propagating an eventual consumption result, or ensure the bridge listener is not registered until the inbox consumer exists and the preload remains the sole buffer. Add an integration test spanning preload, inbox, and the hook.

F3: 🔴 One timeout drops unrelated links

  • Required behavior: Every delivery accepted by deliver() must be processed or individually reported as failed; one unacknowledged link must not silently discard later accepted activations.
  • Evidence: apps/desktop/src/deep-link-delivery.ts:164 — The acknowledgement failure path executes this.pending.splice(0), removing all subsequently queued links even though their deliver() calls already returned true.
  • Minimum fix: Fail only the active delivery and continue draining distinct queued deliveries, or explicitly reject admission for them under a terminal closed state instead of accepting and silently discarding them.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The native lifecycle work and validation depth are strong, but the acknowledgement mechanism is unreliable at a normal buffering boundary and can wedge or lose accepted work during failures. Fifteen current-head checks also remain pending.

Score: 5/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 3m 43s
  • Tokens: 86,462 (74,425 in / 12,037 out)
  • Cost: $0.73

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Starting work on follow-up changes requested by @integry

I'll analyze the 1 request and implement the necessary changes.

View Task Progress


Processing comment ID: 5546836408✓

@integry

integry commented Sep 4, 2026

Copy link
Copy Markdown
Owner

/fix F1 F2 F3

@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Processing Delayed: Claude's usage limit was reached while processing requests from @integry.

The job has been automatically rescheduled and will restart at a later time.


Job ID: pr-comments-batch-integry-propr-2125-codex-gpt56-sol-2122-gpt-5-6-sol-integrate-nat-ratelimit-retry will run again after delay.

@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Starting work on follow-up changes requested by @integry

I'll analyze the 1 request and implement the necessary changes.

View Task Progress


Processing comment ID: 5546852170✓

@propr-dev

propr-dev Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Processing Delayed: Claude's usage limit was reached while processing requests from @integry.

The job has been automatically rescheduled and will restart at a later time.


Job ID: pr-comments-batch-integry-propr-2125-codex-gpt56-sol-2122-gpt-5-6-sol-integrate-nat-ratelimit-retry will run again after delay.

@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Resume the unfinished correction after the previous usage-limit stop. No prior correction was pushed; current head is6c35f78. Recover any existing worktree edits if available and complete both parts in one focused pass: (1) actual Linux x64/ARM64 native lifecycle FIRST_INITIAL_EVIDENCE / IDENTITY / FAILED_EXIT failure from run33922345710, without weakening native assertions; (2) review F1/F2/F3: guard synchronous send failures and settle idle state, preserve eventual ACK across renderer buffering, and do not silently drop accepted queued links after one timeout. Prior comments/review contain exact evidence. All4 ordinary packaged Connect journeys and both Mac DMG/ZIP lifecycles passed on this head; preserve them. Current merge result already preserves rollback try/finally and the Mac/Linux-only release profile. Run focused tests and native CI. Leave open for validation, target desktop epic only, never main.

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

🛑 Execution Cancelled

The task processing was stopped by user request.

You can post a new comment to restart processing.

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 45e8ce3

Summary of Changes

Implemented both corrections without weakening native assertions.

  • Linux failure root cause: safeStorage.encryptString exited because the native artifact intentionally lacked a Secret Service session. Native CI now launches under an isolated unlocked D-Bus/gnome-libsecret session and verifies actual credential round-trip/deletion plus the exact gnome_libsecret backend. Diagnostics add a fixed, secret-safe SECURE_STORAGE_BACKEND milestone.
  • F1: synchronous renderer send exceptions now clear active/timer state and settle whenIdle().
  • F2: buffered renderer deliveries preserve their eventual consumption result through preload and ACK only after the UI consumes them.
  • F3: one ACK timeout no longer discards other accepted queued links.

Validation passed:

  • Focused desktop: 63/63
  • Focused renderer: 29/29
  • Full desktop: 524 passed, 25 skipped
  • Full renderer: 626 passed
  • Desktop/UI typecheck
  • Linux x64 package build
  • git diff --check

No visual preview was created because the changes are nonvisual. No commit, push, or PR-state change was made; the PR remains targeted to the desktop epic. The real Linux/macOS native matrix must run after the system publishes these worktree edits, since GitHub Actions cannot test an uncommitted tree.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 19m 57s
  • Tokens: 10,161,944 (10,128,449 in / 33,495 out)
  • Cost: $7.48
  • Subscription usage: Weekly +2%

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5558427717✓

@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Recovery evidence for the automatically rescheduled worker (same existing correction scope): predecessor task pr-comments-batch-integry-propr-2125-1788688112247 left uncommitted F1/F2/F3 and native-launch fixes before container replacement around 10:02 UTC. Please check its OLD worktree, not only the clean new worktree, for recovery. It reproduced Linux exit: native isolated launch withholds session bus; unselected Electron password backend makes ProfileStore v3 journal encryptString fail after identity. Its proposed/tested fixture correction explicitly selected basic_text only for isolated native launches, retaining refusal of credential storage and the existing declaration that this gate does not exercise libsecret custody. Ordinary packaged Connect must still prove production libsecret behavior. It also added bounded START_FAILED evidence, awaited eventual renderer consumption before ACK, caught synchronous send failures and continued independently queued deliveries. Reported focused results: 60 desktop and 29 renderer checks passed; broader validation was unfinished. Recover and independently verify, preserving Mac results and all three review requirements. No new scope or extra review cycle is requested by this recovery note.

…tions.

Implemented both corrections without weakening native assertions.

- Linux failure root cause: `safeStorage.encryptString` exited because the native artifact intentionally lacked a Secret Service session. Native CI now launches under an isolated unlocked D-Bus/gnome-libsecret session and verifies actual credential round-trip/deletion plus the exact `gnome_libsecret` backend. Diagnostics add a fixed, secret-safe `SECURE_STORAGE_BACKEND` milestone.
- F1: synchronous renderer send exceptions now clear active/timer state and settle `whenIdle()`.
- F2: buffered renderer deliveries preserve their eventual consumption result through preload and ACK only after the UI consumes them.
- F3: one ACK timeout no longer discards other accepted queued links.

Validation passed:

- Focused desktop: 63/63
- Focused renderer: 29/29
- Full desktop: 524 passed, 25 skipped
- Full renderer: 626 passed
- Desktop/UI typecheck
- Linux x64 package build
- `git diff --check`

No visual preview was created because the changes are nonvisual. No commit, push, or PR-state change was made; the PR remains targeted to the desktop epic. The real Linux/macOS native matrix must run after the system publishes these worktree edits, since GitHub Actions cannot test an uncommitted tree.

PR: #2125
Comment by: @integry (ID: 5558427717)
Model: gpt-5.6-sol
@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

ℹ️ Analyzed the follow-up request by @integry

Analysis Summary

The active PR worktree is clean, but the predecessor is no longer registered as a Git worktree. I’m widening the recovery search to archived/renamed directories and task logs; this matters because the current branch has advanced beyond the 6c35f78 head cited in the earlier comment, so I’ll distinguish already-integrated changes from genuinely recoverable edits.

No code changes were necessary based on the current state of the branch.


🤖 Analysis Details

  • Model: GPT-5.6 Sol (high)
  • Time: 2m 33s
  • Subscription usage: Weekly +1%

View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5558523370✓

@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

/review

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

The PR adds native macOS/Linux lifecycle validation and acknowledgement-bound deep-link handling with strong security-oriented test coverage. It needs minor changes before merge: the shutdown path cancels already-accepted deep links instead of performing the claimed bounded drain, and two authoritative Windows checks currently fail.

Semantically bound acknowledgements — Main validates delivery IDs, URLs, consumption kinds, normalized targets, and the originating WebContents.

Comprehensive native validation — The lifecycle gate covers architecture, secure storage, persistence, protocol dispatch, artifact integrity, and bounded cleanup.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F4: 🔴 Shutdown discards accepted deep links

  • Required behavior: Accepted deep links must receive acknowledgement-bound, exactly-once handling, including bounded shutdown draining; closing admission must not immediately discard work already accepted.
  • Evidence: apps/desktop/src/deep-link-delivery.ts:111close() empties the pending queue and rejects the active acknowledgement, while apps/desktop/src/shutdown.ts:77 invokes it before whenIdle(), making the subsequent drain wait observe canceled rather than completed work. IPC admission is also closed before acknowledgements could finish.
  • Minimum fix: Separate admission closure from cancellation: reject new deliver() calls while retaining accepted pending/active deliveries, keep the acknowledgement IPC path available until they settle, and let the existing bounded shutdown deadline handle genuinely stalled work. Add coverage proving pending and active deliveries can acknowledge after shutdown begins.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The implementation is otherwise thoughtful and extensively tested, but a central durability guarantee is contradicted by the shutdown behavior. The two current Windows validation failures also prevent merge readiness until resolved.

Score: 6/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 4m 40s
  • Tokens: 88,321 (74,463 in / 13,858 out)
  • Cost: $0.74

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

/fix F4

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 73f32ea

Addressed 1 AI review comment (IDs: 5558648466)

Summary of Changes

Implemented F4 only.

  • Shutdown now closes deep-link admission without discarding active or queued deliveries.
  • Deep-link acknowledgement IPC remains available during bounded shutdown draining; other IPC admission still closes.
  • Added coverage for active and pending links acknowledging after shutdown begins.

Validation:

  • Desktop typecheck passed.
  • Focused tests: 26 passed.
  • Full desktop suite: 525 passed, 25 skipped.
  • git diff --check passed.

No visual preview was created because this is non-visual lifecycle behavior. Changes remain uncommitted as requested.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 7m 37s
  • Tokens: 1,185,162 (1,173,943 in / 11,219 out)
  • Cost: $1.31
  • Subscription usage: Weekly +1%

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5558656471✓

Implemented F4 only.

- Shutdown now closes deep-link admission without discarding active or queued deliveries.
- Deep-link acknowledgement IPC remains available during bounded shutdown draining; other IPC admission still closes.
- Added coverage for active and pending links acknowledging after shutdown begins.

Validation:

- Desktop typecheck passed.
- Focused tests: 26 passed.
- Full desktop suite: 525 passed, 25 skipped.
- `git diff --check` passed.

No visual preview was created because this is non-visual lifecycle behavior. Changes remain uncommitted as requested.

PR: #2125
Comment by: @integry (ID: 5558656471)
Model: gpt-5.6-sol
@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

/review

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR substantially improves acknowledged deep-link delivery and adds comprehensive Linux/macOS artifact lifecycle validation, but it is not ready to merge. Two macOS failure paths can lose resource authority and leave mounts or LaunchServices registrations behind; additionally, both current unsigned Windows validation checks are failing, although the supplied evidence does not trace those failures to a specific changed line.

Semantically bound acknowledgements — IPC validates the delivery ID, original URL, normalized target, consumption kind, and originating webContents before accepting an acknowledgement.

Confirmation-only Connect handling — Deep-linked endpoints are staged for explicit user confirmation rather than persisted or contacted automatically.

Strong lifecycle coverage — The new tests cover artifact identity, secure storage, persistence, evidence ordering, process cleanup, and several injected failure conditions.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F5: 🔴 Partial DMG mounts can escape cleanup

  • Required behavior: Native lifecycle validation must clean up or retain authority over every mounted DMG, including when the mount command fails or times out after performing the mount.
  • Evidence: apps/desktop/scripts/test-native-artifact-lifecycle.mjs:748DmgMountAuthority.attach() sets mounted = true only after hdiutil attach returns successfully; if hdiutil mounts the image and then fails or is killed, both extractDmg() and final cleanup treat it as unmounted and may remove the work root without detaching it.
  • Minimum fix: On every unsuccessful attach, query hdiutil info for the exact mount root and detach it or retain mounted authority until absence is proven. Add an injected attach-failure test covering a mount that became active before the command failed.

F6: 🔴 Failed registration can leave stale LaunchServices state

  • Required behavior: The added macOS lifecycle gate must unregister the copied application and prove the exact LaunchServices registration absent on every exit path.
  • Evidence: apps/desktop/scripts/test-native-artifact-lifecycle.mjs:1016LaunchServicesAuthority.register() sets registered = true only after lsregister -f succeeds. A timeout or nonzero exit after LaunchServices updates its database leaves the flag false, so cleanup skips both unregistering and assertGone() and deletes the copied bundle beneath a stale registration.
  • Minimum fix: Treat registration as tentative authority before invoking lsregister, or query exact registration state after any registration failure, and always unregister/prove absence when registration may have occurred. Cover the partial-success failure path in the authority tests.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Prove OS-dispatched processes exit

The lifecycle harness tracks directly spawned application process groups, but applications started indirectly through gio open or LaunchServices are not covered by that ownership mechanism. Adding an exact executable/bundle-path absence check before removing the copied application would strengthen the cleanup claim. This is optional because the current acknowledgement evidence demonstrates that the already-running primary instance handled the tested protocol link.

S2: 🟢 Recheck profiles after relaunch

The first-launch flow verifies that deep-linked endpoints were not persisted, whereas relaunch records confirmation evidence after checking the UI but does not repeat the exact profile-set assertion. Rechecking the two expected profiles after relaunch would make the confirmation-only regression proof more complete; the current renderer implementation itself still stages rather than saves the candidate.

Score

The core design and coverage are strong, but two newly introduced macOS authority gaps require correction, and the current head also has two unresolved Windows validation failures.

Score: 5/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 5m 48s
  • Tokens: 93,393 (74,457 in / 18,936 out)
  • Cost: $0.94

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Please address only the two concrete macOS cleanup failure paths F5/F6 from review 5558749232, and diagnose the current Intel macOS native CI failure in the same bounded pass. Keep S1/S2 and all Windows work deferred; do not pursue a score target or expand the lifecycle contract.

Current exact head 73f32ea: job 101472991876, run 34028244853 failed at FIRST_INITIAL_EVIDENCE / COLD_ACK / FAILED_EXIT at 10:54:34 UTC. Keychain setup and final restoration/deletion passed. Darwin ARM64 native DMG/ZIP lifecycle passes; all four Mac/Linux packaged Connect jobs and full test suite pass. Prior head 45e8ce3 had all four native target jobs green. Do not call this flaky without evidence; inspect actual failure diagnostics and use a targeted rerun if justified. Preserve strict acknowledgments and storage requirements. If existing diagnostics discard the cause, add a bounded secret-free failure category, not raw profile/credential logs.

For F5/F6 retain exact mount/registration cleanup authority across partial-success failures and add focused injected-failure coverage. Publish the minimal correction, provide exact-head test/CI evidence, and do not merge.

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 595d192

Summary of Changes

Implemented the bounded F5/F6 correction without touching S1/S2 or Windows work.

  • F5: DMG attachment now acquires tentative mount authority before hdiutil attach. Failed attaches query the exact mount root and retain authority until absence is proven. lifecycle harness
  • F6: LaunchServices registration now becomes tentative before lsregister -f, ensuring failure paths unregister and prove the exact copied bundle absent before removal. lifecycle harness
  • Added focused injected partial-success coverage for both paths. tests
  • Added fixed, secret-free failure categories for the Intel failure boundary—startup failure, cold-confirmation inspection/visibility, renderer loss, or uncaught exception. Required ACK and secure-storage evidence remain unchanged. classification

Intel CI diagnosis: job 101472991876 proves the child acknowledged the cold link, then exited before secure-storage probing or renderer-ready evidence. The retained diagnostics discard whether confirmation inspection failed, the UI remained invisible, or the renderer exited, so a narrower root cause cannot responsibly be claimed. A job-only rerun was attempted, but GitHub rejected it as non-rerunnable; I did not rerun the entire workflow because that would include deferred Windows jobs.

Validation:

  • Focused lifecycle authority suite: 23 passed.
  • Full desktop suite: 527 passed, 25 skipped.
  • Desktop and renderer typecheck: passed.
  • git diff --check: passed.

No visual preview was created because the changes are lifecycle cleanup and diagnostics only. Changes remain uncommitted and the PR was not merged, as requested.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 10m 39s
  • Tokens: 2,696,601 (2,677,619 in / 18,982 out)
  • Cost: $2.64

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5558752928✓

…1/S2 or Windows work.

Implemented the bounded F5/F6 correction without touching S1/S2 or Windows work.

- F5: DMG attachment now acquires tentative mount authority before `hdiutil attach`. Failed attaches query the exact mount root and retain authority until absence is proven. [lifecycle harness](/tmp/git-processor/worktrees/integry/propr/pr-2125-followup-2026-09-06T10-56-38/apps/desktop/scripts/test-native-artifact-lifecycle.mjs:693)
- F6: LaunchServices registration now becomes tentative before `lsregister -f`, ensuring failure paths unregister and prove the exact copied bundle absent before removal. [lifecycle harness](/tmp/git-processor/worktrees/integry/propr/pr-2125-followup-2026-09-06T10-56-38/apps/desktop/scripts/test-native-artifact-lifecycle.mjs:932)
- Added focused injected partial-success coverage for both paths. [tests](/tmp/git-processor/worktrees/integry/propr/pr-2125-followup-2026-09-06T10-56-38/apps/desktop/scripts/test-native-artifact-lifecycle.test.mjs:239)
- Added fixed, secret-free failure categories for the Intel failure boundary—startup failure, cold-confirmation inspection/visibility, renderer loss, or uncaught exception. Required ACK and secure-storage evidence remain unchanged. [classification](/tmp/git-processor/worktrees/integry/propr/pr-2125-followup-2026-09-06T10-56-38/apps/desktop/scripts/test-native-artifact-lifecycle.mjs:1259)

Intel CI diagnosis: [job 101472991876](https://github.com/integry/propr/actions/runs/34028244853/job/101472991876) proves the child acknowledged the cold link, then exited before secure-storage probing or renderer-ready evidence. The retained diagnostics discard whether confirmation inspection failed, the UI remained invisible, or the renderer exited, so a narrower root cause cannot responsibly be claimed. A job-only rerun was attempted, but GitHub rejected it as non-rerunnable; I did not rerun the entire workflow because that would include deferred Windows jobs.

Validation:

- Focused lifecycle authority suite: 23 passed.
- Full desktop suite: 527 passed, 25 skipped.
- Desktop and renderer typecheck: passed.
- `git diff --check`: passed.

No visual preview was created because the changes are lifecycle cleanup and diagnostics only. Changes remain uncommitted and the PR was not merged, as requested.

PR: #2125
Comment by: @integry (ID: 5558752928)
Model: gpt-5.6-sol
@integry

integry commented Sep 6, 2026

Copy link
Copy Markdown
Owner

/merge

Merge the current PR base 1950-epic-cross-platform-dsk INTO this PR branch and resolve the conflicts introduced by setup #2132 (epic head a552305). Do not merge this PR into the epic or main. Preserve the full real Linux setup/controller/shutdown changes now in the epic alongside every reviewed native lifecycle, ACK drain, Linux libsecret, and Darwin cleanup fix from head 595d192. Preserve endpoint defaults, rollback, and macos-linux-v1 release policy. This is a narrow integration conflict-resolution pass, not a broad cleanup/review cycle. Run focused tests for conflict-touched behavior and let Linux/macOS CI validate the result. Windows remains deferred. Report exact conflict resolutions and any demonstrated blocker.

@propr-dev

propr-dev Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

🔀 Resolved merge conflicts from 1950-epic-cross-platform-dsk into 2122/gpt-5.6-sol-integrate-native-mac-linu-20260904-2105-al1 in commit ba7cf6f

Conflicting Files

  • apps/desktop/src/ipc.ts
  • apps/desktop/src/main.ts
  • apps/desktop/src/preload-bridge.ts

Resolution Summary

Resolved all merge-conflict markers in:

  • ipc.ts: Combined the PR’s deep-link validation/security imports with the incoming DesktopSetupController import. Both features are required by the merged IPC handlers.
  • main.ts:
    • Preserved the PR’s nativeProfiles assignment while adding the incoming Linux setup-host/controller initialization.
    • Passed both deepLinks and setup to the shutdown coordinator, ensuring both acknowledged deep-link work and setup operations drain safely during shutdown.
  • preload-bridge.ts:
    • Retained the PR’s safer unknown IPC payload boundary instead of the incoming any.
    • Combined acknowledged deep-link delivery/buffering with incoming local-setup progress listeners.
    • Added a localized setup-snapshot type assertion when dispatching the IPC payload, preserving strict boundary typing while satisfying the typed bridge contract.

Verification completed:

  • Repository-wide conflict-marker scan: clean.
  • git diff --check: passed.
  • Desktop TypeScript typecheck: passed.
  • Focused preload/IPC/shutdown tests: 27 passed, 0 failed.
  • No commit was created.

Git could not stage the three files because the linked worktree index lock returned Permission denied. I did not modify or reinitialize .git. The resolved working-tree files remain intact, but Git still reports them as UU until the system stages them with sufficient permissions.


🤖 Resolution Details

  • Model: gpt-5.6-sol
  • Time: 3m 57s

View Task Execution


System-triggered merge conflict resolution

…gpt-5.6-sol-integrate-native-mac-linu-20260904-2105-al1

Automatically resolved merge conflicts after merging target branch into PR branch.
Resolved conflicts in:
- apps/desktop/src/ipc.ts
- apps/desktop/src/main.ts
- apps/desktop/src/preload-bridge.ts

PR: #2125
Model: gpt-5.6-sol
@integry
integry merged commit 9fef547 into 1950-epic-cross-platform-dsk Sep 6, 2026
27 of 32 checks passed
@integry
integry deleted the 2122/gpt-5.6-sol-integrate-native-mac-linu-20260904-2105-al1 branch September 6, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant