refactor: streamline application startup and IPC service management

[Seele-Official]

Summary by CodeRabbit

• Bug Fixes

  • Enhanced error handling for unsupported execution modes with clearer error messages
  • Improved working directory normalization and script path resolution

• Refactor

  • Reorganized application configuration and runtime planning for better code structure and maintainability

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactored application startup and IPC service management by extracting configuration logic and service implementation from main.cc into new app_config and app_runner modules, and redesigned the Session API to use structured ProcessLaunchPlan/RunPlan objects instead of template-based factory functions, centralizing acceptor setup and event loop management.

Changes

Cohort / File(s)	Summary
New Configuration Module src/catter/core/app_config.h, src/catter/core/app_config.cc	Introduced startup configuration structures (StartupConfig and RuntimePlan) and conversion/loading functions: to_startup_config() derives config from CLI options, build_runtime_plan() maps mode strings to runtime/IPC modes, and load_script_content() loads scripts from file or internal sources.
New Application Runner src/catter/core/app_runner.h, src/catter/core/app_runner.cc	Added run() entry point and ServiceImpl IPC service implementation that orchestrates startup config loading, JavaScript runtime initialization, script execution, and service dispatch (currently supporting INJECT mode); includes decision-making logic for action conversion and session launching.
Session API Refactoring src/catter/core/session.h, src/catter/core/session.cc	Replaced template-based run(shell, factory) with structured ProcessLaunchPlan and RunPlan types; added make_run_plan() static factory, moved acceptor creation and IPC pipe setup into the new run(RunPlan) method, and updated loop() signature to accept ClientAcceptor directly instead of function_ref.
Main Simplification src/catter/main.cc	Removed embedded ServiceImpl, configuration extraction, IPC injection logic, and mode dispatch; simplified main() to delegate to app::run(), reducing this file from 185 lines of control flow to simple option passing.
Integration Test Updates tests/integration/test/catter-proxy.cc	Updated test to use new Session API: replaced inline run(shell, factory) call with explicit ProcessLaunchPlan construction and make_run_plan() + run() invocation; made factory's call operator const.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• refactor: ipc #50 — Modifies IPC/session layer interfaces (DefaultService, accept functions) and Session's run/loop APIs in parallel with this PR's session refactoring.
• refactor: improved type safety between js and cpp #63 — Refactors JS↔C++ callback signatures (on_command, on_execution, Action/ExecutionEvent types) that directly interact with ServiceImpl's decision-making logic introduced here.
• feat: mvp of cdb generator #62 — Changes Session's run/loop API and on_finish invocation, overlapping with this PR's session redesign and runner integration.

Poem

A rabbit hops through configs neat,
Extracting services, making neat,
Plans and runtimes, structured well,
Session APIs now have a tale to tell! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.35% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: a significant refactoring that extracts application startup logic and IPC service management into dedicated modules.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/catter/core/app_config.cc`:
- Around line 58-76: load_script_content currently opens script_path verbatim,
so relative paths are resolved against the original process CWD instead of the
configured working_dir; update load_script_content to detect when script_path is
relative (e.g., does not start with '/' or a drive letter on Windows) and
join/resolve it against the configured working_dir (the same working_dir used to
set the JS runtime pwd) before checking internal_scripts or opening the file,
ensuring internal_scripts lookup still works for names like "script::cdb" and
file I/O uses the resolved path.

In `@src/catter/core/app_runner.cc`:
- Around line 126-145: The startup working directory from plan is only applied
to js::init_qjs but not to the service execution, so the proxy/build commands
still run in the original CWD; update run to ensure plan.working_dir is applied
when launching the service by either (A) changing the process CWD before calling
execute_service (e.g., chdir to plan.working_dir) or (B) passing
plan.working_dir into execute_service and having execute_service use it for
proxy/build command invocations; modify the run function and the execute_service
signature/usage (or the config passed into js::on_finish) so the service/proxy
inherits plan.working_dir rather than the original process working directory.
- Around line 90-108: The function inject uses config.buildSystemCommand.front()
without checking if the vector is empty, which can crash when js::on_start()
returns an empty command; update inject(const js::CatterConfig&) to first test
config.buildSystemCommand.empty() and handle that case (e.g., log an error via
your logging facility and return a sensible non-zero int64_t error code or
construct a safe fallback command) before constructing
Session::ProcessLaunchPlan; ensure references to buildSystemCommand.front() are
only used after the guard and update any related code paths
(Session::ProcessLaunchPlan construction, append_range_to_vector call, and
Session::make_run_plan usage) to operate correctly when the command vector was
empty.

In `@src/catter/core/session.cc`:
- Around line 34-44: When the process launch (spawn_task) fails we must ensure
the accept loop (loop_task) is torn down so accept() can unblock; change the
post-run handling so you first attempt to get spawn_task.result() inside a
try/catch, and on catch request cancellation/stop of loop_task (e.g. call the
loop_task cancel/request_stop/stop API or signal the acceptor) then run the
default_loop again to let loop_task unwind, rethrow the spawn exception,
otherwise (if spawn succeeded) call loop_task.result() to propagate loop
exceptions and return spawn_task.result(); reference loop_task, spawn_task,
loop(), spawn(), default_loop().run(), and
loop_task.result()/spawn_task.result() when making the changes.

In `@tests/integration/test/catter-proxy.cc`:
- Around line 111-114: The test currently ignores the session exit code and
always returns 0; update the end of the test to fail on non-zero session exit by
checking the return value from Session::make_run_plan / session.run (the
variable ret) and propagate or assert on it instead of unconditionally returning
0 — e.g. if ret != 0 then return ret (or call std::terminate/assert) so a
non-zero session.run() causes the test to fail.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a627633-3823-4c98-bb83-7d79cbfe17a5

📥 Commits

Reviewing files that changed from the base of the PR and between e4939b4 and 5adf0ab.

📒 Files selected for processing (8)

• src/catter/core/app_config.cc
• src/catter/core/app_config.h
• src/catter/core/app_runner.cc
• src/catter/core/app_runner.h
• src/catter/core/session.cc
• src/catter/core/session.h
• src/catter/main.cc
• tests/integration/test/catter-proxy.cc