Extract shared modules/approval_token.sh from cleanup.sh and login_items.sh - #67
Merged
Merged
Conversation
…ems.sh
Phase 5-4 ('비파일 maintenance executor 설계, 4번째 REMOVE_MODE'): read
cleanup.sh's REMOVE_MODE values for real first (remove/trash/simulator/
contents already exist -- the label's "4th mode" framing doesn't match
current code) and login_items.sh's own approval-token functions side by
side with cleanup.sh's. They were byte-for-byte identical in four places
(new_approval_token, prepare_private_directory, approval_token_file_size,
read_approval_token_file) -- and the duplication already had a real,
demonstrated cost: the immediately preceding two PRs each had to
independently fix a bug in login_items.sh's copy that cleanup.sh's
original either already handled (the Darwin/Linux stat branch) or that
both scripts shared without either copy protecting the other (the sealed
/dev/fd sourcing failure). A third non-file action would have meant a
third copy and a third chance to drift.
scripts/modules/approval_token.sh now holds all four functions once,
sourced by both scripts through the same PCH_PINNED_*_MODULE convention
modules/support_dir.sh already established (env-selected pinned
descriptor for sealed runs, dirname-relative fallback for dev mode).
Wired into RuntimeExecutionContext.pinnedApprovalTokenModule() and the
three Swift call sites that actually issue or consume approval tokens
(prepareCleanup, executeCleanup, LoginItemService.invoke) -- not
setStorageWatchEnabled, schedule.sh never used token-based approval.
Manifest content (what was approved) stays entirely separate per script
and is NOT shared: cleanup.sh's write_current_manifest still tracks
target paths/sizes/inodes, login_items.sh's cmd_preview still tracks just
a name -- only the token issue/transport/single-use-consume/size-check
plumbing underneath either manifest shape was ever actually identical.
Also closed a third instance of the exact release-manifest-completeness
gap PRs #64 and #65 already found twice: scripts/modules/*.sh (flat,
where support_dir.sh and now approval_token.sh live) was never
glob-guarded at all, unlike scripts/modules/macos/*.sh. New glob
assertion in test_service_contracts.py closes it before it can repeat a
fourth time.
Verification: real end-to-end re-run against a genuine disposable login
item on this machine (create via osascript, preview/execute through the
refactored login_items.sh, confirm removal) after the extraction, not
just unit tests. The sealed-fd contract test from the prior PR now pins
both modules and was proven to fail against a missing
PCH_PINNED_APPROVAL_TOKEN_MODULE pin for cleanup.sh/login_items.sh (and
correctly NOT fail for schedule.sh, which never needed it) via a
revert/restore cycle; same for the new release-manifest glob assertion,
proven to fail with the exact missing file named before the fix. One
real regression caught by the full suite before this commit:
test_write_receipt_strips_embedded_tabs_from_target_paths extracted
prepare_private_directory directly from cleanup.sh's own source, which no
longer contains it -- fixed to extract from the new module file instead.
344 pytest passed, 142 Swift tests passed, release_smoke.py --check-only
ok (macos_entries 140->141).
3 tasks
heznpc
added a commit
that referenced
this pull request
Aug 13, 2026
* Fix build_macos_swift_app.sh silently omitting shipped scripts release_smoke.py's manifest-completeness checks and the app's own RUNTIME_FILES array (which controls what actually gets copied into the signed bundle) were never cross-checked, so a script could pass every existing guard while still being silently absent from the real shipped app. Confirmed for real against this machine's actual installed runtime: modules/macos/idle_cpu.sh, privacy.sh, and devtool_updates.sh were all missing. scanner.sh's sibling-relative `source` failed for each with no `set -e` to stop it, so the scan kept going with three collector functions permanently undefined -- "command not found" on stderr, and the resulting scan_result.json silently reported `collection.complete: true` with all three sections simply absent. Separately, scripts/login_items.sh and modules/approval_token.sh were also missing -- that path fails closed instead (pinnedApprovalTokenModule returns nil when the sealed payload lacks the module), meaning every cleanup preview/execute and login-item removal has failed outright in a real signed build since approval_token.sh was extracted out of cleanup.sh (#67). No test caught it because every existing test runs against the checkout directly, never the installed runtime tree a real user's app uses. Adds a permanent glob-based regression guard, matching the same pattern already used for release_smoke.py's own manifest gaps: any new scripts/**/*.sh file is now required to appear in RUNTIME_FILES unless explicitly build-time-only. * Remove network_watch.sh from this branch's RUNTIME_FILES That file doesn't exist on this branch -- it's part of the separate, not-yet-merged Phase 5-1 work, and CI correctly caught the reference to a nonexistent path. It belongs in that PR instead, alongside the file itself. --------- Co-authored-by: Heznpc <heznpc@users.noreply.github.com>
4 tasks
heznpc
added a commit
that referenced
this pull request
Aug 13, 2026
Six items the #74/#75 reviewers classified low/informational -- nothing here is reachable in a shipped build today; each is a seam a future change could silently reopen. privacy.sh's PCH_TCC_DB_PATH test seam now opens only under PCH_TEST_MODE=1, matching PCH_TEST_BREW_BIN and PCH_TEST_OSASCRIPT_BIN. It was the one injection point in the new modules without the gate, and ungated it doubles as an arbitrary-sqlite-read redirect for any future caller that forwards the variable. login_items.sh now refuses a manifest whose protocol version differs from its own -- fields this version reads could mean something else under another one. cleanup.sh cross-checks its whole manifest; this is the minimum equivalent. The bundled-runtime completeness guard now also covers non-.sh runtime dependencies (rules/*.json, data/*.json, report_i18n, *.jxa.js, and shipped .py), with an explicit build/reference-only exclusion list mirroring the .sh guard's. Proven the same way: removing rules/network.json from RUNTIME_FILES trips it. RuntimeWorkspace's bundled branch now demands the development opt-in whenever the injected resourceURL is not the running app's own signed bundle. That combination skips the code-signature payload comparison and is constructible only through parameter injection; without the gate, a future refactor decoupling the parameters would run unsigned code with no test noticing. The revalidation tests declare the opt-in they were implicitly relying on. Raw pinned-file values now refuse on key collision instead of silently clobbering whichever module payload merged first -- the exact asymmetry behind the approval_token/approval_token_module near-miss fixed in #67. Observation timeout slack grows from 20s to 60s past the window (the snapshots run exactly when the machine is busiest), and a timeout now says it timed out instead of "status 124". Co-authored-by: Heznpc <heznpc@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 5-4 ("비파일 maintenance executor 설계, 4번째 REMOVE_MODE"). Read
cleanup.sh'sREMOVE_MODEvalues for real first:remove/trash/simulator/contentsalready exist, so the "4th mode" framing in the task label doesn't match current code. Readlogin_items.sh's own approval-token functions side by side withcleanup.sh's instead, since that's the concrete pattern both scripts (and any future non-file action) actually share.They were byte-for-byte identical in four places (
new_approval_token,prepare_private_directory,approval_token_file_size,read_approval_token_file) — and the duplication already had a real, demonstrated cost: the two immediately preceding PRs (#65, #66) each had to independently fix a bug inlogin_items.sh's copy that eithercleanup.sh's original already handled (the Darwin/Linuxstatbranch) or that both scripts shared without either copy protecting the other (the sealed/dev/fdsourcing failure). A third non-file action would have meant a third copy and a third chance to drift.scripts/modules/approval_token.shnow holds all four functions once, sourced by both scripts through the samePCH_PINNED_*_MODULEconventionmodules/support_dir.shalready established (env-selected pinned descriptor for sealed runs, dirname-relative fallback for dev mode). Wired into a newRuntimeExecutionContext.pinnedApprovalTokenModule()and the three Swift call sites that actually issue or consume approval tokens (prepareCleanup,executeCleanup,LoginItemService.invoke) — notsetStorageWatchEnabled,schedule.shnever used token-based approval.Manifest content (what was approved) stays entirely separate per script and is not shared:
cleanup.sh'swrite_current_manifeststill tracks target paths/sizes/inodes,login_items.sh'scmd_previewstill tracks just a name — only the token issue/transport/single-use-consume/size-check plumbing underneath either manifest shape was ever actually identical.Also closed a third instance of the exact release-manifest-completeness gap PRs #64 and #65 already found twice:
scripts/modules/*.sh(flat, wheresupport_dir.shand nowapproval_token.shlive) was never glob-guarded at all, unlikescripts/modules/macos/*.sh. New glob assertion intest_service_contracts.pycloses it before it can repeat a fourth time.Test plan
osascript, preview/execute through the refactoredlogin_items.sh, confirm removal) after the extraction, not just unit tests.PCH_PINNED_APPROVAL_TOKEN_MODULEpin forcleanup.sh/login_items.sh(and correctly NOT fail forschedule.sh, which never needed it) via a revert/restore cycle.test_write_receipt_strips_embedded_tabs_from_target_pathsextractedprepare_private_directorydirectly fromcleanup.sh's own source, which no longer contains it — fixed to extract from the new module file instead.release_smoke.py --check-onlyok (macos_entries 140→141).