Skip to content

Refactor module split - #8

Closed
GGzili wants to merge 5 commits into
Wangnov:mainfrom
GGzili:refactor-module-split
Closed

Refactor module split#8
GGzili wants to merge 5 commits into
Wangnov:mainfrom
GGzili:refactor-module-split

Conversation

@GGzili

@GGzili GGzili commented May 17, 2026

Copy link
Copy Markdown

No description provided.

GGzili and others added 5 commits May 12, 2026 02:27
@GGzili
feat: add restore, prune-backups, sync --dry-run, and desktop cap war…
e7503f2 
…ning

- Add `restore <backup>` command to restore SQLite from .bak backup files
- Add `prune-backups --keep N` command to remove old backups, keep N most recent
- Add `--dry-run` flag to `sync` command for preview without writing
- Add 50-session desktop cap warning to `status` command
- Full i18n support (EN/ZH_HANS) for all new features
- Auto-list available backups when no path argument given to restore
- Backup directory auto-discovery alongside SQLite file
@GGzili
fix: add missing #[arg(long)] on Sync.dry_run and resolve_sqlite_path…
b0df1ba 
… ? errors
@gzili
refactor: split monolithic main.rs into focused modules
8bc77ac 
Break the 3324-line main.rs into separate modules by domain:

- types.rs: shared structs, enums, type aliases
- locale.rs: all i18n functions (Locale enum, help text, labels)
- config.rs: config file reading, path resolution, shared constants
- sqlite.rs: SQLite operations (inspect, reconcile, sync helpers)
- rollout.rs: rollout metadata file patching and progress tracking
- backup.rs: backup/restore/prune and reconcile orchestration
- watch.rs: file watcher daemon (run_watch)
- status.rs: display/print functions and status collection

main.rs is now a thin ~1000-line entry point containing only:
CLI structs, main(), parse_cli(), validate_provider_override(),
localized_command(), install_service(), uninstall_service(), and tests.

All 22 tests pass (19 pass, 3 pre-existing Windows failures identical
to baseline).
@gzili
fix: make all 22 tests pass on Windows
edf1c22 
- Fix TOML path escaping: replace Windows backslashes with forward
  slashes when writing paths into TOML config files in tests
- Make builds_install_next_steps platform-agnostic: don't assert
  macOS-specific launchctl output on non-macOS
- Normalize path separators in comparison for Windows compatibility
@gzili
fix: cross-platform home dir detection and remove macOS-specific naming
75d269c 
- default_codex_home() now falls back through HOME → USERPROFILE →
  HOMEDRIVE+HOMEPATH, working in Windows cmd/powershell
- Removed macOS-specific CLI aliases (install-launchd, uninstall-launchd,
  print-plist)
- Renamed locale functions from launchd/plist prefix to platform-neutral
  service_* / config_path_* names
- Same home dir fix applied to service.rs private default_codex_home()
@Wangnov

Wangnov commented May 18, 2026

Copy link
Copy Markdown
Owner

Thanks for doing the module split. I like the overall direction: moving config/sqlite/rollout/backup/status/watch/service/locale/types out of the giant main.rs will make the project much easier to maintain.

However, this PR should not be merged in its current form. Since #7 and its follow-up have now landed on main, this branch needs to be rebased and cleaned up first.

Blocking items:

  1. Preserve the Windows VBS startup fixes from current main

Current main now uses the hidden .vbs Windows Startup launcher, writes the .vbs before deleting the legacy .cmd, removes both on uninstall, and reports the legacy .cmd path correctly if that is what is installed.

This branch still contains the older Windows .cmd startup implementation in src/service.rs:

  • service_config_path() returns codex-threadripper.cmd
  • install_windows_startup() writes build_windows_startup_script(...)
  • build_windows_startup_script() emits a batch file with start "" /B ...
  • uninstall only removes the current config path

Please rebase onto current main and make sure the module split preserves the new VBS behavior exactly.

  1. Keep this PR as a pure refactor, or clearly separate the feature work

This PR is titled Refactor module split, but it also carries feature changes from #5: restore, prune-backups, sync --dry-run, and the desktop 50-session warning.

That makes this difficult to review because behavior changes and file movement are mixed together. Please either:

  1. Do not drop the legacy CLI aliases

Current main still supports:

  • print-plist → print-service-config
  • install-launchd → install-service
  • uninstall-launchd → uninstall-service

This branch removes install-launchd / uninstall-launchd and replaces print-plist with print-config. That is a compatibility regression, and it also contradicts the README’s legacy command mapping.

Please keep the existing aliases. If you want to add print-config, that can be additive, but it should not replace print-plist.

  1. Remove the stray duplicate type

types.rs has both BucketPrepareSummary and BucketPrepareSummary_. The latter looks like a leftover from the split and should be removed.

  1. Re-run formatting/tests after the rebase

After rebasing onto current main, please run:

cargo fmt
cargo test

@Wangnov Wangnov closed this May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GGzili @Wangnov @gzili