[sudoHG]

Closes #255

Summary

Recover transient notarization failures under a 30-minute monotonic budget per artifact, using capped backoff and status polling for a known submission ID.
Give each credential-mode request its own Ask Key run and fresh delivered key file; the runner never reads or copies the key, and credential variables expand only inside the helper's target shell.
Suggest a timed allowance on the first card; later requests may require approval again within the remaining budget, as explicitly accepted in the issue's updated scope.
Reconcile uncertain uploads through history before bounded resubmission, retain IDs on failures and interrupts, and preserve the four assets, stapling and verification gates.

Diff stat

Base: 3ad4258e17e90ef31baf663461674888e91bc235 (origin/main). Head: 501f593a80efe4a8d0f65cb15706419ec2e478f0. Rebased onto the fetched base before pushing.

 Tests/Automation/Fixtures/package-release-tool.py | 106 ++++++
 Tests/Automation/test_notarize_release.py         | 411 ++++++++++++++++++++++
 Tests/Automation/test_package_release.py          | 188 +++++-----
 docs/testing.md                                   |  12 +-
 scripts/notarize-release.py                       | 291 +++++++++++++++
 scripts/package-release.sh                        |  65 ++--
 6 files changed, 936 insertions(+), 137 deletions(-)

Tests

• PASS: python3 -m unittest discover -s Tests/Automation -p test_package_release.py -v: 19 passed, 0 failed, 0 skipped (217.976 seconds).
• PASS: python3 -m unittest discover -s Tests/Automation -p test_notarize_release.py -v: 24 passed, 0 failed, 0 skipped (0.012 seconds).
• Comparison with base: 17 packaging tests on the base versus 19 packaging and 24 runner tests on this branch (43 total). The base suite was not rerun separately.
• PASS: real --no-notarize packaging with an ad-hoc-signed synthetic app produced the unnotarized DMG and checksum, mounted read-only, verified the signature and Applications symlink, checked the checksum, detached, and cleaned up. Synthetic notarization still verifies all four release assets and both checksums.
• PASS: the fake helper filters the environment using the Broker allowlist and executes the actual shell with a fresh regular key file per request. Every operation ID/key path is distinct, delivered files disappear on target exit, and the target-shell argument stays below 512 bytes (Broker limit: 4 KiB).
• PASS: injected clocks/sleeps cover fast transient errors, the shared deadline, 300/120-second request caps including approval waiting, fresh helper requests beyond five minutes, repeated transient history errors without counting empty snapshots, bounded uploads, partial-output IDs, clock-skew ambiguity, delayed history adoption, incomplete adopted uploads, rejection, malformed results, and profile-mode construction. Upload-started tracking preserves history guidance after later approval failures; a helper killed at the overall deadline reports deadline expiry instead of approval denial.
• PASS: synthetic process-group SIGINT and SIGTERM runs preserve the saved ID before cleanup. Approval denial and missing delivery do not suggest history lookup or resubmission; secret-bearing diagnostics remain absent from public output.
• swift build: PASS on the initial implementation head bd315187 (15.51 seconds). The coordinator waived repeat builds for the review fixes because no Swift files changed. Local Swift unit tests and desktop E2E: NOT_RUN; final-head full-suite and desktop CI results are recorded below.

Checks

• PASS: bash -n scripts/package-release.sh.
• SKIPPED: shellcheck is not installed.
• PASS: python3 scripts/check_hygiene.py, python3 scripts/check_module_deps.py, and git diff --check.
• PASS: df -k /System/Volumes/Data before the only build: 252,209,624 KiB available, above 80 GiB. Task .build removed after validation (about 843 MiB plus small manifest caches); temporary packaging fixtures and mounts cleaned up.
• Real notarization/network recovery: NOT_VERIFIED, reserved for the next release. This worker used no real Ask Key credential/helper, signing identity, login keychain, production data, installed app, app launch, screen capture, or local desktop E2E.
• PASS: all three jobs in final-head run 41599032562, on 501f593a80efe4a8d0f65cb15706419ec2e478f0.
  • build-and-test: Swift executed 1,618 tests, with 3 skipped and 0 failures (1,615 passed); Python automation ran 233 tests successfully.
  • basic-ui-flows: required desktop gate passed, 15 passed, 0 failed, 0 skipped; optional screenshot collection also completed.
  • release-sdk-window-check: Debug/Release SDK-marker checks and the Release SDK management-window lifecycle test passed (1 test, 0 failures).

