feat(cli): add get-sync-token subcommand with auto-capture via volume mount#331
Conversation
Add `npx vault-cortex get-token` as a standalone subcommand and rewire init's remote flow to auto-capture the Obsidian Sync token via a Docker volume mount, eliminating the manual paste step. The command creates a temp directory, mounts it into the container at /home/obsidian/.config, runs the interactive login, then reads the auth_token file from the mount — no more copy-paste. On Linux, passes --user uid:gid so the token file is host-user-owned. - `get-token` (no flags): prints the captured token to stdout - `get-token --dir ./vault-cortex`: writes the token directly to .env - `init --mode remote`: auto-captures when Docker is available, falls back to paste prompt when capture fails or the user declines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
captureObsidianToken, when the token file is missing or empty you currently returnundefinedsilently; consider logging a more specific warning in those paths so users can distinguish between Docker failing vs. get-token succeeding but not producing a token. - The
patchEnvObsidianTokenregex only matches lines starting exactly withOBSIDIAN_AUTH_TOKEN=; if you ever expect leading whitespace,export OBSIDIAN_AUTH_TOKEN=..., or inline comments, you may want to broaden the pattern to avoid surprising false negatives when patching existing.envfiles.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `captureObsidianToken`, when the token file is missing or empty you currently return `undefined` silently; consider logging a more specific warning in those paths so users can distinguish between Docker failing vs. get-token succeeding but not producing a token.
- The `patchEnvObsidianToken` regex only matches lines starting exactly with `OBSIDIAN_AUTH_TOKEN=`; if you ever expect leading whitespace, `export OBSIDIAN_AUTH_TOKEN=...`, or inline comments, you may want to broaden the pattern to avoid surprising false negatives when patching existing `.env` files.
## Individual Comments
### Comment 1
<location path="cli/src/get-token.ts" line_range="46-49" />
<code_context>
+ *
+ * Returns the token string on success, undefined on any failure.
+ */
+export const captureObsidianToken = (deps: {
+ docker: DockerRunner
+ prompts: Prompts
+}): string | undefined => {
+ const { docker, prompts } = deps
+ const configMountPath = mkdtempSync(join(tmpdir(), "vault-cortex-get-token-"))
+ try {
+ prompts.log(
+ "Handing the terminal to get-token — it will ask for your Obsidian " +
+ "account login and print a token at the end.",
+ )
+ const succeeded = docker.runGetTokenWithMount(configMountPath)
+ if (!succeeded) {
+ prompts.warn(
+ "get-token did not complete — you can run it later with:\n" +
+ " npx vault-cortex get-token",
+ )
+ return undefined
+ }
+ const tokenPath = join(configMountPath, "obsidian-headless", "auth_token")
+ if (!existsSync(tokenPath)) return undefined
+ const token = readFileSync(tokenPath, "utf8").trim()
</code_context>
<issue_to_address>
**suggestion:** Surface a more specific warning when the token file is missing despite a successful Docker run.
If `runGetTokenWithMount` succeeds but `auth_token` is missing or empty, this function just returns `undefined`, and callers only see the generic "Could not capture the auth token." message. That hides the distinction between Docker errors and issues with the mounted volume/tooling. After `existsSync(tokenPath)` (and possibly after reading the file), consider logging a specific warning explaining that the token file was not found or was empty, with hints like version mismatch or unexpected config path, so users get clearer guidance.
```suggestion
const tokenPath = join(configMountPath, "obsidian-headless", "auth_token")
if (!existsSync(tokenPath)) {
prompts.warn(
[
"get-token completed, but no auth_token file was found at:",
` ${tokenPath}`,
"",
"This usually indicates an issue with the Docker volume mount or the get-token tooling,",
"such as a version mismatch or a change in the Obsidian config path.",
"",
"You can try again with:",
" npx vault-cortex get-token",
].join("\n"),
)
return undefined
}
const token = readFileSync(tokenPath, "utf8").trim()
if (!token) {
prompts.warn(
[
"get-token completed, but the captured auth token file was empty:",
` ${tokenPath}`,
"",
"This may indicate an issue with the Docker volume mount or the get-token tooling,",
"such as a version mismatch or an unexpected Obsidian configuration layout.",
"",
"You can try again with:",
" npx vault-cortex get-token",
].join("\n"),
)
return undefined
}
return token
```
</issue_to_address>
### Comment 2
<location path="cli/src/docker.ts" line_range="47-56" />
<code_context>
+ * On Linux, includes `--user uid:gid` so the token file is host-user-owned
+ * (macOS Docker Desktop translates UIDs automatically).
+ */
+export const buildGetTokenArgs = (params: GetTokenArgParams): string[] => {
+ const { configMountPath, platform = process.platform, uid, gid } = params
+
+ const args = [
+ "run",
+ "--rm",
+ "-it",
+ "--entrypoint",
+ "get-token",
+ "-v",
+ `${configMountPath}:/home/obsidian/.config`,
+ ]
+
+ if (platform === "linux" && uid !== undefined && gid !== undefined) {
+ args.push("--user", `${uid}:${gid}`)
+ }
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Clarify or tighten the `--user` handling on Linux to match the function’s documentation.
The docstring promises that on Linux we always pass `--user uid:gid` so the token file is host-user-owned, but the code only adds `--user` when both `uid` and `gid` are defined. On Linux environments where `getuid/getgid` are unavailable or return `undefined`, this diverges from the documented behavior and the file will be root-owned in the container. Either enforce the documented behavior (e.g., require uid/gid or fail fast on Linux) or adjust the docstring to describe the conditional `--user` usage so callers know when it applies.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (26)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughChangesObsidian Sync token workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
cli/src/__tests__/init.test.ts (1)
617-618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the parsed environment value exactly.
toContaincan pass for a commented, duplicated, or malformed entry. Parse the generated.envand assert thatOBSIDIAN_AUTH_TOKENequals"captured-token".As per coding guidelines, prefer exact assertions on complete deterministic values over substring checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/__tests__/init.test.ts` around lines 617 - 618, Update the .env assertion in the init test to parse the generated environment content and verify that the parsed OBSIDIAN_AUTH_TOKEN value exactly equals "captured-token". Replace the substring-based toContain check while preserving the existing readFileSync and targetDir flow.Source: Coding guidelines
cli/src/__tests__/get-token.test.ts (1)
120-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer exact assertions over substring checks. As per coding guidelines, prefer exact assertions on complete deterministic values over loose matchers like
.toContain()for string matching. Using exact assertions ensures that the full message is correct and hasn't subtly changed.
cli/src/__tests__/get-token.test.ts#L120-L120: replace.toContain("get-token did not complete")with a.toBe()exact match.cli/src/__tests__/get-token.test.ts#L176-L176: replace.toContain("Handing the terminal to get-token")with a.toBe()exact match.cli/src/__tests__/get-token.test.ts#L203-L203: replace.toContain("Container runtime not running")with a.toBe()exact match.cli/src/__tests__/get-token.test.ts#L215-L215: replace.toContain("Could not capture the auth token")with a.toBe()exact match.cli/src/__tests__/get-token.test.ts#L249-L249: replace.toContain("no OBSIDIAN_AUTH_TOKEN line")with a.toBe()exact match.cli/src/__tests__/get-token.test.ts#L265-L265: replace.toContain("the file is missing")with a.toBe()exact match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/__tests__/get-token.test.ts` at line 120, Replace the loose warning substring assertions in cli/src/__tests__/get-token.test.ts at lines 120, 176, 203, 215, 249, and 265 with exact .toBe() assertions, using the complete deterministic warning emitted by each corresponding get-token scenario.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/src/init.ts`:
- Around line 64-70: Update offerGetTokenCapture to catch errors from
captureObsidianToken and return undefined, preserving the existing confirmation
behavior and Promise<string | undefined> contract so initialization can continue
to the paste fallback.
- Around line 367-371: Update the Obsidian Sync token prompt in the init flow to
use prompts.password instead of prompts.text, while preserving the empty default
and existing trim behavior so users can still leave the value blank and fill it
in .env later.
In `@cli/src/scaffold.ts`:
- Line 102: Update the replace call in the scaffolding flow to use a replacer
function that returns the complete OBSIDIAN_AUTH_TOKEN assignment, ensuring
captured token values such as $& or $1 are written literally. Preserve the
existing fullTokenLine match and output format.
---
Nitpick comments:
In `@cli/src/__tests__/get-token.test.ts`:
- Line 120: Replace the loose warning substring assertions in
cli/src/__tests__/get-token.test.ts at lines 120, 176, 203, 215, 249, and 265
with exact .toBe() assertions, using the complete deterministic warning emitted
by each corresponding get-token scenario.
In `@cli/src/__tests__/init.test.ts`:
- Around line 617-618: Update the .env assertion in the init test to parse the
generated environment content and verify that the parsed OBSIDIAN_AUTH_TOKEN
value exactly equals "captured-token". Replace the substring-based toContain
check while preserving the existing readFileSync and targetDir flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ee381ad-6c05-482a-a4cc-a2d16466aeb6
📒 Files selected for processing (17)
AGENTS.mdcli/README.mdcli/src/__tests__/docker.test.tscli/src/__tests__/env.test.tscli/src/__tests__/get-token.test.tscli/src/__tests__/init.test.tscli/src/__tests__/program.test.tscli/src/__tests__/scaffold.test.tscli/src/__tests__/upgrade.test.tscli/src/docker.tscli/src/env.tscli/src/get-token.tscli/src/init.tscli/src/main.tscli/src/program.tscli/src/scaffold.tsdeploy/remote/README.md
…id $ pattern injection String.prototype.replace interprets $-patterns ($&, $', $$, etc.) in string replacement arguments. If an Obsidian Sync token contained a $ character, the .env write would silently corrupt it. A function replacement avoids special-pattern interpretation entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
…n test Tighten three substring assertions (toContain) to exact matches (toBe) on hardcoded error/warning messages in get-token tests. Replace partial not.toContain/toContain with full toEqual on buildGetTokenArgs Linux-no-uid test. Add regression test proving patchEnvObsidianToken handles $-pattern tokens (e.g. $&, $$) literally — mutation-verified against the Phase 1 fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
…g init The init flow prompts for confirmation before running get-token (offerGetTokenCapture calls prompts.confirm). "Runs automatically" implies no user interaction; "is offered automatically" matches the actual behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
… missing token file, mask token paste - captureObsidianToken: wrap in try/catch so any throw degrades to the paste fallback instead of aborting init (CodeRabbit) - warn specifically when get-token completes but the token file is missing or empty, instead of returning undefined silently (Sourcery) - note auto-capture in the handoff message so users know they don't need to copy the printed token (qodo) - clarify buildGetTokenArgs docstring: --user is set when uid/gid are provided, which is always the case on POSIX hosts (Sourcery) - init: use the masked password prompt for the sync-token paste so the credential doesn't echo into scrollback (CodeRabbit) - tests: exact assertions on the remaining loose toContain checks; regression tests for the new warn + throw paths Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
|
Dispositions for review-body findings without inline threads: Sourcery overall comment 1 (silent undefined in captureObsidianToken): fixed in 16f6cb2, see the inline thread reply on cli/src/get-token.ts. Sourcery overall comment 2 (broaden the patchEnvObsidianToken regex for CodeRabbit nitpick (init.test.ts exact env assertion): fixed in 16f6cb2. The test now asserts the exact CodeRabbit nitpick (get-token.test.ts loose 🔍 ship-check · pr-monitor · claude-fable-5 |
…ured token - Replace the nested try/finally + outer try/catch with a single try/catch/finally; cleanup is now best-effort, so an rmSync failure (e.g. root-owned container files) warns instead of discarding a successfully captured token - Run 'ob login' directly instead of the image's get-token script — the script only exists to locate and print the token, and the volume mount makes both unnecessary; the credential no longer echoes into terminal scrollback, and this works with already-published images (the manual --entrypoint get-token flow is unchanged) - Update the handoff message and test expectations accordingly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
…error handling Each fallible operation gets its own narrow wrapper with one explicit failure meaning: makeTempMountDir (warn + undefined), runLoginContainer (throw = failed run), readCapturedTokenFile (missing/empty/unreadable = no token), removeTempMountDir (best-effort, warn only). The main flow keeps a single bare try/finally to scope the temp dir — no catch left that can swallow unrelated errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
… readCapturedTokenFile Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
The old name was inherited from the container's get-token script, which the capture flow no longer uses, and it was ambiguous — the CLI handles two tokens (MCP auth token from init, Obsidian Sync token from this command). The new name matches the docs' "Obsidian Sync token" vocabulary. Pre-release, so no compatibility concern. Also renames the internals that still described the old flow: runGetTokenWithMount -> runObsidianLogin, buildGetTokenArgs -> buildObsidianLoginArgs, and the get-token.ts module -> get-sync-token.ts. The container script keeps its name — the manual --entrypoint get-token flow is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
…ync-token The get-token script was the largest artifact inherited from the absorbed obsidian-headless-sync-docker fork (~98% upstream expression, and the upstream chain carries no license). Rewritten from scratch in our own words and renamed to match the CLI's get-sync-token subcommand: - canonical XDG token path checked first, find as the fallback - single -s guard covers missing path, missing file, and empty file - prints one copy-paste OBSIDIAN_AUTH_TOKEN= line instead of echo banners All references updated: Dockerfile (comments + chmod), init-check-auth hint, DEPLOY.md, ARCHITECTURE.md, AGENTS.md tree, root and deploy .env.example, deploy/remote/README.md, and the docker.ts docstring. Note: images published before this release only contain the old get-token entrypoint name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
…n words init-check-auth and init-obsidian-login were the last byte-identical holdovers from the absorbed obsidian-headless-sync-docker fork (whose upstream chain carries no license). Rewritten with the same semantics: - init-check-auth: positive-first gate with early exit, grouped stderr block, and a fuller hint (generate with get-sync-token, add to .env) - init-obsidian-login: adds a stale-token hint on rejection and set -u With these plus the get-sync-token rewrite, no inherited upstream expression remains in rootfs/. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
DEPLOY.md readers run SST, so Node.js is already a prerequisite — the CLI is the natural primary path, with the docker run one-liner kept as the direct-image alternative. Matches deploy/remote/README.md's CLI-first pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
'Captured automatically' described the implementation, not the user experience. The handoff message, failure warnings, and both READMEs now say what the user sees: sign in, and vault-cortex picks up the token itself — nothing to find or copy. Internal docstrings keep the capture vocabulary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
'Captured automatically' alone didn't tell the user what actually happens to the token. Each flow now supplies its destination to the shared handoff message: init says it is stored in your .env, the bare subcommand says it is printed at the end, and --dir names the .env file it writes to. Tests assert each flow's exact message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
…alistic constant Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117h7grQjdkAMGD38vCfZGk
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Summary
npx vault-cortex get-sync-tokenas a standalone subcommand that auto-captures the Obsidian Sync auth token via a Docker volume mount — no more manual pasteinit --mode remoteto auto-capture when Docker is available, with a masked paste prompt as fallbackrunGetToken(stdio-inherit, uncapturable) from the DockerRunner interfaceHow it works
mkdtempSyncdocker run --rm -it --entrypoint ob -v <tempdir>:/home/obsidian/.config <image> login— the interactive login still shows in the terminal, but the token is never printed (it runsob logindirectly instead of the image's get-token script, so the credential stays out of terminal scrollback; the manual--entrypoint get-tokenflow is unchanged)<tempdir>/obsidian-headless/auth_tokenfinallyblock (best-effort — a cleanup failure warns instead of discarding a captured token)On Linux, passes
--user uid:gidso the token file is host-user-owned (macOS Docker Desktop translates UIDs automatically).Error handling is per-operation: each fallible step (temp dir creation, docker run, token file read, cleanup) has its own narrow wrapper with one explicit failure meaning — no catch-all.
Subcommand UX
npx vault-cortex get-sync-token— run interactively, print token to stdoutnpx vault-cortex get-sync-token --dir ./vault-cortex— run interactively, write token directly to.envThe subcommand is named
get-sync-token(notget-token) to disambiguate from the MCP auth token thatinitgenerates, and to match the docs' "Obsidian Sync token" vocabulary.Files changed
cli/src/get-sync-token.tscaptureObsidianToken(shared) +runGetSyncToken(subcommand entry) + per-operation helperscli/src/docker.tsbuildObsidianLoginArgs(pure) +runObsidianLogin; remove oldrunGetTokencli/src/scaffold.tspatchEnvObsidianTokenfor .env patching ($-pattern-safe function replacement)cli/src/init.tsprompts.password)cli/src/program.tsget-sync-tokensubcommand with--dirflagcli/src/main.tsrunGetSyncTokencli/src/env.tsnpx vault-cortex get-sync-tokencli/README.mddeploy/remote/README.mdAGENTS.mdget-sync-token.tsto structure treecli/src/__tests__/get-sync-token.test.tsTest plan
npm run build:cli— no type errorsnpm test— 1858 tests pass (57 test files)npx vault-cortex get-sync-token— interactive login, token printednpx vault-cortex get-sync-token --dir ./vault-cortex— token written to .envnpx vault-cortex init --mode remote— token auto-captured, no paste🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
get-sync-tokencommand to automatically obtain an Obsidian Sync token through Docker..envfile with--dir.Documentation
get-sync-token.