Skip to content

Merging the open PRs in sequence: the order that works, and six overlaps git does not all catch #71

Description

@Sadykhzadeh

Every open PR merges cleanly into main on its own — GitHub reports all nineteen as MERGEABLE. They do not all merge cleanly into each other: six overlaps appear once they are landed in sequence, two of which git resolves silently and wrongly.

I merged all nineteen into one scratch tree to find out what actually breaks. The combined tree builds and passes: cargo test --workspace 189 passed / 0 failed, vitest run 369 passed across 43 files, vue-tsc --noEmit clean, check-comments.mjs {"ok":true,"count":0,"files":137}. So the set is compatible — this issue is only about the order and the resolutions.

Order that works

  1. Add Greptile PR review agent skills (check-pr, cli-review, greploop) #68 chore/greptile-pr-review-skills
  2. Make the comment hygiene check work on CRLF checkouts and run it in CI #70 fix/check-comments-crlf — early, so everything after it is actually scanned
  3. Pin CI actions to SHAs, drop install scripts, and keep fork PRs off self-hosted runners #42 security/harden-ci-supply-chain
  4. Give Windows state files an explicit owner-only DACL instead of the profile's inherited one #46 security/windows-state-file-dacl
  5. Declare the CSP directives that do not inherit, drop the unused GitHub origins, and scope shell open #47 harden-webview-csp-and-shell-open-scope
  6. Refuse a second instance against the same app data, and validate workspace layouts before they can brick startup #40 fix/persisted-state-integrity
  7. Build russh without zlib to close the unbounded decompression path #29 security/bound-russh-decompression
  8. Report changed host keys with both fingerprints and stop unpinning from downgrading a host to first-use #38 security/host-key-change-confirmationconflict A
  9. Bind the vault header to its ciphertext so tampered key metadata cannot pass #59 fix/vault-header-integrity — carries Keep the vault available and off the KDF during SSH connects #52; merging it merges Keep the vault available and off the KDF during SSH connects #52conflicts B, C, D, E
  10. Close open sessions on vault reset, and stop retaining imported key material after Cancel, a failed import or a vault lock #58 security/vault-reset-containment-and-key-material-clearingconflict F
  11. Gate biometric disable and drop the unused updater/process capability #33 security/gate-biometric-disable-and-trim-capabilitiessilent break G
  12. Leave one update check, the one the user can see #66 fix/single-update-check-pathconflict H
  13. Build tokio with the features the workspace uses, drop two dead direct deps, and delete two unregistered SFTP commands #61 chore/trim-tokio-features-and-dead-commandsconflict I
  14. Reject local terminals on a live session id and cap concurrent ones #31 fix/local-terminal-admission-guardconflicts J, K; silent break L
  15. Coalesce terminal output and move it off the JSON event path #49 perf/terminal-ipc-coalescingconflicts M, N; silent break O
  16. Boot the app in 3 IPC round-trips instead of 12, and stop awaiting the update check #55 perf/parallel-boot-loads
  17. Cut the frontend entry chunk by 25%: split navigable views, drop tailwind-merge, enable Vue production flags #37 perf/frontend-bundle-size
  18. Render terminals on the GPU, with a context budget and a DOM fallback #63 perf/terminal-webgl-renderer

#52 is not listed separately because #59 is stacked on it — git merge-base --is-ancestor confirms #59 contains #52's tip, so merging #59 lands both. Merging #52 first is fine and makes #59 a fast-forward of the overlap; merging #52 after #59 is a no-op.

Nothing in 1–7, 16–18 conflicts with anything.

The conflicts, and how each resolves

A — #38, core/src/connection.rs. Both sides wrap the client::connect error. Keep both: the HostKeyMismatch passthrough (#38) and the negotiation hint (#29), with the hint applied to the other arm.

B — #59, core/src/connection.rs. #59 wraps the connect in tokio::time::timeout and switches Config::default() to client_config(). Keep that shape and put A's match in its inner map_err.

C — #59, desktop/src-tauri/Cargo.toml. Both append to [dev-dependencies]. Union: russh-keys, tauri = { features = ["test"] }, and the Linux zbus block.

D — #59, desktop/src-tauri/src/commands.rs. Two unrelated test modules both appended at EOF (known_host_confirmation_tests, connect_lock_tests). Union, each with its own closing braces. Same shape as I and as part of M.

E — #59, desktop/src-tauri/src/state.rs. #38 adds host_key_prompt, #59 replaces passphrase with vault_session. Keep host_key_prompt and vault_session; passphrase is gone. Same in the constructor.

F — #58, desktop/src-tauri/src/vault_commands.rs. #58 clears state.passphrase; #59 replaced that with state.vault_session.clear().await. Keep the session clear, then #58's contain_open_sessions call. Union the imports.

G — #33, silent. Git merges biometric_commands.rs without complaint and it does not compile: the tests call state.passphrase (gone with E) and Vault::verify_passphrase / initialize, which #52 made async. authorize_biometric_disable becomes async and awaits verify_passphrase; clear_biometric_passphrase reads state.vault_session.passphrase().await; the four #[test]s become #[tokio::test].

H — #66, desktop/src-tauri/src/main.rs. #66 deletes check_for_updates_silent; #40 appends a test module right after it. Take the deletion, keep the tests.

I — #61, desktop/src-tauri/src/commands.rs. Test modules appended at EOF again (pty_dimension_tests against D's pair). Union.

J — #31, desktop/src-tauri/src/commands.rs imports. Union: host_key_prompt and LocalTerminals.

K — #31, create_local_terminal. #61 adds the checked pty_dimension conversion; #31 adds the reservation and release_on_err!. Both, with the dimension check first — a geometry that cannot be represented is refused before a slot is claimed, so there is nothing to give back — and PtySize { rows, cols, .. } taking the checked values.

L — #31, silent. contain_open_sessions (#58) is generic over &Mutex<HashMap<String, T>>; #31 replaces that map with LocalTerminals. It needs a clear() on LocalTerminals that drops live terminals and reservations — a reservation left behind holds a slot for a terminal nobody wants any more — and the signature becomes concrete. Its three tests build a LocalTerminals with one reservation rather than a one-entry HashMap.

M — #49, desktop/src-tauri/src/commands.rs. Four hunks. Union the imports; keep #31's fulfil block, then #49's channel and batching threads; the reader thread feeds the channel rather than emitting; the ownership flag guards only the closing notice.

That last point is worth stating on its own: with #49, output reaches the frontend on the session's own Channel, so a stale reader physically cannot deliver into a replacement terminal. The half of #31's ownership flag that guards session-data becomes redundant, and only session-closed — which is still addressed by id alone — needs it.

N — #49, desktop/src-tauri/src/state.rs. Union: output_sinks alongside local_terminals: Mutex<LocalTerminals>, and unlocked_vault (from #59) kept.

O — #49, silent. #52's connect_lock_tests calls connect_ssh with seven arguments; #49 adds on_output: OutputSink. The test passes tauri::ipc::Channel::new(|_| Ok(())).

Two things this turned up

check-comments.mjs scanned nothing on a CRLF checkout before #70, so every clean run of it quoted in these PRs' descriptions was vacuous. Re-running the fixed scanner against all nineteen branch heads finds exactly two violations, both since fixed: one in #38 (connection.rs) and one in #63 (settings.ts).

Four of the fifteen overlaps — G, L, O, and the session-data half of M — are invisible to git. Anyone merging these should build and test after each one rather than trusting a clean merge.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions