feat(plugins): add connection, authentication, and importer providers

[ryan-wong-coder]

Summary

• add production connection Provider contracts and SDK APIs for validation, probing, opening, bidirectional terminal streams, resize, signals, reconnect, status, diagnostics, and close
• add host-rendered authentication Provider challenges with operation-bound one-use secret leases and host-owned credential references
• add bounded importer detection and streaming JSONL parsing with progress, exact public draft schemas, safe previews, deduplication, confirmation, and recoverable all-or-nothing Vault commits
• preserve opaque plugin connection configuration across session restore, export, sync payloads, missing-plugin states, split/copied sessions, workspace-created sessions, and copied new-window sessions without coercing them into SSH

Security and functional completeness

• connection Providers now register as operation-keyed SDK/runtime handler maps, so each operation receives its exact invocation type and must return its exact result type; resize, signal, reconnect, and close acknowledge with JSON null
• Provider adapters validate the exact public result for every connection operation: validation/probe/open/status use their declared result shapes, while control operations reject objects or any non-null result
• runtime connection diagnostics are accepted on status polling, bounded through the shared Provider issue schema, propagated through terminal lifecycle exit events, and displayed with plugin close output even when the Provider omits an error string
• importer drafts are part of the public contract and SDK surface: host, identity, key, snippet, and group records have exact schemas, limits, required fields, enums, and documented sensitive-field handling
• importer hosts cannot persist executable startup commands or plaintext built-in credentials; plugin hosts must use a bounded plugin:<providerId> protocol with opaque provider-owned configuration
• connection configuration, authentication challenges, importer selection/parsing, and Vault commit workflows are now owned by application hooks; UI components are presentational glue around those hooks
• the host owns terminal session IDs, renderer ownership, cancellation, flow control, connection logging, lifecycle cleanup, status transitions, and startup-command readiness
• startup commands for plugin terminals run only after a host-owned pluginConnectionReady marker emitted from Provider status polling, so banner/progress output cannot corrupt an in-progress handshake
• passwords, OTPs, private keys, and passphrases remain behind the existing safeStorage, CredentialRef, and SecretLease boundary; authentication challenge plaintext is never returned directly to a plugin runtime
• authentication requests are cancellable before Provider open: the host installs the terminal session cancellation boundary before challenge IPC, and terminal close aborts the challenge/open path
• plugin hosts cannot retain hidden first-party credentials or SSH, Telnet, Mosh, Eternal Terminal, serial, proxy, chain, or SSH tuning state; a shared domain sanitizer enforces this boundary for UI saves, imports, sync, and restored data
• importer drafts are whitelist-constructed, validate supported fields before Vault normalization, reject malformed supplied protocols and enum/string fields fail closed, and discard arbitrary extra fields
• connection and importer streams use bounded windows, deadlines, byte and record limits, exact runtime identity checks, deterministic cancellation, and deterministic cleanup
• terminal input for plugin connections is serialized into negotiated stream-window chunks before entering the Provider stream, so large pastes cannot overflow queue credit and disconnect a session
• importer previews exclude passwords, private keys, passphrases, startup commands, and snippet commands; final persistence validates, sanitizes, deduplicates, encrypts sensitive Vault data, detects concurrent edits, and commits through a recovery journal
• renderer-side Provider configuration validation matches JSON Schema/AJV string length semantics by counting Unicode code points
• plugin protocol hosts keep their provider selected by default even when group configuration enables optional built-in protocols such as Telnet
• explicit JSON null Provider configuration is preserved end-to-end and importer detection is cancellable before Provider work begins
• plugin terminal output is decoded incrementally as UTF-8, including split multibyte sequences; silent connected sessions preserve a zero-byte readiness marker through worker, port, backlog, and preload paths
• Provider status monitoring publishes readiness once but continues polling after connected, including already-connected opens, until terminal closure, Provider error/closed status, abort, or disposal
• Provider stream failures finish the terminal as errors with bounded messages instead of clean disconnects
• optional plugin event subscriptions degrade to inert no-op disposers when the preload bridge is unavailable
• Vault credentials are selectable by plugin connections only after their encrypted entries are accepted by the trusted host credential catalog; missing or failed catalog updates clear the renderer snapshot fail closed
• plugin-backed terminal output participates in host-owned automatic session logging through a worker-local token-bound stream; host log paths remain outside the public Provider payload
• SDK provider registration overloads infer operation-specific connection, authentication, and importer invocations before the broad non-interceptor fallback, so streaming providers are implementable without casts
• all plugin surfaces remain behind the development gate; the unreleased plugin database stays at complete schema user_version = 1 with no migration chain

Review follow-up

This head addresses the latest Codex review gaps on exact head 874fb255cce217fb1f21655de45d2f435908a596:

• importer parsing now enforces the per-record JSONL byte limit while an unterminated line is still pending, so malformed output is rejected at the 256 KiB record cap instead of growing toward the full import cap
• authentication challenge response failures now retain the active dialog with a bounded localized error, leaving the user able to retry or cancel instead of leaving the main-process request pending without UI
• Host Details provider choices now subscribe to plugin contribution changes and refetch connection/authentication providers with stale-request suppression, so install, enable, disable, and uninstall changes are reflected without remounting the panel

This PR also addresses the prior Codex review gaps on exact head 60368b014776876ffdf623b8a7b819fcb5ff459b:

• connection Provider registration now uses an operation-keyed SDK/runtime handler map, so TypeScript rejects wrong-operation results such as returning a probe result from resize; runtime dispatch validates the same shape and rejects the legacy single-callback form for connection Providers
• plugin connection exit output now includes bounded status diagnostics when a Provider closes or errors without an explicit error string, preserving actionable Provider failure details in the terminal display

