Repository navigation
feat: one-click GitHub, Jira and Linear credentials pane - #12
Conversation
A hooks module adds an integrations band above the prompt, a side pane, a status line entry and /integrations. It lists credentials found on the machine, syncs them to the Altimate workspace or keeps them local, runs the browser consent flow, and offers Reconnect when a token expires. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each provider row expands into the workspace's own credential fields, pre-filled from the machine, with Create token and Paste from clipboard for the secret. Connect & sync, Keep local and Update replace the browser sign-in buttons, and a rejected token shows in the row, the band and the status line. Secrets never pass through the mod. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hkaltai
changed the title
The pane said "synced to your Altimate workspace" whatever happened. It now shows the CLI's message after connect/update (attached, not linked, or why the attach failed) and reads each sync result, so a skipped or failed provider shows its reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When the project is not linked, the pane says so and offers Link workspace: it lists the account's workspaces, links the chosen one, then syncs so credentials already saved attach to it straight away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lick-local-integrations-sync
- Sign in to sync when signed out; sync controls hidden until then. - Keep local, Update and Disconnect offered per this tool's own connection. - CLI messages, notes and warnings reach toasts on every action. - Missing-binary fallback only when altimate is not installed; a timeout never reruns a command against altimate-code. - Timeouts sized per command; Sync all gets its own budget. - README, CHANGELOG and CONTRIBUTING describe the real flow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lick-local-integrations-sync # Conflicts: # plugins/altimate-code/CHANGELOG.md
- Sync all and Link workspace run one `sync <provider> --keep-synced` call per provider, each within its own timeout; one failure doesn't stop the rest. - Band Connect and Keep local discard leftover drafts first, close an open form for that provider, and stop if the discard fails. - A detected credential is offered beside a hand-made workspace connection. - Status reads that finish late no longer overwrite a newer one; the scan at session start only runs in interactive sessions. - README and CHANGELOG describe the Keep local outcomes and timing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // A local credential that is not blocked only waits for a sync; a blocked one | ||
| // is unblocked and synced in the same click. One saved in the workspace has no | ||
| // local copy to keep: Update (above) offers Keep local, which moves it here. | ||
| if (!signedOut && one.state === 'connected' && one.sync === 'local') { |
There was a problem hiding this comment.
WARNING: A sync === 'pending' row has no action left
The pane still renders this state as ready to sync (SYNC_LABEL.pending) and SyncState still declares it, but the only Sync button now requires one.sync === 'local', and the band no longer lists connected && sync === 'pending' either (line 624). A provider reported as pending therefore shows "ready to sync" with no control to sync it (only Sync all reaches it). Either keep 'pending' in this condition or drop the state and its label.
There was a problem hiding this comment.
Not changed: the CLI reports sync pending only together with state detected, never on a connected row, and the pane shows the sync label only for connected rows. A detected row offers Connect and Keep local instead, so no connected row can show a pending label.
| async function runJson<T extends { message?: string | null }>($: EngineInterface, args: string[], timeoutMs?: number) { | ||
| const result = await runCli($, args, timeoutMs) | ||
| const data = parseJson<T>(result.stdout) | ||
| const failure = result.exitCode === 0 ? null : (data?.message ?? firstLine(result.stderr.replace(ANSI, '')) ?? 'failed') |
There was a problem hiding this comment.
SUGGESTION: runJson ignores the JSON error field
failureText (line 122) treats a CLI body's error as a failure reason, but runJson reads only message. When connect/update/fields/set/discard fail with a body shaped { "error": ... }, the pane falls back to failed / not connected instead of the CLI's reason. Reuse failureText here (or also read data?.error) so both paths report the same message.
There was a problem hiding this comment.
Fixed in #16: runJson now takes the failure reason from failureText, so a { error } body is reported the same way as { message }. Covered by a new test.
| const result = await runAltimate($, ['workspace', 'list', '--json']) | ||
| const data = parseJson<{ ok?: boolean; workspaces?: WorkspaceChoice[] }>(result.stdout) | ||
| if (result.exitCode !== 0 || !Array.isArray(data?.workspaces)) { | ||
| await update($, message, () => failureText(result) === 'failed' ? 'Could not list workspaces.' : failureText(result)) |
There was a problem hiding this comment.
SUGGESTION: Redundant double evaluation
failureText(result) is called twice in the same expression (the same pattern with failureText(link) appears at line 291). Each call re-parses stdout. Bind it once, e.g. const reason = failureText(result), and use reason in both branches.
There was a problem hiding this comment.
Fixed in #16: the reason is computed once and reused in listWorkspaces and linkWorkspace.
| } | ||
|
|
||
| /** Forgets what was typed or pasted for a provider, so an abandoned form cannot leak into the next connect. */ | ||
| async function discardEntries($: EngineInterface, provider: ProviderId) { |
There was a problem hiding this comment.
SUGGESTION: Duplicate discard logic, and failures are silent here
discardEntries and discardForAction (line 331) both run discard <provider> --json, and only the second reports the failure. Because cancelForm and openForm use this one and its catch {} swallows errors, a failed discard leaves the aborted form's stored secret in the CLI, which a later connect for that provider can pick up -- the band path deliberately refuses to connect in exactly this case. Fold both into one helper that returns the failure and surface it in the form.
There was a problem hiding this comment.
Fixed in #16: one discardEntries helper returns the failure and discardForAction builds on it. Opening a form shows the failure in the form, and cancelling reports it in the pane instead of ignoring it. Covered by a test.
| async function loadForm($: EngineInterface, open: Expanded) { | ||
| try { | ||
| const { data, failure } = await runJson<Form & { message?: string }>($, ['fields', open.provider, ...(open.variant ? ['--variant', open.variant] : []), '--json']) | ||
| if (data && Array.isArray(data.fields)) await update($, form, () => data) |
There was a problem hiding this comment.
SUGGESTION: Only fields is validated before the render dereferences the rest
loadForm accepts any payload that has a fields array, but the pane reads fields.variants.length (line 786) and fields.tokenUrl (line 760) unguarded. A payload without variants -- including a persisted form atom written by an older plugin version, since these atoms carry no shape tag -- throws during render. Normalize the missing pieces, e.g. { ...data, variants: data.variants ?? [] }.
There was a problem hiding this comment.
Fixed in #16: loadForm turns a missing variants list into an empty one. A new session already starts with empty mod state, so no form from an earlier session is rendered.
| description: 'One-click GitHub, Jira and Linear integrations for your Altimate workspace', | ||
| }) | ||
| // A press that was running when an earlier session ended can never finish, so it must not block this one. | ||
| await update($, busy, () => null) |
There was a problem hiding this comment.
SUGGESTION: Reset the rest of the per-session state too
Only busy is cleared here. expanded, form, typed and workspaces are persisted PluginState atoms and are never reset at session start, so a form left open when a session ended reappears in the next session with its typed values, and the workspace picker can show a stale list. Clearing them alongside busy matches the reason given for resetting busy.
There was a problem hiding this comment.
Not changed, on purpose. Mod state is held per session, so a new session already starts without the old form or picker. session.start also runs when the plugin reloads or its worker restarts, and clearing the form there would close one the user is filling in and lose what they typed. The busy reset stays, because a press that was running can't finish after a restart. Explained in #16.
| const who = ['github', 'jira', 'linear'].includes(one.id) ? nameOf(one.id as ProviderId) : one.id | ||
| if (one.result !== 'synced') return `${who}: ${one.reason ?? one.result ?? 'not synced'}` | ||
| const steps = one.steps ?? [] | ||
| const bad = steps.filter(step => !step.ok).map(step => step.label) |
There was a problem hiding this comment.
SUGGESTION: Tolerate both step shapes in the sync report
steps is typed as objects here, but the sync report has also emitted each step as a plain label string (the previous revision normalized that). For a string step, step.ok and step.label are undefined, so every step counts as failed and the line reads ... undefined failed. A one-line normalizer (typeof step === 'string' ? { label: step, ok: !step.startsWith('Not linked') } : step) keeps both shapes working.
There was a problem hiding this comment.
Not changed: the CLI has emitted sync steps as { label, ok } objects since the first version of the command, so the string form is never produced.
| - `gh`, jira-cli and Linear credentials on this machine, and GitHub, Atlassian or Linear MCP servers configured in Claude Code, are detected when a session starts. | ||
| - A band above the prompt offers **Connect**, **Keep local**, **Review** or **Later** for what it found, and shows **Token rejected · Update** when a saved token stops working. The status line shows how many integrations are connected. | ||
| - `/integrations` opens a side pane with one row per provider. **Add credentials** (or **Update**) opens a short form: type the non-secret fields, then **Create token ↗** opens the provider's token page and **Paste from clipboard** stores the token (shown masked afterwards). **Connect & sync** saves it, **Keep local** saves it without syncing, **Cancel** discards the form. | ||
| - A connected row offers **Sync**, **Allow sync** (after Keep local) and **Disconnect**. In a folder not linked to a workspace, **Link workspace** lists your workspaces and links the one you pick. When you are not signed in to Altimate, everything works locally: Connect behaves as Keep local, the sync buttons are hidden, and the band and pane offer **Sign in to sync**, which opens the browser sign-in on that click only. |
There was a problem hiding this comment.
SUGGESTION: Documentation contradicts the signed-out behaviour
This says Connect "behaves as Keep local" when signed out, but the implementation hides Connect (integrations.tsx:668) and hides Connect & sync in the form (integrations.tsx:800); the CHANGELOG describes the hidden-button behaviour. Adjust the sentence so users are not looking for a Connect button that is never drawn.
There was a problem hiding this comment.
Fixed in #16: the README now says Connect and Connect & sync are not shown when signed out (Keep local stays), matching the pane and the changelog.
Code Review SummaryStatus: 9 Issues Found | Recommendation: Address before merge Overview
Incremental review of Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit 3d6a04c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3d6a04c)Status: 9 Issues Found | Recommendation: Address before merge Overview
Incremental review of Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 4542fad)Status: 8 Issues Found | Recommendation: Address before merge Overview
The credentials path is sound: secrets only move through Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Reviewed by deepseek-v4.1-flash-1017 · Input: 42.7K · Output: 6.6K · Cached: 422.5K |
The Link workspace picker laid the workspaces out in a single row, so the ones past the pane's width were cut off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sahrizvi
left a comment
sahrizvi
left a comment
There was a problem hiding this comment.
Approved. One-click GitHub, Jira and Linear credentials via the hooks module look correct against the integrations CLI contract in AltimateAI/altimate-code#1622 (head c6d1aa3).
Checked and holding: no secret value reaches the mod's state, props, toasts, status line or argv. Secret fields come back as a masked tail only, are never rendered as an input, and are set only by set-secret --from-clipboard in the CLI. The browser opens only on a click, and only to the CLI's hardcoded provider token pages. Commands run as argv with no shell. Long work runs off the press handlers, and session start doesn't wait for the scan (skipped for -p/SDK). Field names, statuses and exit codes match the CLI.
Four minor findings are inline. None blocks:
- The
busyguard is check-then-set across awaits: safe to do in a follow-up (the CLI lock keeps data consistent; only redundant work). - An older CLI without
integrationsshows a help/logo line rather than "update altimate": edge input. - Allow sync, Update → Keep local, the row's Connect and Rescan are never pressed in the tests: test-only.
- The no-rerun-after-timeout guarantee depends on the wording of the error text, which the tests fake: test-only.
Nits are in a separate PR comment.
| /** Connect (or keep local) every provider the band found, one after the other. */ | ||
| function connectFound($: EngineInterface, providers: Provider[], localOnly: boolean) { | ||
| later($, async () => { | ||
| if (await read($, busy)) return |
There was a problem hiding this comment.
Minor (logic): the busy guard is check-then-set across awaits
connectFound reads busy, awaits update(message), and only sets busy inside the loop. Two presses that land before the band redraws (Connect then Keep local, or a double click) both pass the check and run two connect sequences. The same check-then-set pattern is in syncAll (196-197), act (212-213) and withProgress (373-374), though with fewer awaits in between. The CLI's lock and the upsert of the managed connection keep the result consistent, so the cost is redundant provider checks and uploads, and the last press wins rather than the first.
Suggestion: set busy immediately after the check, before any other await, in every guarded entry point.
Not a blocker: the data stays consistent; only redundant work results.
| await update($, status, () => next) | ||
| showStatusLine($, next) | ||
| } else { | ||
| const failure = parseJson<{ message?: string }>(result.stdout)?.message ?? firstLine(result.stderr.replace(ANSI, '')) |
There was a problem hiding this comment.
Minor (CLI contract): an older CLI without integrations gives no actionable message
A released CLI that lacks the subcommand fails fast: yargs .strict() prints help (with the logo) to stderr and exits 1, so nothing hangs. Here, though, the pane then shows firstLine(stderr), which is the first line of that help/logo banner. The user is never told to update altimate. The altimate-code fallback doesn't apply either, because altimate exists.
Suggestion: when the exit code is non-zero and stdout is not JSON (optionally when stderr contains Unknown argument), show something like "altimate is too old for one-click integrations; update it".
Not a blocker: edge input. The README states the requirement, and session start is not slowed.
| expect(await ui.find({ key: 'jira-add' })).toBeDefined() | ||
| expect(await ui.find({ key: 'jira-update' })).toBeUndefined() | ||
| expect(await ui.find({ key: 'jira-off' })).toBeUndefined() | ||
| expect(await ui.find({ key: 'linear-allow' })).toBeDefined() |
There was a problem hiding this comment.
Minor (testing): several row actions are never pressed
This test checks that linear-allow exists, but no test presses Allow sync (unblock then sync, stopping after a failed unblock). No test opens an Update form and presses Keep local, which is the path that sends connect --local-only instead of update (integrations.tsx:466) and moves a workspace credential back to this machine. No test presses a row's single Connect (without --local-only) or Rescan either.
Suggestion: add press tests that assert the argv sequence for each.
Not a blocker: test-only.
|
|
||
| // A command that cannot start rejects; only that moves on to the next name, so a | ||
| // timeout never reruns a command that may have taken effect. | ||
| const MISSING_BINARY = /ENOENT|not found|no such file|(cannot|could not|failed to) (start|spawn|run)/i |
There was a problem hiding this comment.
Minor (testing / robustness): the no-rerun guarantee rests on error wording
Besides ENOENT, this regex matches any rejection containing not found, could not run or failed to run. The two timeout tests (integrations.test.tsx:383, 496) fake the rejection with a hand-written 'timed out while the command was still running'. Nothing pins the engine's real timeout or abort wording against this pattern, and a match would rerun a possibly non-idempotent connect against altimate-code.
Suggestion: narrow the fallback to a spawn-not-found error (ENOENT) only, or test against the engine's documented rejection.
Not a blocker: test-only. The documented timeout path behaves correctly today.
|
Review nits and notes (not blocking; the approval and inline findings are in the review above) Nits
Note (not a finding)
|
…lick-local-integrations-sync
| {choices && !running && ( | ||
| <Box flexDirection="column"> | ||
| <Text dimColor>Link this folder to:</Text> | ||
| <Box flexDirection="column"> |
There was a problem hiding this comment.
SUGGESTION: The inner column Box is now redundant
The outer Box on line 720 is already flexDirection="column", so this wrapper only restates the same direction and adds a nesting level. Rendering the workspace buttons directly in the outer Box (dropping this element and its closing tag) gives the same one-per-line layout with one container fewer.
There was a problem hiding this comment.
Fixed in #16: the workspace buttons are rendered directly in the outer column, dropping the redundant inner Box; still one per line.
open-token-page only opens the browser with --open under --json, and the button press is the explicit request to open it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Adds one-click GitHub, Jira and Linear integrations to the plugin as a hooks module (a Claude Code mod). It collects the same credentials the Altimate workspace web form asks for; there is no OAuth app. Stacked on #11; needs
altimate integrationsfrom altimate-code./integrations): one row per provider, with its state, account and whether it is local or synced.••••a1b2, with Create token ↗ and Paste from clipboard.Test plan
claude plugin validate plugins/altimate-codepasses, andtscis clean.claude plugin test plugins/altimate-code: 19 pass, 0 fail. Every test case is listed below.tests/*.sh(data-mode, refresh-skills, resolve-cli, session-start, warehouse-guard, workspace-tools), and pytest with 65 passed.claude --plugin-dir) against a local Altimate backend with real GitHub and Jira credentials:All 19 test cases in
hooks/integrations.test.tsxKnown limitations and follow-ups
0600file, not an OS keychain. Spec AC5 asks for the keychain only; this needs libsecret and DPAPI support.Provider (from <hostname>), not asource.machinefield. Renaming the host orphans it, and fixing that needs a backend field. Keychain keys are not yet namespaced per Altimate tenant.PATCH /datamates/{id}). The CLI re-reads it just before writing, but two concurrent writers can still race until there is an incremental or conditional update.-:integrations setcan't take one, because the argument parser reads it as a flag.🤖 Generated with Claude Code