fix: address review findings in auth flow

src/cli.ts

@@ -6,6 +6,7 @@ import {
   copyFileSync,
   rmSync,
 } from "node:fs";
+import { loadCredentials } from "./services/auth.js";
 import { join, dirname } from "node:path";
 import { homedir } from "node:os";
 import { fileURLToPath } from "node:url";
@@ -296,7 +297,15 @@ function uninstall() {
 }
 
 function status() {
-  const apiKey = process.env.SUPERMEMORY_CODEX_API_KEY;
+  const envApiKey = process.env.SUPERMEMORY_CODEX_API_KEY;
+  const credentialsApiKey = !envApiKey ? loadCredentials() : undefined;
+  const apiKey = envApiKey || credentialsApiKey;
+  const apiKeySource = envApiKey
+    ? "SUPERMEMORY_CODEX_API_KEY env var"
+    : credentialsApiKey
+    ? "credentials file (~/.codex/supermemory/credentials.json)"
+    : null;
+
   const hooksInstalled = existsSync(RECALL_SCRIPT) && existsSync(CAPTURE_SCRIPT);
   const hooksJsonExists = existsSync(CODEX_HOOKS_JSON);
   const configTomlExists = existsSync(CODEX_CONFIG_TOML);
@@ -325,7 +334,7 @@ function status() {
   );
 
   console.log("codex-supermemory status:\n");
-  console.log(`  API key:       ${apiKey ? "✓ set (SUPERMEMORY_CODEX_API_KEY)" : "✗ not set"}`);
+  console.log(`  API key:       ${apiKey ? `✓ set (${apiKeySource})` : "✗ not set"}`);
   console.log(`  Hook scripts:  ${hooksInstalled ? `✓ installed at ${SUPERMEMORY_HOOKS_DIR}` : "✗ not installed"}`);
   console.log(`  hooks.json:    ${hooksEnabled ? "✓ registered (implicit memory)" : "✗ not registered"}`);
   console.log(`  Skills:        ${skillsInstalled ? `✓ installed (${SKILLS.map(s => s.name).join(", ")})` : "✗ not installed"}`);

src/hooks/recall.ts

@@ -1,5 +1,5 @@
-import { readFileSync, existsSync, writeFileSync, unlinkSync } from "node:fs";
-import { join } from "node:path";
+import { readFileSync, existsSync, writeFileSync, unlinkSync, mkdirSync } from "node:fs";
+import { join, dirname } from "node:path";
 import { homedir } from "node:os";
 import { isConfigured, CONFIG, reloadApiKey } from "../config.js";
 import { SupermemoryClient } from "../services/client.js";
@@ -46,6 +46,7 @@ async function main() {
 
     if (!alreadyAttempted) {
       try {
+        mkdirSync(dirname(AUTH_ATTEMPTED_FILE), { recursive: true });
         writeFileSync(AUTH_ATTEMPTED_FILE, new Date().toISOString());
       } catch {}
 

src/services/auth.ts

@@ -3,6 +3,7 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync } from "node:fs";
 import { join } from "node:path";
 import { homedir } from "node:os";
 import { execFile } from "node:child_process";
+import { randomBytes } from "node:crypto";
 
 const SUPERMEMORY_DIR = join(homedir(), ".codex", "supermemory");
 const CREDENTIALS_FILE = join(SUPERMEMORY_DIR, "credentials.json");
@@ -51,10 +52,11 @@ export function loadCredentials(): string | undefined {
 }
 
 function saveCredentials(apiKey: string): void {
-  mkdirSync(SUPERMEMORY_DIR, { recursive: true });
+  mkdirSync(SUPERMEMORY_DIR, { recursive: true, mode: 0o700 });
   writeFileSync(
     CREDENTIALS_FILE,
-    JSON.stringify({ apiKey, savedAt: new Date().toISOString() }, null, 2)
+    JSON.stringify({ apiKey, savedAt: new Date().toISOString() }, null, 2),
+    { mode: 0o600 }
   );
 }
 
@@ -72,11 +74,19 @@ function openBrowser(url: string): void {
 export function startAuthFlow(): Promise<string> {
   return new Promise((resolve, reject) => {
     let resolved = false;
+    const stateToken = randomBytes(16).toString("hex");
 
     const server = createServer((req: IncomingMessage, res: ServerResponse) => {
       const url = new URL(req.url || "/", `http://localhost:${AUTH_PORT}`);
 
       if (url.pathname === "/callback") {
+        const callbackState = url.searchParams.get("state");
+        if (callbackState !== stateToken) {
+          res.writeHead(403, { "Content-Type": "text/html" });
+          res.end(AUTH_ERROR_HTML);
+          return;
+        }
+
         const apiKey =
           url.searchParams.get("apikey") || url.searchParams.get("api_key");
 
@@ -85,6 +95,7 @@ export function startAuthFlow(): Promise<string> {
           res.writeHead(200, { "Content-Type": "text/html" });
           res.end(AUTH_SUCCESS_HTML);
           resolved = true;
+          clearTimeout(timer);
           server.close();
           resolve(apiKey);
         } else {
@@ -99,17 +110,18 @@ export function startAuthFlow(): Promise<string> {
 
     server.listen(AUTH_PORT, "127.0.0.1", () => {
       const callbackUrl = `http://localhost:${AUTH_PORT}/callback`;
-      const authUrl = `${AUTH_BASE_URL}?callback=${encodeURIComponent(callbackUrl)}&client=codex`;
+      const authUrl = `${AUTH_BASE_URL}?callback=${encodeURIComponent(callbackUrl)}&client=codex&state=${stateToken}`;
       openBrowser(authUrl);
     });
 
     server.on("error", (err) => {
       if (!resolved) {
+        clearTimeout(timer);
         reject(new Error(`Failed to start auth server: ${err.message}`));
       }
     });
 
-    setTimeout(() => {
+    const timer = setTimeout(() => {
       if (!resolved) {
         server.close();
         reject(new Error("AUTH_TIMEOUT"));

test/unit.mjs

@@ -187,7 +187,7 @@ describe("integration: install/uninstall", () => {
     assert.equal(result.status, 0, `install should exit 0: ${result.stderr}`);
 
     const skillsDir = join(codexDir, "skills");
-    for (const skillName of ["supermemory-search", "supermemory-save", "supermemory-forget"]) {
+    for (const skillName of ["supermemory-search", "supermemory-save", "supermemory-forget", "supermemory-login"]) {
       const skillMd = join(skillsDir, skillName, "SKILL.md");
       assert.ok(existsSync(skillMd), `${skillName}/SKILL.md should exist`);
       const content = readFileSync(skillMd, "utf-8");
@@ -207,7 +207,7 @@ describe("integration: install/uninstall", () => {
     assert.equal(uninstallResult.status, 0, `uninstall should exit 0: ${uninstallResult.stderr}`);
 
     const skillsDir = join(codexDir, "skills");
-    for (const skillName of ["supermemory-search", "supermemory-save", "supermemory-forget"]) {
+    for (const skillName of ["supermemory-search", "supermemory-save", "supermemory-forget", "supermemory-login"]) {
       assert.ok(
         !existsSync(join(skillsDir, skillName)),
         `${skillName} skill dir should be removed`
@@ -236,12 +236,26 @@ describe("integration: install/uninstall", () => {
 describe("recall hook output envelope", () => {
   const recallBin = new URL("../dist/hooks/recall.js", import.meta.url).pathname;
 
-  test("outputs hookSpecificOutput envelope when not configured", () => {
-    const result = spawnSync("node", [recallBin], {
-      input: JSON.stringify({ session_id: "s1", prompt: "hello" }),
-      env: { ...process.env, SUPERMEMORY_CODEX_API_KEY: "" },
+  // Helper: run recall hook with an isolated HOME so the .auth-attempted file is
+  // writable. Without this, the recall hook cannot persist the "already attempted"
+  // marker (directory doesn't exist), causing every no-key invocation to spin up a
+  // 25-second browser auth flow and timing out — making tests extremely slow.
+  function runRecallUnconfigured(t, input) {
+    const tmpDir = makeTmpDir();
+    mkdirSync(join(tmpDir, ".codex", "supermemory"), { recursive: true });
+    t.after(() => rmSync(tmpDir, { recursive: true, force: true }));
+    return spawnSync("node", [recallBin], {
+      input,
+      env: { ...process.env, HOME: tmpDir, SUPERMEMORY_CODEX_API_KEY: "" },
       encoding: "utf-8",
+      // Generous timeout: first invocation writes .auth-attempted then times out
+      // after AUTH_TIMEOUT (25s). Set timeout slightly above that.
+      timeout: 30_000,
     });
+  }
+
+  test("outputs hookSpecificOutput envelope when not configured", (t) => {
+    const result = runRecallUnconfigured(t, JSON.stringify({ session_id: "s1", prompt: "hello" }));
     const parsed = JSON.parse(result.stdout);
     assert.ok("hookSpecificOutput" in parsed, "must have hookSpecificOutput key");
     assert.equal(parsed.hookSpecificOutput.hookEventName, "UserPromptSubmit");
@@ -269,20 +283,35 @@ describe("recall hook output envelope", () => {
     assert.equal(parsed.hookSpecificOutput.hookEventName, "UserPromptSubmit");
   });
 
-  test("never outputs bare additionalContext at top level (old wrong shape)", () => {
+  test("never outputs bare additionalContext at top level (old wrong shape)", (t) => {
+    // When .auth-attempted already exists (second invocation), the hook exits quickly.
+    // Create it ahead of time so this test doesn't incur the 25s auth timeout.
+    const tmpDir = makeTmpDir();
+    const supermemoryDir = join(tmpDir, ".codex", "supermemory");
+    mkdirSync(supermemoryDir, { recursive: true });
+    writeFileSync(join(supermemoryDir, ".auth-attempted"), new Date().toISOString());
+    t.after(() => rmSync(tmpDir, { recursive: true, force: true }));
+
     const result = spawnSync("node", [recallBin], {
       input: JSON.stringify({ prompt: "test" }),
-      env: { ...process.env, SUPERMEMORY_CODEX_API_KEY: "" },
+      env: { ...process.env, HOME: tmpDir, SUPERMEMORY_CODEX_API_KEY: "" },
       encoding: "utf-8",
     });
     const parsed = JSON.parse(result.stdout);
     assert.ok(!("additionalContext" in parsed), "must NOT have top-level additionalContext");
   });
 
-  test("exits with code 0", () => {
+  test("exits with code 0", (t) => {
+    // Pre-create .auth-attempted so the hook returns quickly without the 25s timeout.
+    const tmpDir = makeTmpDir();
+    const supermemoryDir = join(tmpDir, ".codex", "supermemory");
+    mkdirSync(supermemoryDir, { recursive: true });
+    writeFileSync(join(supermemoryDir, ".auth-attempted"), new Date().toISOString());
+    t.after(() => rmSync(tmpDir, { recursive: true, force: true }));
+
     const result = spawnSync("node", [recallBin], {
       input: JSON.stringify({ prompt: "test" }),
-      env: { ...process.env, SUPERMEMORY_CODEX_API_KEY: "" },
+      env: { ...process.env, HOME: tmpDir, SUPERMEMORY_CODEX_API_KEY: "" },
       encoding: "utf-8",
     });
     assert.equal(result.status, 0);
@@ -393,23 +422,23 @@ describe("skill scripts: search/save/forget", () => {
     });
   }
 
-  test("search-memory prints not-configured message and exits 0 when no API key", (t) => {
+  test("search-memory prints not-configured message and exits 1 when no API key", (t) => {
     const result = runSkillUnconfigured(t, searchBin, ["hello"]);
-    assert.equal(result.status, 0);
-    assert.match(result.stdout, /Supermemory API key not configured/);
-    assert.match(result.stdout, /SUPERMEMORY_CODEX_API_KEY/);
+    assert.equal(result.status, 1);
+    assert.match(result.stderr, /Supermemory is not authenticated/);
+    assert.match(result.stderr, /supermemory-login/);
   });
 
-  test("save-memory prints not-configured message and exits 0 when no API key", (t) => {
+  test("save-memory prints not-configured message and exits 1 when no API key", (t) => {
     const result = runSkillUnconfigured(t, saveBin, ["some content"]);
-    assert.equal(result.status, 0);
-    assert.match(result.stdout, /Supermemory API key not configured/);
+    assert.equal(result.status, 1);
+    assert.match(result.stderr, /Supermemory is not authenticated/);
   });
 
-  test("forget-memory prints not-configured message and exits 0 when no API key", (t) => {
+  test("forget-memory prints not-configured message and exits 1 when no API key", (t) => {
     const result = runSkillUnconfigured(t, forgetBin, ["some content"]);
-    assert.equal(result.status, 0);
-    assert.match(result.stdout, /Supermemory API key not configured/);
+    assert.equal(result.status, 1);
+    assert.match(result.stderr, /Supermemory is not authenticated/);
   });
 
   test("search-memory prints usage and exits 0 when no query is given", (t) => {
@@ -446,10 +475,10 @@ describe("skill scripts: search/save/forget", () => {
       ["--user", "--no-profile", "find", "thing"],
     ]) {
       const result = runSkillUnconfigured(t, searchBin, args);
-      assert.equal(result.status, 0, `flags ${args.join(" ")} should exit 0`);
+      assert.equal(result.status, 1, `flags ${args.join(" ")} should exit 1 when unconfigured`);
       assert.match(
-        result.stdout,
-        /Supermemory API key not configured/,
+        result.stderr,
+        /Supermemory is not authenticated/,
         `flags ${args.join(" ")} should hit the unconfigured branch`
       );
     }