Skip to content

feat(sql-files): review Git changes and commit selected scripts - #1454

Merged
NewtTheWolf merged 6 commits into
TabularisDB:mainfrom
lejyfps:feat/1418-linked-folder-git
Nov 10, 2026
Merged

NewtTheWolf merged 6 commits into
TabularisDB:mainfrom
lejyfps:feat/1418-linked-folder-git

Conversation

@lejyfps

@lejyfps lejyfps commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1418. Depends on #1423 (#1409), which is still open. This draft includes that prerequisite branch merged onto current main; review the final feature commit separately until #1423 lands.

Linked SQL folders now offer a desktop dialog to inspect saved-file Git diffs and explicitly commit selected, reviewed scripts. Unsaved editor drafts are left alone. The containing repository may be larger than the approved folder or a Git worktree. Renames appear as an explicit deletion/addition pair.

The backend uses literal argument arrays and an alternate index starting from HEAD. Review tokens bind disk content, staged blobs, HEAD/ref and the real index. Commits acquire the real index lock, preserve unrelated staged entries, refuse stale reviews/conflicts/in-progress merges, run commit hooks, honor signing, and update the branch with an expected-old-HEAD check. Hook/signing errors abort before the ref update; failures after creation return the commit id with a warning. No reset, clean or remote operations are used.

Only persisted approved folder ids reach the desktop wrappers; paths, symlinks, file types and sizes are checked in the shared Git module. Browser RPC has no routes for the three new commands. The feature is independent of built-in or external database drivers. UI strings are included in all 13 locales.

Validation:

  • 156 focused frontend tests passed across the new dialog, linked folders, sidebar, platform capabilities and dual-transport contract.
  • 12 real disposable-repository Git tests passed on Windows, including unrelated staging, stale disk/index/HEAD, nested links, worktrees, concurrent commits, unusual paths, rename/delete, locks, failing/scope-changing hooks, oversized historical blobs and signing failure. Clippy passed with warnings denied. An isolated Cargo harness compiles the production Git module directly, avoiding the unrelated OpenSSL dependency.
  • Typecheck, targeted ESLint, Prettier, theme tokens, frontend transport boundaries and git diff --check passed.
  • GitNexus detect_changes was run on the staged feature. It reported CRITICAL/all 300 flows and attributed unrelated grid flows to an isolated new Rust test. Direct reference inspection instead confirms three production callers of the approved-root resolver, all new desktop wrappers. The 26-file feature diff and additive command contract were checked directly; graph risk classification is not reliable for this run.
  • Full cargo check --no-default-features is locally blocked before compiling the app: vendored OpenSSL requires Perl, which is absent from PATH. Full app integration, root-authorization/RPC tests and the Unix-only symlink/literal-special-path cases require CI. No interactive desktop smoke test was performed.

Manual check: in a disposable repository with an initial commit, link a nested scripts folder containing a modified query.sql and stage an unrelated file outside it. Open Review Git changes, inspect query.sql, check it, enter a message and commit. Confirm the new commit contains only the script and the unrelated file remains staged. Review again, edit the file externally and confirm the commit is refused. Repeat with a failing pre-commit hook and verify HEAD stays unchanged. Keep an unsaved editor draft open throughout and confirm its content is retained.

Limits: Git 2.36+ and an existing HEAD on a checked-out branch are required. Binary/non-UTF-8 scripts and files/blobs/diffs above 2 MiB are refused; status is capped at 1000 scripts. Repository hooks/signers execute according to the user's Git configuration and can wait or modify working files; the app never discards those changes.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

@lejyfps
lejyfps marked this pull request as ready for review November 9, 2026 20:27
@lejyfps
lejyfps marked this pull request as draft November 9, 2026 20:28
@kilo-code-bot