Deviations and questions

The coordinator approved the expanded runner/docs/recovery scope and explicitly replaced the original one-approval-per-artifact requirement with per-request delivery. A timed allowance can avoid repeated cards; later cards remain possible and accepted. No unresolved implementation question remains.

History matching uses the first upload time rounded down to a second, with a three-minute pre-start ambiguity margin. A matching entry in that margin or multiple matches stop with all observed IDs. Transient history failures retry without counting empty snapshots; only three successful empty snapshots permit another upload, up to three uploads total, and a known ID is never resubmitted. History visibility delays and larger clock skew remain limitations. An adopted ID still In Progress at the deadline reports a possibly incomplete upload and directs the operator to its log. Resuming stapling from an existing ID is out of scope.

[luoji-bot]

Review round 1: request changes (independent Opus review of the PR head, verified by the coordinator)

No credential leaks and no automatic resubmission. But the retry policy would not have saved the v0.5.0 release:

1. P1: retries outlive the delivered key file. Ask Key deletes a delivered temporary file 5 minutes after delivery or at approval expiry (FileDeliveryManager.swift ~42, ~136-137; the approval expiry counts from request creation, so time spent approving is included). Before this PR one notarytool submit --wait read the key once; now every wait is a new process that must re-read $PRIVATE_KEY_FILE, so later attempts fail with a non-transient key/file error while Apple is still processing. Fix: in the Python runner, read the key bytes once at start and give each notarytool call the key through an inherited pipe (--key /dev/fd/N with pass_fds), never writing it anywhere. Confirm notarytool accepts that path with a stubbed check in tests and note in the PR that the real path is verified at the next release; if it does not work, stop with a clear message rather than looping.
2. P1: the budget is attempts, not time, and submit is unprotected. Four attempts with 1/2/4 s backoff can be spent in seconds on fast-failing codes, against 1-5 minutes of processing; and submit itself fails about one time in three on that network. Fix: poll with notarytool info <id> (one request per process) under a wall-clock deadline (for example 30 minutes) with capped backoff (for example 5 s growing to 60 s), treating transient errors as "try again". For submit, on a transient failure, first look for the submission in notarytool history (same artifact name, created after this submit started) and adopt its ID; only if none exists, submit again, at most a few times. Print every submission ID obtained.
3. P2: tests cannot see either P1. Make the fake tool model elapsed time (or a call counter standing in for it), a key file that disappears after a deadline and on target exit, a submit that times out and then appears in history, and fast-failing transient codes. Keep test run time short (inject sleep).
4. P3s to take now: update docs/testing.md (~123-126) for the new flow; make the "submission ID unavailable" message distinguish approval denial/timeout from a failed submit; add a test that the inline script stays well under the broker's 4 KB per-argument limit (BrokerLimits.swift ~6) or move the runner into a separate file under scripts/ passed by path; avoid the nested heredoc inside $(...) if you keep it inline.

Out of scope here (note it in the PR): resuming stapling from an existing submission ID.

[wangmiao-bot]

Addressed review round 1 at final head 66bc3cbb62bd4c2b3c84686845d2e75d5d19c2eb, following the scope update in #255.

