feat(connections): reach a database through a tunnel command - #2638
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a fifth
ConnectionTunnelKind: a process TablePro starts on connect, holds for the life of the connection, and restarts when it dies. Presets coverkubectl port-forwardandaws ssm start-session; a third method takes a command line.Fixes #2520
Approach
This follows the extension point CLAUDE.md already names: a new tunnel type extends
ConnectionTunnelKind,activeTunnelManager(for:)andbuildEffectiveConnection, rather than adding per-site branches.TunnelCommandManageris shaped likeCloudSQLProxyManager(allocate a loopback port, start the process, wait for the port or for the process to give up, watch it afterwards) and reusesSupervisedProcessRunner,LoopbackPort,StaleProcessReaperandrecoverDeadTunnelunchanged.The local port is TablePro's to pick.
{port}marks where it goes;{host}and{remotePort}carry the connection's own endpoint. Presets read the remote port from the connection, so it is never typed twice.No implicit shell. The command is split into an argument vector the way a shell would split it, quoting included, and
execved directly. Two reasons: a shell between TablePro and the forward is one more thing to signal, and nothing is expanded, so$HOMEstays four characters. A user who wants a shell writes/usr/bin/envor/bin/sh -cas the first word. Substitution runs after the split, so a value carrying a space cannot become two arguments.A preset carries parameters, not code. Every preset flag is written
--flag=value, so no field can be read as a flag of its own, and the two positional arguments kubectl still needs are validated instead (TunnelCommandBuilderTests.presetValuesCannotBecomeFlags).Killing the process is the load-bearing part
ProcessSupervisedRunner.stop()sentSIGTERMto the child pid and nothing else.aws ssm start-sessionrunssession-manager-pluginbeside itself, and the plugin is what actually holds the forwarded port, so signalling the pid alone leaves an orphan holding it.Measured on this machine: Foundation's
Processputs every child in its own process group (getpgid(child) == child, distinct from ours) and descendants inherit it.stop()now signals the group and escalates toSIGKILLafter two seconds, with the guard that bothterminationResultandisRunningmust still say the process is live, so a forced kill can never reach a group that inherited a recycled pid. Cloudflare and Cloud SQL Proxy get the same improvement.SupervisedProcessRunnerTestspins both the group-leadership fact and the descendant teardown.A cancelled connect now stops the process too. It is not registered as a tunnel until it is ready, so nothing else would have closed it.
Sharing, and the arbitrary-code question
The issue asked for a confirmation on import. The change does both halves:
SyncRecordMapperexcludes it with the other three tunnel modestablepro://deeplink, team library.tableprofilekeepTunnelCommandsdefaults tofalse, so a route that never asks cannot let one in by omissionconnections.jsonConnectionStoreIntegrityalready detects this. The tunnel refuses to launch until the connection is saved again in the app, the same gatePasswordSourceusesThe split is deliberate: a deeplink is a click, a file import is a decision.
Before / After
New pane, so there is no before.
docs/images/tunnel-command-pane*.pngare placeholder cards at 1560x960, which is whatsocks-proxy-pane.png,cloud-sql-proxy-pane.pngandcloudflare-tunnel-pane.pngalready are (a shipped shot is 3024x1722). The page markup and dimensions are right; a real capture is still owed, and I did not take one because another session is running builds in this checkout and a second TablePro instance breaks an in-flight test host.Verification
Run in an isolated worktree, because the main checkout has unrelated in-flight work that does not compile.
verify.sh build: PASSverify.sh testover 15 suites (the five newTunnelCommand*suites,SupervisedProcessRunnerTests, and the nine existing suites that own the types this touches): PASS, 117 of 117 casesverify.sh docs: PASS, house style and source claims both agreeswiftlint --strictover the 51 changed Swift files: cleanCodex could not review this diff: the account is out of credits until Sep 7 (
Codex error: You've hit your usage limit, read from the job log, not stdout).Skill(code-review)at high effort read it instead and found one thing on this branch:TunnelCommandLine.ParseErrorescaping unlocalized, which madeTunnelCommandError.commandEmptyand.unbalancedQuoteunreachable and would have shown "The operation couldn't be completed" for an imported command with an unclosed quote. Fixed, with a test.No UI automation. The pane's behaviour that matters is the argument vector and the process lifecycle, both covered by unit tests; a UI test would only assert that a
Formrenders its fields.https://claude.ai/code/session_016cKgYoccE88XFeVPuBLRak