Skip to content

Add actionable CLI startup diagnostics - #457

Open
tninja wants to merge 20 commits into
mainfrom
agent/startup-diagnostics
Open

Add actionable CLI startup diagnostics#457
tninja wants to merge 20 commits into
mainfrom
agent/startup-diagnostics

Conversation

@tninja

@tninja tninja commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

Make failed AI CLI startup diagnostics actionable while keeping sensitive terminal output, credentials, and forged control-character lines out of persistent message logs.

Root cause

  • The backend/session prefix was written to the terminal buffer only after successful startup, so the real failure path reported backend=unknown even though the caller already had the prefix.
  • For vterm, process-command describes the internal /bin/sh -c wrapper rather than the AI CLI launch command, so diagnostics reported the wrong executable and could expose secrets embedded in the wrapper string.
  • Exact option-name matching missed common credential forms such as --github-token, --api_key, access-key variants, and authorization headers.
  • Reusing the option suffix matcher for environment assignments regressed names with embedded secret components such as AWS_SECRET_ACCESS_KEY.
  • The inline name=value matcher required the value to fit on one line, so credentials containing newlines bypassed redaction.
  • Nested environment options such as --env=AWS_SECRET_ACCESS_KEY=value treated only the outer option as the name and exposed the inner value.
  • A launch command beginning with shell environment assignments treated the first assignment as the executable and could copy its value into *Messages*.
  • The terminal output tail was copied into message, which could persist credentials or private output in *Messages*.
  • Compact failure-message fields used raw string formatting, allowing embedded control characters to create forged message lines.

Changes

  • Pass the backend/session prefix and actual terminal launch command explicitly through the shared session-start failure path.
  • Derive diagnostic argv from the supplied launch command instead of the terminal process wrapper; keep the process object only for status and exit code.
  • Skip leading shell environment assignments when deriving the executable, then defensively redact the selected executable before formatting the summary.
  • Keep the echo-area message limited to backend, executable, working directory, process status, and exit status, with every field escaped onto one physical line.
  • Append the complete argv structure to the already displayed failed terminal buffer instead of the message log.
  • Safely escape command arguments and redact exact or provider-prefixed token, API key, access key, password, credential, authorization, secret, environment-variable, --header, -H, and compact -H... value forms.
  • Redact multiline inline credentials without matching their value, and inspect nested --env= and -e environment assignments before rendering them.
  • Fail closed with an unparseable-command placeholder if launch argv parsing fails.
  • Preserve process-table cleanup and failed-buffer display even when diagnostic text cannot be appended.
  • Keep Ghostel executable resolution in the final PR Resolve Ghostel executable paths before startup #456 architecture without a separate startup helper module, hardcoded CLI registry, or startup advice.
  • Add focused coverage for the real vterm wrapper boundary, credential variants, multiline and nested credentials, leading environment assignments, benign environment names, message privacy and control characters, cleanup, and best-effort buffer diagnostics.
  • Use temporary buffers and a shared rendering helper in startup diagnostic tests to match repository testing conventions.

Merge conflict resolution

  • Merge the latest main branch at 1367dca.
  • Keep the final Ghostel executable-resolution tests from main for the add/add conflict.
  • Remove the obsolete ai-code-backends-infra-startup.el helper and its side-panel activation coupling.
  • Restore the complete unit-test workflow instead of the temporary focused CI command.

Validation

  • Startup diagnostics tests: 12 passed.
  • Shared terminal infrastructure tests: 145 passed.
  • Full ERT suite: 1262 passed, 13 skipped, 0 unexpected (1275 total).
  • Adversarial startup diagnostics checks for multiline credentials, nested environment options, malformed commands, and control characters: passed.
  • Touched-file byte compilation with warnings treated as errors: passed.
  • Checkdoc, diff hygiene, English-only added source, secret scan, conflict-marker scan, and diagnostics comparison: clean.
  • GitHub Actions: all 4 checks passed on commit fefc2a2.

@tninja
tninja changed the base branch from agent/ghostel-executable-resolution to main July 26, 2026 01:10
@tninja
tninja marked this pull request as ready for review July 28, 2026 01:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a09492f524

ℹ️ 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".

Comment thread ai-code-backends-infra.el
Comment on lines +1495 to +1497
(if failure-buffer
(progn
(ai-code-backends-infra--handle-session-start-failure

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve terminal setup errors

When Ghostel is installed but lacks ghostel-exec, ai-code-backends-infra--start-ghostel-process deliberately raises an actionable version error after the Ghostel session buffer has been created. Because failure-buffer is therefore non-nil, this branch suppresses that error and reports a generic CLI startup failure with unknown process status instead, incorrectly directing the user to troubleshoot the CLI rather than upgrade Ghostel. Re-signal terminal configuration errors or safely retain their diagnostic reason.

Useful? React with 👍 / 👎.

Comment thread ai-code-backends-infra.el
Comment on lines +1156 to +1158
(resolved-program (if (string-prefix-p "~" program)
(expand-file-name program)
program))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resolve tilde paths against the selected session host

When a backend program is configured as ~/bin/codex and a prefix invocation selects a remote working directory, this expands ~ before ai-code-backends-infra--session-working-directory has chosen that directory. The resulting command therefore contains a path resolved for the source buffer—often a local home path or a different TRAMP host—which is then executed in the selected remote session and fails unless both homes happen to match. Defer expansion until the working directory is known or leave remote shell expansion intact.

Useful? React with 👍 / 👎.

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