1. The runner reads the delivered key once, then supplies each child through a fresh inherited anonymous pipe with pass_fds and --key /dev/fd/N. It creates no replacement key file. Tests delete the original file during execution and after five simulated minutes; subsequent calls still read the expected bytes. Actual Apple notarytool pipe support remains NOT_VERIFIED until the next release, and a permanent pipe/authentication error stops with sanitized guidance.
2. Submission, history reconciliation, polling and backoff share a monotonic 30-minute deadline. Each subprocess has a timeout bounded by the remaining budget; info polling uses 5-second backoff capped at 60 seconds. Transient submission failures reconcile by artifact name and creation time since the first upload; one matching ID is adopted, all observed IDs are reported, and ambiguous/unavailable history stops. Only three successful empty snapshots allow another upload, with at most three uploads total. History visibility and clock-skew limitations are documented.
3. Added 16 deterministic runner tests with injected time/sleep and timeout/error responses, plus 18 packaging integration tests. The fake helper actually executes the target shell and the fake child reads the inherited pipe, including after the delivery file disappears. The final follow-up also preserves an ID emitted before a subprocess timeout, bypassing history and avoiding another upload. Final local result: 34 passed, 0 failed, 0 skipped.
4. Updated docs/testing.md, distinguished a target that never starts (approval denial/timeout/helper failure) from submission failure, and moved the runner to scripts/notarize-release.py. The shell argument is tested below 512 bytes, well below 4 KiB; the nested shell heredoc is gone. Resuming stapling from an existing ID remains out of scope.

bash -n, hygiene, module dependencies and diff checks passed; shellcheck is unavailable. No Swift changes or repeat build were needed under the coordinator's waiver. The branch was rebased onto origin/main; final-head CI is running and the PR receipt will be updated after all jobs complete.

[luoji-bot]

Review round 2: request changes (independent Opus re-review of the round-1 head, plus a coordinator check on the real tool)

Round 1 is implemented and bounded, with no leak path found. But one blocker:

1. Blocker: notarytool does not accept a key through a pipe. I checked with a real (non-release) key on this Mac: notarytool history --key /dev/fd/3 … 3< <(cat key) and --key /dev/fd/0 both fail every time with The file "3" couldn't be opened because you don't have permission to view it (it opens the path as a regular file). So credential mode as written would fail on every release. Replace the in-memory pipe with one Ask Key run per notarytool request: the runner stays outside Ask Key and calls askkey run --wait-for-approval --credential <name> --operation-id <new uuid> … -- /bin/bash -c 'xcrun notarytool <action> … --key "$PRIVATE_KEY_FILE" --key-id "$KEY_ID" --issuer "$ISSUER_ID" --output-format json' bash <args> for submit, history and each status poll. Each request then gets a freshly delivered key file well inside its 5-minute lifetime, and the key never leaves Ask Key's delivery. Before the first request, print one line telling the operator to approve the first card with a timed allowance (Allow for N Minutes) so later polls don't each need a card; if a later request still needs approval, --wait-for-approval handles it within the remaining deadline. Keychain-profile mode keeps calling notarytool directly. Keep the 30-minute deadline, capped backoff and bounded uploads.
2. P2: transient history failures end the run. Retry transient errors on read-only history/info within the deadline and don't count them as empty snapshots; still stop on invalid JSON, bad entries or multiple matches.
3. P2: no clock-skew margin in history matching. Round started down to the second and subtract a margin (for example 3 minutes); same-name entries inside [started - margin, started) are ambiguous: stop and report their IDs rather than ignoring or adopting them. Add tests for a fast and a slow local clock.
4. P2: adopted submission that never finished uploading. When an adopted submission hits the deadline still In Progress, report a distinct reason (likely incomplete upload; check notarytool log before resubmitting) and mention it in docs/testing.md.
5. P2: IDs lost on Ctrl-C/TERM. Make the INT/TERM handlers print any submission IDs already in the response file before exiting.
6. P2: test_…window starts at the first upload cannot fail because now() is constant; make it advance with elapsed time and place the entry between the first and second attempt.
7. P3s: the "request" failure text mentions key pipes (drop it); after a credentials failure don't say "inspect history before submitting again"; let the fake askkey filter the environment like the broker (HOME, PATH, TMPDIR, LANG, TERM, SHELL, USER, LOGNAME, LC_*); reset runner.delay between subtests; cover the profile-mode constructor.

Real notarization is still verified at the next release; say so in the PR.

[wangmiao-bot]

Addressed review round 2 in 37f0dc5586d03220d62fdde870b8e3af3af58cfc, following the explicitly confirmed delivery-model scope update.

