Skip to content

feat(cli): add get-token subcommand with auto-capture via volume mount#331

Open
aliasunder wants to merge 1 commit into
mainfrom
worktree-cli-get-token
Open

feat(cli): add get-token subcommand with auto-capture via volume mount#331
aliasunder wants to merge 1 commit into
mainfrom
worktree-cli-get-token

Conversation

@aliasunder

@aliasunder aliasunder commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add npx vault-cortex get-token as a standalone subcommand that auto-captures the Obsidian Sync auth token via a Docker volume mount — no more manual paste
  • Rewire init --mode remote to auto-capture when Docker is available, with paste prompt as fallback
  • Remove the old runGetToken (stdio-inherit, uncapturable) from the DockerRunner interface

How it works

  1. Creates a host temp dir via mkdtempSync
  2. Runs docker run --rm -it --entrypoint get-token -v <tempdir>:/home/obsidian/.config <image> — the interactive login still shows in the terminal
  3. Reads the auth token from <tempdir>/obsidian-headless/auth_token
  4. Cleans up the temp dir in a finally block

On Linux, passes --user uid:gid so the token file is host-user-owned (macOS Docker Desktop translates UIDs automatically).

Subcommand UX

  • npx vault-cortex get-token — run interactively, print token to stdout
  • npx vault-cortex get-token --dir ./vault-cortex — run interactively, write token directly to .env

Files changed (17)

File Change
cli/src/get-token.ts NewcaptureObsidianToken (shared) + runGetToken (subcommand entry)
cli/src/docker.ts Add buildGetTokenArgs (pure) + runGetTokenWithMount; remove old runGetToken
cli/src/scaffold.ts Add patchEnvObsidianToken for .env patching
cli/src/init.ts Rewire remote flow: auto-capture → paste fallback; remove GET_TOKEN_COMMAND
cli/src/program.ts Wire get-token subcommand with --dir flag
cli/src/main.ts Inject deps for runGetToken
cli/src/env.ts Update fill-in comment to npx vault-cortex get-token
cli/README.md Add Get Token section
deploy/remote/README.md Add CLI method to step 3
AGENTS.md Add get-token.ts to structure tree
cli/src/__tests__/get-token.test.ts New — 14 tests (capture + subcommand)
6 test files Update DockerRunner stubs + remote flow tests

Test plan

  • npm run build:cli — no type errors
  • npm test — 1854 tests pass (57 test files)
  • Pre-commit hooks pass (tsc, eslint, prettier)
  • Manual: npx vault-cortex get-token — interactive login, token printed
  • Manual: npx vault-cortex get-token --dir ./vault-cortex — token written to .env
  • Manual: npx vault-cortex init --mode remote — token auto-captured, no paste
  • Manual: init remote with Docker down — paste prompt fallback works

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added the get-token command to capture an Obsidian Sync authentication token.
    • Tokens can be printed directly or written to an existing .env file using --dir.
    • Remote initialization now automatically captures tokens when Docker is available, with a manual fallback.
  • Documentation

    • Updated CLI and remote setup guides with token-generation instructions.

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>
@umm-actually

umm-actually Bot commented Jul 14, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Fix stale user message about token printing

The prompts.log message claims get-token will "print a token at the end," but the
new auto-capture design writes the token to a file instead. This stale copy misleads
the user — they might scroll up expecting to see a printed token that never appears.
Update the message to describe the volume-mount capture behavior so the user knows
the token is automatically captured without any terminal output to look for.

cli/src/get-token.ts [27-53]

 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.",
+        "account login. The token will be captured automatically.",
     )
     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()
     return token || undefined
   } finally {
     rmSync(configMountPath, { recursive: true, force: true })
   }
 }
Suggestion importance[1-10]: 5

__

Why: The suggestion updates a log message to better describe the new auto-capture behavior, improving clarity for the user. It is a minor enhancement, not a critical fix.

Low