This head also addresses the prior Codex review gap on exact head f78ff1250c63c5d9bab3d63f8e05b6675dec259e:

• plugin connection attachments now require the host-owned pluginConnectionReady marker before transitioning a still-connecting Provider session to connected, while preserving first-output connected behavior for built-in transports

This head also addresses the prior Codex review gap on exact head b9fc1dfb7f8ea6600b49f1c0de401d0e8a218b8b:

• terminal popup routes now mount PluginAuthenticationHost inside the popup I18nProvider, so plugin authentication Provider challenges sent to a copied new-window renderer have a listener/dialog path and no longer wait until cancellation or timeout

This PR also addresses the latest maintainer review gaps on exact head 53b47eec51bf5aff547d7e4ae6763325c51c6d56:

• connection control operations now reject any non-null result, and the SDK exposes operation-specific connection result mapping
• importer draft shapes are explicit in the canonical schema, generated TypeScript contract, Electron schema bundle, SDK typing, tests, and docs
• plugin connection configuration, authentication challenge, importer parse/cancel, and Vault commit lifecycles moved from components into application hooks
• status polling diagnostics are now part of ConnectionStatusResult and flow to terminal exit diagnostics for asynchronous Provider errors
• docs/plugin-platform/* now describes the implemented PR 7 connection/authentication/importer behavior and the remaining PR 8/9 boundaries

Prior corrective rounds also covered closed-session reuse, imported protocol validation, streaming UTF-8 output, public SecretLease shape, silent connected readiness, importer cancellation and completion bounds, zero-delay status polling, isolated plugin snapshots, hidden built-in credential stripping, explicit JSON null configuration, secure credential catalogs, auto-save logging, SDK overloads, readiness-gated startup commands, renderer cancellation, status monitoring, input chunking, and plugin-session command-history suppression.

Downstream audit

• PR 8 can add sync Providers without changing RPC names, permission names, scopes, lifetimes, runtime identity, cancellation, or stream framing; current sync payload preservation keeps opaque plugin connection fields intact while the shared sanitizer removes unrelated built-in transport state
• PR 9 can layer signed distribution and publisher trust onto the existing immutable plugin identity, advanced-runtime, permission, companion, credential, operation-keyed Provider, exact importer draft, and diagnostic boundaries
• connection, authentication, and importer Provider APIs remain stable for downstream plugins after the intentional pre-release SDK tightening: connection handlers are now operation-keyed to prevent compile-time data loss, while public operation names and result schemas remain the contract source of truth
• provider lazy activation, host-owned terminal lifecycle, cancellation, status polling, stream framing, JSON Schema validation, diagnostics, and credential indirection remain aligned across contract, generated schema, SDK, preload, renderer bridge, and runtime dispatch

Validation

• current machine-review regressions: 27/27 passed
• npm run test:plugin-runtime: 403 passed, 1 intentional Electron skip
• npm run test:plugin-runtime:electron: passed
• npm run test:plugin-contract: 77/77 passed
• npm run lint -- --quiet: passed
• npm test: 7,368 total, 7,352 passed, 13 skipped, 3 unrelated upstream/environment failures:
  • application/state/sftpTransferCenterStore.test.ts: orphan directory pause rolls back successful child pauses on hard fail
  • electron/bridges/sshBridge.authRetryExit.test.cjs: failed keyboard-interactive retry still offers encrypted default key fallback
  • electron/bridges/transferBridge.test.cjs: pause soft-drains concurrent ranges but resume waits before truncating
• the affected SFTP/SSH/transfer files are byte-identical to current upstream/main and outside this plugin PR's changed surface
• npm run check:plugin-contract: passed
• npm run build: passed
• npm run pack:dir: passed
• git diff --check: passed
• Codex exact-head review for f7b5f9b8529cb8ad66146fd8e48c202e068d6302: no major issues
• remote CI for exact head f7b5f9b8529cb8ad66146fd8e48c202e068d6302: lint-and-test failed only in two unchanged upstream/environment SFTP and transfer timing assertions (application/state/sftpTransferCenterStore.test.ts and electron/bridges/transferBridge.test.cjs), while all four platform package builds passed; maintainer rerun of the failed test job is requested because this account cannot rerun it (Must have admin rights to Repository)

Rebased/synced with upstream/main at 5e5c98a6828e10092bec465c929ed268c6f20ba9; upstream/main was 0 commits ahead at the corrective push, and this branch was 18 commits ahead.

Related to #2269.

[ryan-wong-coder]

Independent review of exact head fede51ea03eb7fb1141eb5539dc2062914c4a2d9 completed.

No blocking findings remain.

Reviewed areas:

• contract/schema/generated SDK alignment for connection, authentication, and importer Providers
• exact plugin/runtime/security-principal ownership and permission boundaries
• host-owned terminal session identity, renderer ownership, cancellation, flow control, status transitions, and cleanup
• operation-bound SecretLease and host-owned CredentialRef handling, including cross-plugin authentication restrictions
• bounded connection/importer streams, deadlines, record/byte budgets, progress routing, and failure cleanup
• importer normalization, safe preview redaction, deduplication, concurrent-write protection, encrypted persistence, and recovery-journal transaction behavior
• session restore, missing-plugin preservation, export, and PR 8 sync-payload compatibility
• PR 9 signed-distribution extensibility through the existing immutable identity, advanced-runtime, permission, companion, and credential seams
• development-gate coverage and complete unreleased schema v1 policy

Functional completeness assessment: this head provides the stable public and host integration seams required for the remaining PR 8–9 work without changing RPC method names, permission names/scopes/lifetimes, runtime identity, cancellation, or stream framing. Publisher verification remains correctly scoped to PR 9.

Validation results are recorded in the PR body. The sole full-suite failure is the unchanged upstream SSH keyboard-interactive retry test; both the implementation and failing test are byte-identical to upstream/main.

[ryan-wong-coder]

@codex review\n\nPlease review exact head fede51ea03eb7fb1141eb5539dc2062914c4a2d9.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fede51ea03

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

[github-actions]

@codex review

[ryan-wong-coder]

Independent review of exact head c3ef41e2f985c20d328b6f239e0cdb8b73f362f3

Result: no additional actionable correctness, security, compatibility, accessibility, localization, or lifecycle findings.

Reviewed areas:

• all seven Codex findings are addressed with regression coverage: closed-session reuse, imported protocol validation, streaming UTF-8 output, public SecretLease shape, silent connected readiness, importer cancellation, and bounded importer completion
• the silent-connection path is complete across the Provider bridge, terminal worker manager, worker IPC, output ports, preload backlog, and late listener replay; the zero-ingress marker is retained without being treated as positive flow credit
• importer close and replacement paths invalidate late async results, cancel active parsing, release unconsumed selection tokens, and avoid stale UI repopulation
• companion credential references now match the generated public contract and retain operation-bound lease handling
• the change does not alter public RPC method names, permission names/scopes/lifetimes, runtime identity, provider cancellation semantics, or stream framing
• PR 8 sync Providers can reuse the current Provider activation, deadline, cancellation, opaque configuration, and secret reference seams; PR 9 trust and distribution can layer on the existing immutable identity and permission boundaries without contract changes

Validation on this head:

• focused follow-up regressions: 174/174 passed
• plugin runtime: 392 passed, 1 intentional Electron skip
• Electron runtime smoke: passed
• plugin contract: 76/76 passed
• lint, contract drift, build, pack, and diff checks: passed
• full suite: 7,032 passed, 11 skipped, with only the unchanged upstream SSH keyboard-interactive retry failure; the related implementation and test are byte-identical to upstream/main

The implementation is functionally complete for PR 7 scope and provides the required stable seams for PRs 8 and 9.

[ryan-wong-coder]

Independent review of exact head ac89909aaeec2fddf0349b6759628abebbfec86e

Result: no actionable findings.

This follow-up fixes the CI-only cancellation observed on c3ef41e2. The silent-connection regression uses the explicit internal connectionStatusPollMs: 0 configuration. The zero-delay timer previously inherited the production unref() behavior, allowing the isolated Node test process to exit before the immediate callback ran. The new boundary retains only zero-delay polls until their callback executes; every positive production poll remains unrefed, so Electron shutdown and long-lived monitoring behavior are unchanged.

Review conclusions:

• no public contract, RPC, permission, runtime identity, cancellation, or stream framing changed
• no production polling interval or connection-state semantics changed
• the status-monitor regression passed ten consecutive isolated runs and the 167-test affected CJS/UI set completed without cancellation
• plugin runtime: 392 passed, 1 intentional Electron skip
• Electron smoke and 76/76 contract tests passed
• lint, contract drift, build, pack, and diff checks passed
• full suite completed normally with 7,032 passed and 11 skipped; the sole failure remains the unchanged upstream SSH keyboard-interactive retry test
• PR 8 and PR 9 downstream seams remain unchanged and usable

The head remains functionally complete for PR 7.

[github-actions]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac89909aae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[github-actions]

@codex review

[ryan-wong-coder]

Independent review of exact head 55a6105352a84a3d295ff98e32d504e37127fb9b

Result: no additional actionable correctness, security, compatibility, accessibility, localization, lifecycle, or downstream-delivery findings.

Reviewed areas:

• split and copied plugin sessions preserve independent structured snapshots of pluginConnection without becoming eligible for SSH connection reuse
• plugin host persistence removes hidden first-party credentials, identities, proxy/chain configuration, SSH/Telnet/Mosh/Eternal Terminal/serial transport state, and SSH tuning fields while retaining the validated opaque plugin connection
• the cleanup boundary is shared by general host sanitization and enforced again at the end of the Host Details save path, preventing later UI steps from restoring stale built-in identity state
• importer, restored-data, export, and sync paths retain opaque provider-owned configuration while passing through host-owned validation and sanitation
• public RPC method names, permission names/scopes/lifetimes, runtime identity, provider cancellation semantics, and stream framing remain unchanged
• PR 8 sync Providers can reuse the existing activation, deadline, cancellation, opaque configuration, and secret-reference seams; PR 9 distribution and publisher trust can layer onto the immutable identity and permission boundaries without contract changes
• the complete unreleased plugin database remains at schema user_version = 1, and all plugin surfaces remain behind the development gate

Validation:

• focused clone, host-boundary, and importer regressions: 88/88 passed
• plugin runtime: 392 passed, 1 intentional Electron skip
• Electron runtime smoke: passed
• plugin contract: 76/76 passed
• lint, contract drift, build, pack, and diff checks: passed
• full suite: 7,036 passed, 11 skipped, with only the unchanged upstream SSH keyboard-interactive retry failure; the related implementation and test are byte-identical to upstream/main

This head remains functionally complete for PR 7 and preserves the required stable seams for PRs 8 and 9.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55a6105352

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[binaricat]

@codex review

[ryan-wong-coder]

Independent review of exact head 2a1478e8b95d4e437056c6048dfe3077ab3d656b

Result: no additional actionable correctness, security, compatibility, accessibility, localization, lifecycle, or downstream-delivery findings.

Reviewed areas:

• explicit JSON null Provider configuration is preserved through domain sanitation, persisted host state, the host configuration editor, connection open, and authentication begin; default {} applies only to an actually absent field
• connection and authentication configuration still pass the same bounded JSON and declared-schema validation before plugin code runs
• importer detection now owns a cancellable request identity before awaiting Provider work; dialog close cancels the exact detection request, invalidates late UI work, and releases an unconsumed host-owned file selection in finally
• detection and parsing reuse the request identity only sequentially, after the completed detection request has released host-side ownership, so cancellation correlation and request-map bounds remain intact
• no public RPC method names, permission names/scopes/lifetimes, runtime identity, cancellation protocol, stream framing, or database schema changed
• PR 8 sync Providers can continue to reuse activation, deadlines, cancellation, opaque configuration, and secret-reference seams; PR 9 distribution and publisher trust can layer onto the existing immutable identity and permission boundaries without contract changes
• the complete unreleased database remains at schema user_version = 1, and all plugin surfaces remain behind the development gate

Validation on the rebased head:

• focused Provider request and lifecycle regressions: 23/23 passed
• plugin runtime: 393 passed, 1 intentional Electron skip
• Electron runtime smoke: passed
• plugin contract: 76/76 passed
• lint, contract drift, build, pack, and diff checks: passed
• full suite: 7,098 passed and 11 skipped; the two failures are the unchanged upstream SSH retry failure and an intermittent terminal flow-control assertion whose implementation and test are byte-identical to upstream/main; the terminal attachment suite passed five consecutive isolated reruns

This head is functionally complete for PR 7 scope and preserves the stable seams required for PRs 8 and 9.

[ryan-wong-coder]

Maintainer rerun requested for the failed lint-and-test job in Actions run 30093959229.

The sole CI failure is the existing intermittent Telnet assertion startTelnetSession answers login prompts with saved credentials at electron/bridges/terminalBridge.telnetAutoLogin.test.cjs:98 (false == true). This PR does not modify the Telnet implementation or test: both electron/bridges/terminalBridge.cjs and electron/bridges/terminalBridge.telnetAutoLogin.test.cjs are byte-identical to current upstream/main. The isolated Telnet file passed five consecutive local reruns on exact head 2a1478e8b95d4e437056c6048dfe3077ab3d656b.

All PR-specific runtime, contract, lint, build, pack, and diff gates pass. This account cannot rerun the failed job because GitHub requires repository admin permission. No unrelated Telnet change or empty commit will be made to churn the reviewed SHA.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a1478e8b9

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

[ryan-wong-coder]

Independent exact-head review for 21e5fedb1890ecc9f9119cf584b4fc3c62ecd247

No additional actionable findings.

I re-reviewed both corrective paths beyond the reported cases:

• optional importer and authentication subscriptions now remain inert and disposable when the preload bridge or individual event method is absent;
• the renderer publishes only a frozen snapshot of credential IDs accepted in full by the trusted host catalog;
• partial, missing, or failed catalog updates clear that snapshot fail closed;
• plugin connection options require both secure-catalog membership and a valid bounded local Vault value;
• an existing unresolved credential reference remains visible only as unavailable and cannot be selected as a usable credential.

The downstream audit found no changes to public RPC method names, permissions, scopes, lifetimes, runtime identity, cancellation, or stream framing. PR 8 retains its encrypted sync sidecar, namespaced storage, and stream seams; PR 9 retains the immutable identity and trust boundaries.

Validation:

• focused secure bridge and credential-catalog regressions: 7/7 passed
• plugin runtime: 393 passed, 1 intentional Electron skip
• Electron runtime smoke: passed
• plugin contract: 76/76 passed
• lint, contract drift, build, directory packaging, and diff checks: passed
• full test suite: 7,127 total, 7,114 passed, 11 skipped, with only the unchanged upstream SSH retry failure and one intermittent terminal-session timing assertion; the terminal-session starter file passed 62/62 on isolated rerun

[binaricat]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21e5fedb18

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

[ryan-wong-coder]

Independent follow-up review of exact head 7ed9b06940338ea7db39fb2a462679a49a1cd121 is complete.

No additional correctness, security, compatibility, accessibility/localization, functional-completeness, or downstream PR 8-9 blockers were found.

The auto-save fix keeps session-log ownership entirely on the host path: log configuration and host labels are forwarded only through the internal renderer/main/worker route, never through the public Provider payload. Each plugin terminal owns a token-bound worker-local stream; validated decoded output is appended before renderer delivery, while Provider finish and user close stop only the matching stream instance. Manual session logging remains on the existing main-process output-tap path, so the two modes do not duplicate output.

Public Provider RPC names, permission names/scopes/lifetimes, runtime identity, cancellation, stream framing, and opaque plugin connection persistence are unchanged. PR 8 encrypted sync Providers and PR 9 distribution/trust can continue on the existing seams without contract changes.

Validation: focused logging/bridge regressions 166/166; plugin runtime 393 passed with one intentional Electron skip; Electron smoke passed; contract 76/76; lint, contract drift, build, pack:dir, and diff check passed. Full npm test produced 7,136 passed and 11 skipped with only the unchanged upstream SSH keyboard-interactive retry failure; the affected source and test are byte-identical to upstream/main.

[binaricat]

@codex review

[ryan-wong-coder]

Maintainer rerun requested for the failed lint-and-test job in Actions run 30099443161 on exact head 7ed9b06940338ea7db39fb2a462679a49a1cd121.

The sole CI failure is the existing intermittent Telnet assertion startTelnetSession answers login prompts with saved credentials at electron/bridges/terminalBridge.telnetAutoLogin.test.cjs:98 (false == true). This PR does not modify the Telnet implementation or test: both electron/bridges/terminalBridge.cjs and electron/bridges/terminalBridge.telnetAutoLogin.test.cjs are byte-identical to current upstream/main. The isolated Telnet file passed five consecutive local reruns on this exact head, and the prior full local suite also passed this assertion.

All PR-specific logging/runtime, contract, lint, build, pack, and diff gates pass. This account cannot rerun the failed job because GitHub requires repository admin permission. No unrelated Telnet change or empty commit will be made to churn the reviewed SHA.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7ed9b06940

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

[binaricat]

@codex review

[binaricat]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9fc1dfb7f

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

[ryan-wong-coder]

Maintainer follow-up and independent exact-head review for b9fc1dfb7f8ea6600b49f1c0de401d0e8a218b8b.

This addresses the five review gaps raised in #2413 (comment):

1. Connection control results now validate the exact public operation result: resize, signal, reconnect, and close must return JSON null; non-null objects are rejected, and the SDK exposes operation-specific result mapping.
2. Importer drafts are now explicit public contract and SDK shapes for host, identity, key, snippet, and group records, with exact schema limits, required fields, enums, and sensitive-field handling. Executable startup commands and plaintext built-in credentials are rejected or stripped before persistence.
3. Plugin connection configuration, authentication challenge, importer parse/cancel, and Vault commit lifecycles moved into application hooks; the UI components are now presentational glue around those hooks.
4. Runtime connection diagnostics are part of ConnectionStatusResult and propagate through terminal exit diagnostics for asynchronous Provider errors.
5. docs/plugin-platform/* now documents the implemented PR 7 connection/authentication/importer behavior and keeps PR 8/9 boundaries explicit.

Independent review result: no additional actionable correctness, security, compatibility, accessibility/localization, functional-completeness, lifecycle, or downstream PR 8-9 blockers were found after the corrective changes.

Downstream audit:

• PR 8 sync Providers can reuse the current activation, deadline, cancellation, opaque configuration, diagnostics, credential-reference, and stream-framing seams without changing RPC names, permission names/scopes/lifetimes, runtime identity, or storage boundaries.
• PR 9 distribution and publisher trust can layer onto the existing immutable plugin identity, advanced-runtime, permission, companion, credential, Provider, importer-draft, and diagnostic boundaries without contract churn.
• The complete unreleased plugin database remains at user_version = 1; no migration chain was added.

Validation on this head:

• targeted review regressions: 156/156 passed
• npm run test:plugin-runtime: 401 passed, 1 intentional Electron skip
• npm run test:plugin-runtime:electron: passed
• npm run test:plugin-contract: 77/77 passed
• npm run lint -- --quiet: passed
• npm run check:plugin-contract: passed
• npm run build: passed
• npm run pack:dir: passed
• git diff --check: passed
• npm test: 7,363 total, 7,347 passed, 13 skipped, with three unrelated upstream/environment failures in SFTP transfer-center, SSH auth-retry, and transfer bridge timing tests; the affected files are byte-identical to current upstream/main and outside this plugin PR's changed surface

The PR description has been refreshed with the current Markdown and validation text. Remote CI is still being tracked separately: current completed failures are unrelated to this corrective commit (lint-and-test failed only in unchanged SFTP/transfer timing tests, and build-linux-x64 failed before checkout while pulling almalinux:8 from Docker Hub). I will not churn the reviewed SHA with an unrelated empty commit; I will rerun failed jobs when the workflow finishes if permissions allow, or request a maintainer rerun with the exact evidence.

[ryan-wong-coder]

Maintainer rerun requested for the failed jobs on exact head b9fc1dfb7f8ea6600b49f1c0de401d0e8a218b8b.

This account cannot rerun the failed jobs:

• gh run rerun 30166234436 --failed returned Must have admin rights to Repository.
• gh run rerun 30166234444 --failed returned Must have admin rights to Repository.

I will not create an empty commit or churn the reviewed SHA to retrigger CI.

Current non-code CI failures:

• lint-and-test in Actions run 30166234436 failed only in unchanged upstream transfer tests:
  • application/state/sftpTransferCenterStore.test.ts: orphan directory pause rolls back successful child pauses on hard fail
  • electron/bridges/transferBridge.test.cjs: pause soft-drains concurrent ranges but resume waits before truncating
• build-linux-x64 in Actions run 30166234444 failed before checkout while initializing the almalinux:8 container from Docker Hub: Client.Timeout exceeded while awaiting headers / context deadline exceeded / Docker pull failed with exit code 1.

The affected SFTP/transfer bridge files are byte-identical to current upstream/main on this PR head and outside this plugin PR's changed surface. Local PR-specific validation remains green and is recorded in the PR body and in the exact-head review comment above.

Please rerun the failed jobs when convenient. In the build workflow, macOS, Windows, and Linux arm64 have passed; only the pre-checkout Docker pull failure on Linux x64 remains red.

[binaricat]

@codex review

[ryan-wong-coder]

Independent follow-up review of exact head f78ff1250c63c5d9bab3d63f8e05b6675dec259e.

Result: no additional actionable correctness, security, compatibility, accessibility/localization, functional-completeness, lifecycle, or downstream PR 8-9 blockers were found after the corrective change.

This head fixes the current Codex P2 finding by mounting PluginAuthenticationHost inside the #/terminal-popup page provider tree. Copy-to-new-window plugin connections now have the same renderer-local authentication challenge listener/dialog path as the main window, so authentication Provider challenges cannot stall in popup windows until cancellation or timeout.

Regression coverage was added in components/TerminalPopupPage.test.ts to pin the popup provider tree to include the authentication host under the popup I18nProvider.

Downstream audit:

• connection, authentication, and importer Provider APIs remain stable; no public RPC method names, permission names/scopes/lifetimes, runtime identity, cancellation protocol, stream framing, or database schema changed;
• PR 8 sync Providers can continue to reuse the existing activation, opaque plugin connection configuration, credential-reference, diagnostics, cancellation, and stream seams;
• PR 9 distribution and publisher trust can still layer onto the existing immutable identity, advanced-runtime, permission, companion, credential, Provider, importer-draft, and diagnostic boundaries;
• the complete unreleased plugin database remains at user_version = 1, with no migration chain.

Validation on this head:

• targeted popup/provider-tree regressions: 13/13 passed
• npm run test:plugin-runtime: 401 passed, 1 intentional Electron skip
• npm run test:plugin-runtime:electron: passed
• npm run test:plugin-contract: 77/77 passed
• npm run lint -- --quiet: passed
• npm run check:plugin-contract: passed
• npm run build: passed
• npm run pack:dir: passed
• git diff --check: passed
• npm test: 7,364 total, 7,348 passed, 13 skipped, with three unrelated upstream/environment failures in SFTP transfer-center, SSH auth-retry, and transfer bridge timing tests; the affected files are byte-identical to current upstream/main and outside this plugin PR's changed surface

Remote CI for this exact head is running. A single automated @codex review request for this SHA already exists, so I will not duplicate it.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[ryan-wong-coder]

Maintainer rerun request for exact head f78ff1250c63c5d9bab3d63f8e05b6675dec259e.

The corrective change for the latest Codex finding is in place and the review thread is resolved. The only completed failing check is lint-and-test in https://github.com/binaricat/Netcatty/actions/runs/30167149502/job/89702010644.

The failed assertions are outside this plugin PR's changed surface:

• application/state/sftpTransferCenterStore.test.ts: orphan directory pause rolls back successful child pauses on hard fail
• electron/bridges/transferBridge.test.cjs: pause soft-drains concurrent ranges but resume waits before truncating

The affected files are byte-identical to current upstream/main:

• application/state/sftpTransferCenterStore.test.ts
• electron/bridges/transferBridge.cjs
• electron/bridges/transferBridge.test.cjs

Local validation for the plugin change passed: targeted popup/provider-tree regressions 13/13, npm run test:plugin-runtime 401 passed with one intentional Electron skip, Electron runtime smoke, contract 77/77, lint, contract drift, build, pack, and diff check. Full npm test only reproduced unrelated upstream/environment failures in SFTP/SSH/transfer tests whose affected files are byte-identical to upstream/main.

I attempted to rerun the failed job with gh run rerun 30167149502 --failed, but this account still lacks the required repository admin permission (Must have admin rights to Repository). Please rerun the failed lint-and-test job when available. I will not churn the reviewed SHA with an empty commit.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f78ff1250c

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

[binaricat]

@codex review

[ryan-wong-coder]

Independent follow-up review of exact head 60368b014776876ffdf623b8a7b819fcb5ff459b.

Result: no additional actionable correctness, security, compatibility, accessibility/localization, functional-completeness, lifecycle, or downstream PR 8-9 blockers were found after the corrective change.

This head fixes the current Codex P2 finding by requiring plugin connection attachments to wait for the host-owned pluginConnectionReady marker before transitioning a still-connecting Provider session to connected. Ordinary Provider banner or handshake output is still displayed and logged, but it no longer dismisses the connecting UI or enables connected-state downstream behavior early. Built-in transports keep their existing first-output connected behavior because only the plugin connection start path opts into the explicit readiness gate.

Regression coverage now asserts that a plugin Provider returning status: "connecting" remains in connecting state after pre-readiness output, then transitions exactly once when the host readiness marker arrives and only then schedules startup commands.

Downstream audit:

• connection, authentication, and importer Provider APIs remain stable; no public RPC method names, permission names/scopes/lifetimes, runtime identity, cancellation protocol, stream framing, or database schema changed;
• PR 8 sync Providers can continue to reuse the existing activation, opaque plugin connection configuration, credential-reference, diagnostics, cancellation, readiness, and stream seams;
• PR 9 distribution and publisher trust can still layer onto the existing immutable identity, advanced-runtime, permission, companion, credential, Provider, importer-draft, and diagnostic boundaries;
• the complete unreleased plugin database remains at user_version = 1, with no migration chain.

Validation on this head:

• targeted terminal attachment/startup regressions: 170/170 passed
• npm run test:plugin-runtime: 401 passed, 1 intentional Electron skip
• npm run test:plugin-runtime:electron: passed
• npm run test:plugin-contract: 77/77 passed
• npm run lint -- --quiet: passed
• npm run check:plugin-contract: passed
• npm run build: passed
• npm run pack:dir: passed
• git diff --check: passed
• npm test: 7,364 total, 7,348 passed, 13 skipped, with three unrelated upstream/environment failures in SFTP transfer-center, SSH auth-retry, and transfer bridge timing tests; the affected files are byte-identical to current upstream/main and outside this plugin PR's changed surface

Remote CI for this exact head is running. A single automated @codex review request for this SHA already exists, so I will not duplicate it.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60368b0147

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

[ryan-wong-coder]

Maintainer rerun/restart requested for exact head 60368b014776876ffdf623b8a7b819fcb5ff459b.

I attempted to rerun the failed test job with:

gh run rerun 30167643701 --failed

GitHub rejected it with: run 30167643701 cannot be rerun; Must have admin rights to Repository.

The failed lint-and-test job is https://github.com/binaricat/Netcatty/actions/runs/30167643701/job/89703290459. Its only failing tests are:

• application/state/sftpTransferCenterStore.test.ts: orphan directory pause rolls back successful child pauses on hard fail
• electron/bridges/transferBridge.test.cjs: pause soft-drains concurrent ranges but resume waits before truncating

The affected SFTP/transfer files are byte-identical to current upstream/main and outside this plugin PR's changed surface:

• application/state/sftpTransferCenterStore.test.ts
• application/state/sftp/useSftpTransfers.ts
• electron/bridges/transferBridge.cjs
• electron/bridges/transferBridge.test.cjs

The build run https://github.com/binaricat/Netcatty/actions/runs/30167643716 also appears stuck: build-linux-arm64, build-macos, build-windows, and build-linux-x64 have remained in_progress since 2026-07-25T17:31Z.

Local PR-specific validation remains green on this exact head: targeted terminal attachment/startup regressions, plugin runtime, Electron plugin-runtime smoke, plugin contract, lint, contract drift, build, pack:dir, and diff check all passed. The full local suite only has unrelated upstream/environment failures in SFTP/SSH/transfer paths; the affected files are byte-identical to upstream/main.

I will not modify unrelated SFTP/transfer code or create an empty commit just to churn the reviewed SHA.

[binaricat]

@codex review

[ryan-wong-coder]

Independent follow-up review of exact head 874fb255cce217fb1f21655de45d2f435908a596.

Result: no additional actionable correctness, security, compatibility, accessibility/localization, functional-completeness, lifecycle, or downstream PR 8-9 blockers were found after the corrective change.

This head fixes the two latest Codex P2 findings from exact head 60368b014776876ffdf623b8a7b819fcb5ff459b:

• Connection Providers now register as an operation-keyed SDK/runtime handler map. The SDK no longer allows the broad fallback overload to catch connection Providers, ConnectionProviderHandler ties every operation to its exact invocation and result type, and regressions prove resize cannot return a probe result or use the previous single-callback shape. Runtime registration validates the same operation map and dispatches provider.invoke through the matching operation handler.
• Plugin connection exit output now includes bounded status diagnostics when a Provider closes/errors without an explicit error string. The terminal attachment callback type carries the full TerminalSessionExitEvent, and the regression verifies diagnostic lines are written for a diagnostics-only error exit.

Downstream audit:

• PR 8 sync Providers can continue to reuse the existing Provider activation, opaque plugin connection configuration, credential-reference, diagnostics, cancellation, status polling, and stream seams without changing RPC method names, permission names/scopes/lifetimes, runtime identity, cancellation, or stream framing.
• PR 9 signed distribution and publisher trust can still layer onto the existing immutable identity, advanced-runtime, permission, companion, credential, operation-keyed Provider, exact importer draft, and diagnostic boundaries.
• The connection Provider SDK tightening is intentional and pre-release: public operation names and result schemas remain the contract source of truth, but plugin implementations now fail at compile time instead of compiling a wrong-operation result that the host would reject at runtime.
• The complete unreleased plugin database remains at user_version = 1, with no migration chain.

Validation on this head:

• focused review/completeness regressions: 81/81 passed
• npm run test:plugin-runtime: 402 passed, 1 intentional Electron skip
• npm run test:plugin-runtime:electron: passed
• npm run test:plugin-contract: 77/77 passed
• npm run lint -- --quiet: passed
• npm run check:plugin-contract: passed
• npm run build: passed
• npm run pack:dir: passed
• git diff --check: passed
• npm test: 7,366 total, 7,350 passed, 13 skipped, with three unrelated upstream/environment failures in SFTP transfer-center, SSH auth-retry, and transfer bridge timing tests; the affected files are byte-identical to current upstream/main and outside this plugin PR's changed surface

Remote CI for this exact head is running. A single automated @codex review request for this SHA already exists at #2413 (comment) and currently has EYES, so I will not duplicate it.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[ryan-wong-coder]

Maintainer rerun request for exact head 874fb255cce217fb1f21655de45d2f435908a596.

The two latest Codex findings are fixed, replied to, and resolved. The only completed failing check is lint-and-test in https://github.com/binaricat/Netcatty/actions/runs/30168468671/job/89705380339.

The failed assertions are outside this plugin PR's changed surface:

• application/state/sftpTransferCenterStore.test.ts: orphan directory pause rolls back successful child pauses on hard fail
• electron/bridges/transferBridge.test.cjs: pause soft-drains concurrent ranges but resume waits before truncating

The affected SFTP/transfer files are byte-identical to current upstream/main on this PR head:

• application/state/sftpTransferCenterStore.test.ts
• application/state/sftp/useSftpTransfers.ts
• electron/bridges/transferBridge.cjs
• electron/bridges/transferBridge.test.cjs

I attempted to rerun the failed job with:

gh run rerun 30168468671 --failed

GitHub rejected it with: run 30168468671 cannot be rerun; Must have admin rights to Repository.

Local validation for the plugin change passed: focused review/completeness regressions 81/81, npm run test:plugin-runtime 402 passed with one intentional Electron skip, Electron runtime smoke, contract 77/77, lint, contract drift, build, pack:dir, and diff check. Full local npm test only reproduced unrelated upstream/environment failures in SFTP/SSH/transfer paths whose affected files are byte-identical to upstream/main.

Please rerun the failed lint-and-test job when available. I will not modify unrelated SFTP/transfer code or create an empty commit just to churn the reviewed SHA.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 874fb255cc

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

[binaricat]

@codex review

[ryan-wong-coder]

Independent follow-up review of exact head f7b5f9b8529cb8ad66146fd8e48c202e068d6302.

Result: no additional actionable correctness, security, compatibility, accessibility/localization, functional-completeness, lifecycle, or downstream PR 8-9 blockers were found after the corrective change.

This head fixes the three current Codex P2 findings from exact head 874fb255cce217fb1f21655de45d2f435908a596:

• Importer parsing now enforces MAX_IMPORT_LINE_BYTES while an unterminated JSONL record is still pending, rejecting malformed streamed output at the per-record cap instead of letting it grow toward the full import byte budget.
• Authentication challenge response failures now retain the active renderer dialog with a bounded localized error, so users can retry or cancel and the main-process authentication request is not left pending without UI.
• Host Details provider choices now subscribe to plugin contribution changes and refetch connection/authentication providers with stale-request suppression, keeping installed/enabled/disabled/uninstalled providers reflected without remounting the panel.

Downstream audit:

• No public RPC method names, permission names/scopes/lifetimes, runtime identity, cancellation protocol, database schema, or stream framing changed.
• PR 8 sync Providers keep the existing activation, opaque configuration, credential-reference, diagnostics, cancellation, provider-refresh, and stream seams.
• PR 9 signed distribution and publisher trust can still layer on the immutable identity, advanced-runtime, permission, companion, credential, operation-keyed Provider, importer-draft, and diagnostic boundaries.
• The complete unreleased plugin database remains at user_version = 1, with no migration chain.

Validation on this head:

• current machine-review regressions: 27/27 passed
• npm run test:plugin-runtime: 403 passed, 1 intentional Electron skip
• npm run test:plugin-runtime:electron: passed
• npm run test:plugin-contract: 77/77 passed
• npm run lint -- --quiet: passed
• npm run check:plugin-contract: passed
• npm run build: passed
• npm run pack:dir: passed
• git diff --check: passed
• npm test: 7,368 total, 7,352 passed, 13 skipped, with three unrelated upstream/environment failures in SFTP transfer-center, SSH auth-retry, and transfer bridge timing tests; the affected files are byte-identical to current upstream/main and outside this plugin PR's changed surface

Remote CI for this exact head is starting/running. I will request Codex exactly once for this SHA unless the existing automation has already posted that request.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: f7b5f9b852

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

[ryan-wong-coder]

Maintainer rerun/restart requested for exact head f7b5f9b8529cb8ad66146fd8e48c202e068d6302.

Codex completed the exact-head review with no major issues: #2413 (comment). All GraphQL review threads are resolved and there is no CHANGES_REQUESTED review.

The completed lint-and-test failure in https://github.com/binaricat/Netcatty/actions/runs/30169166894/job/89707151797 is outside this plugin PR changed surface. The only failed assertions are:

• application/state/sftpTransferCenterStore.test.ts: orphan directory pause rolls back successful child pauses on hard fail
• electron/bridges/transferBridge.test.cjs: pause soft-drains concurrent ranges but resume waits before truncating

The affected SFTP/transfer files are byte-identical to current upstream/main on this PR head:

• application/state/sftpTransferCenterStore.test.ts
• application/state/sftp/useSftpTransfers.ts
• electron/bridges/transferBridge.cjs
• electron/bridges/transferBridge.test.cjs

I attempted to rerun the failed job with:

gh run rerun 30169166894 --failed

GitHub rejected it with: run 30169166894 cannot be rerun; Must have admin rights to Repository.

The build-package workflow https://github.com/binaricat/Netcatty/actions/runs/30169166863 has build-macos, build-linux-x64, and build-linux-arm64 green, but build-windows remains pending/stuck. Please rerun the failed lint-and-test job and restart/rerun the stuck build workflow when available.

I will not modify unrelated SFTP/transfer code or create an empty commit just to churn the reviewed SHA.

[binaricat]

Thank you for addressing the five gaps from the previous maintainer review. I re-reviewed exact head f7b5f9b8529cb8ad66146fd8e48c202e068d6302 against the PR 7 requirements and the current repository architecture.

The previous five findings are now addressed: connection control results are operation-specific, importer draft records are public, UI workflows have moved into application hooks, runtime diagnostics are propagated, and the plugin-platform documentation has been updated. My local merge-with-current-main validation also passed 403 plugin-runtime tests (1 intentional skip), 77 plugin-contract tests, 175 changed-area tests, lint, contract drift checking, and the production build.

The PR is still not ready to merge, because two actionable findings remain:

1. Public importer key typing does not match runtime validation

The canonical schema requires exactly one of privateKey or filePath for ImporterKeyDraft: schema.

However, the handwritten generator override allows both properties at once because the opposite property remains optional in each branch: generator override, generated public type.

A third-party importer can therefore compile successfully and then have the same record rejected at runtime by the schema's oneOf. Please make the two branches truly mutually exclusive (for example, by making the opposite property never) and add a regression that proves the generated public type and runtime validator accept the same shapes.

2. Authentication overflow performs an external action inside a React state updater

When the authentication queue reaches its cap, the queue updater directly sends a cancellation response.

State updaters must be pure and replay-safe. If React retries or replays that updater, the same challenge can send cancellation more than once; the second response then reaches a request that the main process has already consumed. Please decide the overflow result in the state update and send the cancellation exactly once outside the updater, with a regression for one outbound response.

Separately, the current exact-head lint-and-test check is still red, and the branch is now behind the latest main even though a local merge tree is conflict-free. After the findings above are fixed, please sync with the current base and obtain a green current-head check run.

Merge verdict: do not merge this head yet. The direction remains sound and the previous review gaps are closed, but the public importer contract must be made self-consistent, the authentication cancellation path must be replay-safe, and the latest checks must pass.