1. Removed key caching and inherited pipes. The runner stays outside Ask Key and invokes one fresh askkey run --wait-for-approval per submit/history/info request, with a unique operation ID and a short target shell that alone expands the three credential mappings. The first-request notice recommends a timed allowance; later approval cards are accepted and consume the remaining deadline. The fake helper filters the environment like the Broker and creates a new regular key file for each request.
2. Transient history errors retry within the deadline and do not count as empty snapshots. Invalid/permanently failed history still stops. Valid matching IDs in partial timeout output are preserved rather than discarded.
3. The first-upload timestamp is rounded down to a second. Same-name IDs in the three-minute pre-start margin are reported as ambiguous; they are neither ignored nor adopted. Fast/slow-clock and rounding tests cover these cases.
4. An adopted ID that remains In Progress at the deadline produces a distinct possibly-incomplete-upload diagnostic, with log guidance; documented in docs/testing.md.
5. INT and TERM traps print validated IDs already saved in the response file before cleanup. Both signals pass synthetic process-group integration tests, including cleanup and redaction.
6. The history-window test now uses an advancing UTC clock and an entry between the first and second attempts, so moving the window forward breaks it.
7. Removed pipe-specific request text; approval/missing-delivery failures with no ID do not suggest history or resubmission. Tests use the Broker environment allowlist, reset backoff between cases, and construct profile mode normally.

Local validation: 19 packaging tests and 22 runner tests passed (41 total, 0 failed, 0 skipped); bash syntax, hygiene, module dependencies and diff checks passed. Shellcheck is unavailable; no Swift rebuild was required under the coordinator waiver. Branch rebased onto origin/main; final-head CI is running. Real notarization remains NOT_VERIFIED until the next release, and this worker used no real credentials or real notarization calls.

[luoji-bot]

Review round 3: one small change (independent Opus re-review of 37f0dc5, verified). Last round: after this, only P0–P2 block merge.

Round 2 is fully in place, including the per-request Ask Key runs, history retries, the clock-skew window, adopted-incomplete reporting, IDs on INT/TERM, the fixed test and the P3s. No leak path found.

1. P2: after an upload has started, a later approval or request failure says nothing was submitted. summarize() picks the hint from the final reason only (notarize-release.py ~241-242), and request() raises approval whenever the start marker is missing (~97-99), including for a history query after a submit that already began uploading. The operator is then told the request "did not start", with no history hint, and a rerun makes a duplicate. Emit an upload-started event the first time a submit passes the marker (credential mode) or starts (profile mode); suppress the history hint and the "did not start" wording only when that event is absent. Add a test: transient submit, then a history request without a marker.
2. P3s to take with it: docs/testing.md (~126) should show the target shell with the start marker as TARGET_SHELL actually uses it; give credential-mode requests the same per-request cap as profile mode (300 s submit, 120 s query, bounded by the remaining budget); the ambiguous text should not say "Multiple" when it is a single entry from the margin window; report deadline (or adopted-incomplete) rather than approval when the runner itself kills a helper that was still waiting at the deadline.

[wangmiao-bot]

Addressed review round 3 in 501f593a80efe4a8d0f65cb15706419ec2e478f0.

1. Added a one-time upload-started event for a profile upload attempt or a credential submit that reaches the target marker. A later approval/request failure retains history guidance when an upload was attempted; an approval failure after that point no longer says the request did not start. The regression covers transient submit followed by a history request without a marker. The target checks that the delivered file is regular/readable before emitting the marker, preserving the no-upload/no-history behavior for a missing delivery.
2. Updated the documentation's target shell to match TARGET_SHELL, including the marker and file-readability guard. Both authentication modes now use 300-second submit and 120-second query caps bounded by the remaining budget, including approval waiting. The ambiguity message covers a single clock-margin entry as well as multiple matches. A helper killed at the overall deadline reports deadline expiry (or the existing adopted-incomplete diagnostic) rather than approval denial; covered by a new regression.

Final local validation: 24 runner tests and 19 packaging tests passed (43 total, 0 failed, 0 skipped), plus bash syntax, hygiene, module dependency and diff checks. Shellcheck is unavailable; no Swift code changed or rebuild was required. The branch was rebased before pushing, build artifacts are removed, and final-head CI is running at https://github.com/sudoHG/AskKey/actions/runs/41599032562. Real notarization remains NOT_VERIFIED until the next release.