Skip to content

fix(completion): install static script instead of eval at login - #7

Merged
kallioli merged 1 commit into
mainfrom
fix/static-completion-script
Apr 27, 2026
Merged

fix(completion): install static script instead of eval at login#7
kallioli merged 1 commit into
mainfrom
fix/static-completion-script

Conversation

@kallioli

Copy link
Copy Markdown
Contributor

Context

orca completion install bash (and zsh) used to append
eval "$(_ORCA_COMPLETE=bash_source orca)" to the rc file. Each shell
startup re-spawned orca (which auto-walks ~60 command modules at
import time) plus a bash --version subprocess from Click's
_check_version(), costing 1–3 s per login.

A real incident on 2026-04-27 saw hundreds of orca processes pile
up on a single SSH session
, all stuck inside _check_version()
waiting on their bash --version subprocess. The user-visible symptom
was a multi-thousand-line traceback after Ctrl-C.

Changes

  • orca_cli/core/shell_completion.pyinstall_completion_bashzsh
    now generates the completion script once into
    $XDG_DATA_HOME/orca/completion.<shell> (XDG-spec compliant) and
    rewrites the rc to source that file. No orca process is spawned
    at login; sourcing a static script is microseconds.
  • Migration: any pre-existing
    eval "$(_ORCA_COMPLETE=...)" line is detected and replaced on
    re-install, preserving surrounding rc content. Idempotent.
  • orca_cli/commands/completion.pyINSTRUCTIONS rewritten for
    the manual two-step flow (generate static file + source).
  • Lazy completion callbacks in orca_cli/core/completions.py are
    unchanged — tab completion still hits the API on demand with the
    per-profile cache and 5-minute TTL added in v2.0.1.
  • docs/adr/0010-static-completion-script.md — full context,
    decision, alternatives considered, migration instructions.

Tests

TestInstallCompletionBashZsh rewritten and extended (5 tests):

  • writes the static script and emits a source line
  • appends alongside existing rc content
  • idempotent re-install (no duplication)
  • migrates legacy eval to source line
  • fails gracefully when orca is not on PATH

pytest -q --cov=orca_cli --cov-fail-under=85: 2338 passed, coverage
88.42%. ruff + mypy clean.

Risks

  • Behavioural change for existing users: the rc no longer contains
    the eval line. Re-running orca completion install bash migrates
    silently; users who never re-run install will keep the slow eval
    until they do. Mitigation: documented in CHANGELOG and ADR 0010.
  • Two files installed instead of one rc line. Uninstall now needs
    to remove both. Documented in the install message.

Migration

orca completion install bash    # detects legacy eval and replaces it
exec \$SHELL                     # pick up the new sourced file

The bash/zsh completion install used to append
`eval "$(_ORCA_COMPLETE=bash_source orca)"` to the rc file. Each shell
startup re-spawned `orca` (which auto-walks ~60 command modules) plus a
`bash --version` subprocess from Click's `_check_version()`, costing
1-3 s per login on a developer laptop. A real incident on 2026-04-27
saw hundreds of `orca` processes pile up on a single SSH session, all
stuck inside `_check_version()` waiting on their `bash --version`
subprocess; the user-visible symptom was a multi-thousand-line
traceback after Ctrl-C.

`orca completion install bash` (and `zsh`) now generates the completion
script once into `\$XDG_DATA_HOME/orca/completion.<shell>` and writes a
plain `[ -f ... ] && source ...` line into the rc, matching what fish
has always done. Login cost drops to microseconds; no `orca` process
is spawned at login. Lazy completion callbacks in
`orca_cli/core/completions.py` are unchanged, so tab completion still
hits the API on demand with the same per-profile cache.

Re-running the install on an older config silently migrates the legacy
`eval` line out of the rc, preserving surrounding user content. New
tests in `TestInstallCompletionBashZsh` cover the rewrite, idempotency,
the legacy-eval migration, and the orca-not-on-PATH error path.

See ADR 0010 for the full context, alternatives considered, and
migration instructions for upgrading users.
@kallioli
kallioli force-pushed the fix/static-completion-script branch from af33541 to 2350e98 Compare April 27, 2026 10:11
@kallioli
kallioli merged commit bff4c6c into main Apr 27, 2026
10 checks passed
@kallioli
kallioli deleted the fix/static-completion-script branch April 27, 2026 10:14
Vinetos pushed a commit to Vinetos/orca-cli that referenced this pull request May 30, 2026
Addresses the remaining high/medium findings from the 2026-04-20 audit:
- stackopshq#4 insecure TLS warning + cacert path validation
- stackopshq#7 atomic token cache writes
- stackopshq#9 publish action pinned to immutable SHA
- #13 CI gaps: Poetry cache, poetry build, gitleaks, pip-audit,
  deploy-docs gated on CI success
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant