Skip to content

Keep Git identity probes from blocking SSH host reconversion - #26578

Merged
OrcaWin merged 2 commits into
mainfrom
nwparker/orcad-migration-git-cache
Oct 9, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
nwparker/orcad-migration-git-cache

Conversation

@nwparker

@nwparker nwparker commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​125 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​125
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​32 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​27

ELI5

After converting an SSH host, stopping its server and reconnecting could leave it in relay mode. A routine Git check had changed a cached value, and Orca mistook that for a conflicting repository. The same host can now convert again while keeping the server’s current Git result.

What Changed

Repository imports use the existing catalog merge with a comparison that excludes gitRemoteIdentity, which the execution host resolves from Git. Matching rows stay in place; names, paths, settings, ownership, and all other fields still have to match. Other catalog types keep their exact comparison.

Added persistence regressions: three cache cases (restart with a resolved or no-remote identity, and a stale incoming identity) fail before the fix; three configuration-conflict guards stay enforced.

The earlier Docker Stop/reconnect end-to-end spec and its CI lane wiring were dropped when rebasing onto main: the unit regressions cover the merge rule directly, and the Stop flow is being reworked separately.

Why

Replacing an existing row with the retained source would overwrite the server’s newer cache. Comparing repository identity only would also hide genuine configuration conflicts. Excluding this one computed field preserves the server row and keeps those conflicts visible.

Linked Issue

Found while verifying SSH → Remote consolidation in #26483.

Visual Proof

Before: reconnect reports orcad_migration_repository_id_conflict and no managed server returns. After: reconnect reports managed, and reopening Settings shows the connected server. Both runs use a hidden macOS Electron client and an owned Linux Docker SSH host.

migration-git-cache-before.png
migration-git-cache-after.png
migration-git-cache-final-runtime-red.log
migration-git-cache-final-runtime-green.log

Testing

  • Manually exercised the rendered flow locally through CDP, real keyboard input, host process checks, CLI terminal close, UI Stop, reconnect, and repository RPCs.
  • Automated regressions added: three cache cases fail before the fix; three configuration-conflict guards stay enforced.
  • 152 tests across 22 files pass, including persistence and CI routing contracts.
  • Full pnpm tc, pnpm lint, affected oxlint, changed-code quality, Electron build, and Node server build pass.
  • Final real before run fails on the repository conflict; corrected run passes in 17.6 seconds. The live-terminal refusal preserves the original process; idle stop only runs after a zero terminal census. The redeployed server contains one repository with its resolved-null Git identity.
  • Corrected template uses rebuilt common server artifacts and the installed template’s unchanged native files; all nine target inventories verify. Real server execution was Linux, with a macOS client.

Review

Checked that matching imports retain existing objects, real configuration conflicts refuse before publication, other catalog comparisons stay exact, and request/response shapes stay compatible with older peers.

Agent skill upstream boundary

  • Not applicable; no upstream skill source copied or translated.

Notes

Rebased onto main (Phase 3.5 has merged). Existing hosts receive the fix when their managed server updates. Repository configuration differences still refuse import to preserve the destination’s state.

Remote Settings can keep its old list after a reconnect until reopened; that separate rendered finding remains under investigation. A document-list error toast during cutover also remains separate from this cache fix.

Checklist

  • Focused production change
  • ELI5, before/after, mechanism, and approach explained
  • Before/after screenshots and actual runtime logs attached
  • Self-reviewed for correctness and performance
  • Cross-platform, SSH, folder workspace, and mixed-version behavior considered
  • Local lint, typecheck, affected tests, and builds pass

@OrcaWin
OrcaWin force-pushed the nwparker/orcad-migration-git-cache branch from 28d5cbe to 6476aad Compare October 9, 2026 02:14
@OrcaWin
OrcaWin changed the base branch from OrcaWin/phase35 to main October 9, 2026 02:14
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bcd3ff0e-defb-48a3-8334-128fff7995f6

📥 Commits

Reviewing files that changed from the base of the PR and between 540955533dbaf62d999dcb634856fb77d7920094 and ba37006.


You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e43af7f1-0d3d-48ab-83de-1bca0b2ebcfb

📥 Commits

Reviewing files that changed from the base of the PR and between 6476aad and b6d9b5d.


📒 Files selected for processing (2)
  • src/main/persistence-orcad-repository-cache-reimport.test.ts
  • src/main/persistence/migrating-orcad-catalog/orcad-catalog-row-identity.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.



📝 Walkthrough

Walkthrough

Repository migration row selection now accepts an optional comparison function. Migration catalog preparation uses a repository comparison that ignores cached Git identity and ignores icons when both repositories use automatic GitHub icons. Tests cover identity retention after restart, stale incoming identities, automatic avatar retention, and conflicts when repository configuration or an uploaded icon differs.


Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b6d9b

Reconnecting to a converted SSH host should no longer fail repository import because of a cached Git identity update, while real configuration differences still conflict. No merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preventing Git identity probes from blocking SSH host reconversion.
Description check Passed The description is complete and well aligned with the template. It explains the user impact, mechanism, rationale, linked issue, visual evidence, testing, review scope, compatibility considerations, a…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.


✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@OrcaWin
OrcaWin force-pushed the nwparker/orcad-migration-git-cache branch from b6d9b5d to 5409555 Compare October 9, 2026 07:40
@OrcaWin
OrcaWin force-pushed the nwparker/orcad-migration-git-cache branch from 5409555 to ba37006 Compare October 9, 2026 07:51
@OrcaWin
OrcaWin merged commit 6ef42ad into main Oct 9, 2026
37 checks passed
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.

2 participants