kilo-code-bot Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge (after prerequisite #1423 lands; this draft bundles it)

Notes

  • The diff bundles the prerequisite branch (feat(editor): browse a linked local folder of SQL scripts #1423) with the feature commit. Review focused on the feature files: the new Git module linked_folder_git.rs and its tests, the LinkedFolderGitModal UI, the desktop command wrappers, the linked_git_root resolver, the contract and capabilities wiring, and the i18n keys.
  • Static review only. No code was executed, and the Git tests were not run in this environment.
  • The design checks out on read-through. Commands use literal argument arrays and an alternate index, the real index.lock is held through commit, review tokens bind disk content, staged blobs, HEAD and the real index, and the ref update checks the expected old HEAD. The new commands are not registered for browser RPC, and rpc_tests.rs asserts this.
  • Behaviors worth manual confirmation, none of them defects:
    • The review token hashes the whole real index file, so external tools that refresh index stat data (for example git status in a terminal) can invalidate a review. The user then has to re-review.
    • The success message (result) in the modal is not cleared when a later review or commit fails, so a stale success line can sit next to a new error.
Files Reviewed (40 files)
  • src-tauri/src/application/linked_folder_git.rs
  • src-tauri/src/application/linked_folder_git_tests.rs
  • src-tauri/src/application/linked_folders.rs
  • src-tauri/src/application/linked_folders_tests.rs
  • src-tauri/src/application/mod.rs
  • src-tauri/src/transport/desktop/linked_folders.rs
  • src-tauri/src/transport/desktop/mod.rs
  • src-tauri/src/transport/web/rpc_tests.rs
  • packages/web-ui/src/components/modals/LinkedFolderGitModal.tsx
  • packages/web-ui/src/components/layout/sidebar/LinkedFoldersSection.tsx
  • packages/web-ui/src/api/contract.ts
  • packages/web-ui/src/types/linkedFolderGit.ts
  • packages/web-ui/tests/components/modals/LinkedFolderGitModal.test.tsx
  • packages/web-ui/src/i18n/locales/*.json (13 locales)
  • Remaining prerequisite-branch files (Editor.tsx, ExplorerSidebar.tsx, Sidebar.tsx, LinkedFileConflictModal.tsx, utils/linkedFolders.ts, tabCleaner.ts, platform capabilities and related tests) were skimmed only.

Reviewed by claude-sonnet-5.5 · Input: 254 · Output: 5.3K · Cached: 404.4K

Review guidance: REVIEW.md from base branch main

@lejyfps
lejyfps marked this pull request as ready for review November 9, 2026 20:32

@NewtTheWolf NewtTheWolf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for keeping commits scoped to approved saved scripts, using a separate index and preserving unrelated staging. I reviewed 4a2fb03c2bbe7030278a033207131f17040b115f, including the bundled linked-folder prerequisite and desktop/web capability boundaries.

The 41 targeted frontend tests, typecheck, production frontend build, targeted ESLint and desktop cargo check passed. The targeted Rust run had 42 passes and one failure: signing_failure_does_not_move_head_or_modify_the_index unexpectedly creates a commit when the inherited Git signing format is SSH.

I reproduced this independently with a generated throwaway Ed25519 key and temporary GIT_CONFIG_GLOBAL: gpg.format=ssh fails the new test, while changing only that format to openpgp makes it pass. With global Git configuration disabled, all 43 targeted Rust tests pass. The test overrides gpg.program, which applies to OpenPGP, but leaves the signing format inherited, so it can use a developer's real SSH signer instead of exercising failure.

Please make the signing-failure fixture independent of user/system Git configuration. Pin its format and failed signer explicitly, and cover SSH failure separately if required. This is a new test portability/isolation issue, not a failure attributed to existing main code.

I have not run real-app E2E or the full merge-gate chain for this revision. After the test fix and prerequisite #1423, verify the native dialog with a disposable nested scripts folder: selected-script-only commit, unrelated staging retained, stale-file/index/HEAD rejection, failing hook/signing behavior and an unsaved editor draft retained throughout.

git(temp.path(), &["config", "commit.gpgsign", "true"]);
git(
temp.path(),
&["config", "gpg.program", "tabularis-missing-test-signer"],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Isolate this failure test from the inherited signing format. With a valid SSH signing key and gpg.format=ssh, setting gpg.program to a missing executable has no effect; the production commit correctly uses the SSH signer and this test fails at line 276. I reproduced it with a disposable key and temporary global config, then got a pass by changing only the format to OpenPGP. Pin the fixture's format/failed signer and prevent tests from invoking a developer's configured signer.

NewtTheWolf
NewtTheWolf previously approved these changes Nov 10, 2026

@NewtTheWolf NewtTheWolf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for isolating both signing formats and using a fixture signing key. All 43 linked-folder/Git Rust tests now pass with my normal Git configuration, including the SSH and OpenPGP failure cases. The 18 frontend tests and typecheck also pass.

I exercised the Git modal in the real Linux Tauri app: opened a linked SQL file, edited and saved it, reviewed its diff, selected it and committed it. The resulting commit contains only that SQL script; an unrelated file staged externally before the test remains staged and unchanged. This resolves my signing-test blocker. Approved.

@NewtTheWolf NewtTheWolf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I resolved the conflicts after #1423 landed and rechecked c07bc98. The 43 Rust linked-folder/Git tests, 18 frontend tests, typecheck and production build pass on the integrated tree.

The Git implementation, linked-file writing code and Git modal are unchanged from cf5018c, where I verified the native diff/selected-file commit flow. The external staged file was preserved in that app test. Approved on the updated head.

@NewtTheWolf
NewtTheWolf merged commit 03aa4bc into TabularisDB:main Nov 10, 2026
14 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.

[Feat]: Linked SQL folders: inspect Git changes and commit reviewed scripts

3 participants