Comment thread cli/src/get-token.ts
Comment on lines +27 to +53
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()
return token || undefined
} finally {
rmSync(configMountPath, { recursive: true, force: true })
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The prompts.log message claims get-token will "print a token at the end," but the new auto-capture design writes the token to a file instead. This stale copy misleads the user — they might scroll up expecting to see a printed token that never appears. Update the message to describe the volume-mount capture behavior so the user knows the token is automatically captured without any terminal output to look for. [general, importance: 5]

Suggested change
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()
return token || undefined
} finally {
rmSync(configMountPath, { recursive: true, force: true })
}
}
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. The token will be captured automatically.",
)
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()
return token || undefined
} finally {
rmSync(configMountPath, { recursive: true, force: true })
}
}

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread cli/src/get-token.ts
Comment on lines +46 to +49
const tokenPath = join(configMountPath, "obsidian-headless", "auth_token")
if (!existsSync(tokenPath)) return undefined
const token = readFileSync(tokenPath, "utf8").trim()
return token || undefined

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Suggested change
const tokenPath = join(configMountPath, "obsidian-headless", "auth_token")
if (!existsSync(tokenPath)) return undefined
const token = readFileSync(tokenPath, "utf8").trim()
return token || undefined
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

Comment thread cli/src/docker.ts
Comment on lines +47 to +56
export const buildGetTokenArgs = (params: GetTokenArgParams): string[] => {
const { configMountPath, platform = process.platform, uid, gid } = params

const args = [
"run",
"--rm",
"-it",
"--entrypoint",
"get-token",
"-v",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@aliasunder

Copy link
Copy Markdown
Owner Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a get-token CLI command that captures Obsidian Sync tokens through a mounted Docker workflow, prints or stores them in .env, integrates capture into remote initialization, and updates related tests, documentation, and project structure.

Changes

Obsidian token capture workflow

Layer / File(s) Summary
Mounted Docker token capture
cli/src/docker.ts, cli/src/get-token.ts, cli/src/__tests__/docker.test.ts, cli/src/__tests__/get-token.test.ts, cli/src/__tests__/upgrade.test.ts
Docker token execution now mounts a temporary directory, supports Linux user mapping, and reads the trimmed auth_token output with cleanup and failure handling.
CLI command and .env persistence
cli/src/program.ts, cli/src/main.ts, cli/src/get-token.ts, cli/src/scaffold.ts, cli/src/__tests__/program.test.ts, cli/src/__tests__/scaffold.test.ts, cli/src/__tests__/get-token.test.ts
Adds get-token --dir, command wiring, stdout output, and replacement of existing OBSIDIAN_AUTH_TOKEN entries in .env.
Remote init auto-capture and fallback
cli/src/init.ts, cli/src/__tests__/init.test.ts
Remote initialization attempts automatic capture and uses the paste prompt when capture is declined or fails.
Token guidance and documentation
AGENTS.md, cli/README.md, cli/src/env.ts, cli/src/__tests__/env.test.ts, deploy/remote/README.md
Updates CLI structure, token-generation guidance, remote quickstart instructions, and related assertions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • aliasunder/vault-cortex#89: Introduced the earlier remote initialization and Docker get-token workflow that this change extends.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: a new CLI get-token subcommand with Docker volume-mount auto-capture.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-cli-get-token

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
cli/src/__tests__/init.test.ts (1)

617-618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the parsed environment value exactly.

toContain can pass for a commented, duplicated, or malformed entry. Parse the generated .env and assert that OBSIDIAN_AUTH_TOKEN equals "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 value

Prefer 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

📥 Commits

Reviewing files that changed from the base of the PR and between 52a9c95 and 47961fb.

📒 Files selected for processing (17)
  • AGENTS.md
  • cli/README.md
  • cli/src/__tests__/docker.test.ts
  • cli/src/__tests__/env.test.ts
  • cli/src/__tests__/get-token.test.ts
  • cli/src/__tests__/init.test.ts
  • cli/src/__tests__/program.test.ts
  • cli/src/__tests__/scaffold.test.ts
  • cli/src/__tests__/upgrade.test.ts
  • cli/src/docker.ts
  • cli/src/env.ts
  • cli/src/get-token.ts
  • cli/src/init.ts
  • cli/src/main.ts
  • cli/src/program.ts
  • cli/src/scaffold.ts
  • deploy/remote/README.md

Comment thread cli/src/init.ts
Comment on lines +64 to +70
const offerGetTokenCapture = async (
prompts: Prompts,
docker: DockerRunner,
): Promise<boolean> => {
const runNow = await prompts.confirm("Run the get-token command now?", true)
if (!runNow) return false
prompts.log(
"Handing the terminal to get-token — it will ask for your Obsidian " +
"account login and print a token at the end.",
)
const tokenGenerated = docker.runGetToken()
if (!tokenGenerated) {
prompts.warn(
"get-token did not complete — you can run it later and edit .env.",
)
return false
}
return true
): Promise<string | undefined> => {
const runNow = await prompts.confirm("Generate the token now?", true)
if (!runNow) return undefined
return captureObsidianToken({ docker, prompts })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Catch capture errors so the paste fallback remains reachable.

captureObsidianToken can throw from temporary-directory, Docker, read, or cleanup operations. Those failures currently abort initialization instead of returning undefined as this flow promises.

Proposed fix
   const runNow = await prompts.confirm("Generate the token now?", true)
   if (!runNow) return undefined
-  return captureObsidianToken({ docker, prompts })
+  try {
+    return captureObsidianToken({ docker, prompts })
+  } catch {
+    prompts.warn("Automatic token capture failed; please paste the token instead.")
+    return undefined
+  }
🤖 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/init.ts` around lines 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.

Comment thread cli/src/init.ts
Comment on lines +367 to +371
await prompts.text(
"Paste the Obsidian Sync token (leave blank to fill in .env later):",
{ defaultValue: "" },
)
).trim()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Use the masked prompt for this credential.

prompts.text echoes the Obsidian Sync token to the terminal and scrollback. Use prompts.password while retaining an empty response as the “fill later” option.

Proposed fix
-      await prompts.text(
+      await prompts.password(
         "Paste the Obsidian Sync token (leave blank to fill in .env later):",
-        { defaultValue: "" },
       )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await prompts.text(
"Paste the Obsidian Sync token (leave blank to fill in .env later):",
{ defaultValue: "" },
)
).trim()
await prompts.password(
"Paste the Obsidian Sync token (leave blank to fill in .env later):",
)
).trim()
🤖 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/init.ts` around lines 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.

Comment thread cli/src/scaffold.ts
/** Matches the full OBSIDIAN_AUTH_TOKEN line for replacement. */
const fullTokenLine = /^OBSIDIAN_AUTH_TOKEN=.*$/m
if (!fullTokenLine.test(content)) return false
const patched = content.replace(fullTokenLine, `OBSIDIAN_AUTH_TOKEN=${token}`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Prevent accidental special pattern replacements in replace.

If the captured token contains special replacement patterns (like $& or $1), passing it as a string to replace will cause it to be interpolated with matched substrings, potentially corrupting the token in the .env file. Using a replacer function safely treats the return value as a literal string.

🐛 Proposed fix
-  const patched = content.replace(fullTokenLine, `OBSIDIAN_AUTH_TOKEN=${token}`)
+  const patched = content.replace(fullTokenLine, () => `OBSIDIAN_AUTH_TOKEN=${token}`)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const patched = content.replace(fullTokenLine, `OBSIDIAN_AUTH_TOKEN=${token}`)
const patched = content.replace(fullTokenLine, () => `OBSIDIAN_AUTH_TOKEN=${token}`)
🤖 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/scaffold.ts` at 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.

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.

1 participant