Repository navigation
feat: prepare matching Codex voice builds without delaying text startup - #62
Conversation
Keep text startup independent of voice provisioning and global npm packages unchanged. Qualify private native builds before activation and preserve owned launcher rollback.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ba7ec28de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with open(state / 'sync.lock', 'a') as lock, bridge.helper_lock(state / 'config.json'): | ||
| fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) | ||
| previous = bridge.load_state(state) if (state / 'config.json').exists() else None | ||
| desired = {'codex': str(entry), 'host': args.host, 'identity': args.identity, 'launcher': str(target)} |
There was a problem hiding this comment.
When manage receives a relative identity such as --identity ./key, it stores that path unchanged even though bridge.ssh_args() resolves it during validation. The background worker later calls ssh_args() again from its own working directory, so it selects a different, potentially nonexistent key. This prevents matching builds from being prepared when authentication depends on that identity, despite the key being valid at installation time. Persist the expanded absolute identity path, as is already done for the Codex entry point.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d630349. manage now expands and resolves the identity before both validation and persistence. The regression covers relative, tilde, absolute and absent identities, changes to the worker working directory, and verifies the selected SSH key. Relative and tilde cases failed before the fix; all 38 relay tests now pass.
| unit_text = ('[Unit]\nDescription=Codex native voice build synchronization\nStartLimitIntervalSec=0\n' | ||
| '[Service]\nType=oneshot\nExecStart=' + command + '\nEnvironment=' + environment | ||
| + '\nTimeoutStartSec=7200\nRestart=on-failure\nRestartSec=5min\n' | ||
| 'NoNewPrivileges=yes\nProtectSystem=strict\nReadWritePaths=%h\n') |
There was a problem hiding this comment.
When --state-dir resolves outside the user's home directory, installation accepts it, but the generated unit combines ProtectSystem=strict with only ReadWritePaths=%h. On systems enforcing this sandbox, the updater cannot write to that state directory: even opening sync.lock for append in sync() fails before its error-recording handler runs, so no build can become ready. This also affects state directories reached through home-directory symlinks pointing to another filesystem. Include the resolved state directory in the unit's writable paths, with appropriate systemd escaping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d630349. The generated unit retains ProtectSystem=strict and adds the resolved state directory to ReadWritePaths with field-appropriate quoting (percent escaped, dollar literal). The regression uses a state directory outside the fixture home and retains rollback checks. A real generated systemd user unit with spaces, dollar and percent characters in an outside-home state path passed structured D-Bus ReadWritePaths readback and wrote there successfully; the temporary unit and files were removed. Systemd semantics checked against https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#ReadWritePaths=.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codex upgrades invalidate the old voice launcher, which checks one hard-coded build and warns on every startup. Add opt-in managed mode that starts normal text Codex while a systemd user worker prepares a private copy of the installed release and its matching Windows helper. The global npm package remains untouched by the relay. New versions and normal preparation are quiet; startup reports an actual failure recorded for the current build, a damaged prepared build, or an unavailable updater.
Only builds that pass identity, manifest hashes, native handshake, runtime initialization and default-device enumeration become eligible on the next launch. Transient registry/SSH failures retry in the background; incompatible or modified builds are held without repeated downloads. Older build failures do not warn about a newly installed release. Immutable builds remain available to active sessions. Manage/unmanage verify ownership and preserve the previous launcher and permissions. Manager source upgrades require a new state directory; native Codex release matching is automatic.
Validation at d630349:
Earlier build validation covered isolated Windows staging/reuse, malformed JSON, missing-file/hash rejection, transient registry classification, actual CLI argument/environment/cwd/exit preservation, systemd literal-path quoting and rollback. Full details remain in the initial PR revision.
Shape: 472 counted lines, one managed-update lifecycle.
Limits: successful automatic qualification is not proof of spoken conversation on every future release. A release that changes the private protocol must pass the checks before activation. This PR has not replaced the live launcher.
E2E: After merge, adopt managed mode with the live launcher ownership hash, inspect status for the installed release, run doctor --audio against its prepared relay state, verify quiet text startup and a spoken F8 conversation, and check that the saved prior launcher permits rollback.