Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions apps/web/src/managedDevPc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,57 @@ describe("managed DevPC WebSocket authorization", () => {
"invalid WebSocket URL",
);
});

it("reloads once when the managed browser session must be paired again", async () => {
vi.stubEnv("VITE_DEVPC_MANAGED", "1");
vi.resetModules();
const reload = vi.fn();
const storage = new Map<string, string>();
vi.stubGlobal("window", {
location: { origin: "https://app.example.test", reload },
sessionStorage: {
getItem: (key: string) => storage.get(key) ?? null,
setItem: (key: string, value: string) => storage.set(key, value),
removeItem: (key: string) => storage.delete(key),
},
});
vi.stubGlobal(
"fetch",
vi.fn(async () => new Response(null, { status: 401 })),
);

const { prepareManagedWebSocketUrl } = await import("./managedDevPc");
await expect(prepareManagedWebSocketUrl("wss://app.example.test/ws")).rejects.toThrow(
"Refreshing the managed workspace connection.",
);
expect(reload).toHaveBeenCalledOnce();
expect(storage.size).toBe(1);

await expect(prepareManagedWebSocketUrl("wss://app.example.test/ws")).rejects.toThrow(
"could not be authorized",
);
expect(reload).toHaveBeenCalledOnce();
Comment on lines +151 to +154

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
bash -lc 'vp test run apps/web/src/managedDevPc.test.ts'

Repository: logancsack/t3code

Length of output: 195


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== node version =="
node -v || true

echo "== locate managedDevPc files =="
fd -a 'managedDevPc' . || true

echo "== git status/stat =="
git diff --stat || true
git status --short || true

echo "== inspect test and implementation candidates =="
for f in $(git ls-files | grep -E 'managedDevPc\.(test\.)?ts$|managedDevPc\.(test\.)?tsx$'); do
  echo "--- $f ($(wc -l < "$f") lines) ---"
  sed -n '1,240p' "$f"
done

Repository: logancsack/t3code

Length of output: 13313


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== package/dependency hints =="
for f in package.json pnpm-lock.yaml yarn.lock package-lock.json; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    if [ "$f" = "package.json" ]; then
      node -e "const p=require('./package.json'); console.log(JSON.stringify({scripts:p.scripts,devDependencies:p.devDependencies,dependencies:p.dependencies,vp:p.devPackages?.vp||p.devDependencies?.vp,engines:p.engines}, null, 2).slice(0,5000))" || true
    else
      sed -n '1,160p' "$f"
    fi
  fi
done

echo "== repository files mentioning sessionRecoveryReloadScheduled or prepareManagedWebSocketUrl =="
rg -n "sessionRecoveryReloadScheduled|prepareManagedWebSocketUrl|loadManagedDevPcState|ManagedDev Pc|managedDevPc" . || true

Repository: logancsack/t3code

Length of output: 13640


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

cat > /tmp/import-isolation-probe.ts <<'TS'
let sessionRecoveryReloadScheduled = false;

function scheduleManagedSessionRecovery(): boolean {
  if (sessionRecoveryReloadScheduled) return false;
  sessionRecoveryReloadScheduled = true;
  return true;
}

async function simulateFirst401(): Promise<boolean> {
  const scheduledThisCall = scheduleManagedSessionRecovery();
  reloadCallCount += 1;
  return scheduledThisCall;
}

async function simulateSecond401SameModule(): Promise<boolean> {
  const scheduledThisCall = scheduleManagedSessionRecovery();
  reloadCallCount += 1;
  return scheduledThisCall;
}

let reloadCallCount = 0;

(async () => {
  const first = await simulateFirst401();
  const second = await simulateSecond401SameModule();
  console.log(JSON.stringify({ first, second, reloadCallCount }));
})();
TS

node /tmp/import-isolation-probe.ts

Repository: logancsack/t3code

Length of output: 205


Test the cooldown and 403 behavior through sessionStorage.

The second 401 call keeps sessionRecoveryReloadScheduled, so reload() is only exercised through the in-module guard rather than the persisted devpc-managed-session-recovery-at cooldown. Add an isolated-import case that re-reads sessionStorage before the second prepareManagedWebSocketUrl() call, and add the same coverage for a 403 response.

🤖 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 `@apps/web/src/managedDevPc.test.ts` around lines 151 - 154, Extend the managed
WebSocket authorization tests around prepareManagedWebSocketUrl to isolate
module imports and re-read sessionStorage before the second 401 request,
verifying cooldown behavior through the persisted
devpc-managed-session-recovery-at value rather than the in-module guard. Add
equivalent isolated-import coverage for a 403 response, including the expected
authorization failure and reload assertion.

Source: Coding guidelines

});

it("clears the managed session recovery cooldown after authorization succeeds", async () => {
vi.stubEnv("VITE_DEVPC_MANAGED", "1");
vi.resetModules();
const storage = new Map([["devpc-managed-session-recovery-at", String(Date.now())]]);
vi.stubGlobal("window", {
location: { origin: "https://app.example.test" },
sessionStorage: {
getItem: (key: string) => storage.get(key) ?? null,
setItem: (key: string, value: string) => storage.set(key, value),
removeItem: (key: string) => storage.delete(key),
},
});
vi.stubGlobal(
"fetch",
vi.fn(async () => Response.json({ ticket: "gateway-ticket-that-is-long-enough" })),
);

const { prepareManagedWebSocketUrl } = await import("./managedDevPc");
await prepareManagedWebSocketUrl("wss://app.example.test/ws");
expect(storage.size).toBe(0);
});
});
32 changes: 32 additions & 0 deletions apps/web/src/managedDevPc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ export const isManagedDevPc = import.meta.env.VITE_DEVPC_MANAGED === "1";

const BOOTSTRAP_PATH = "/_devpc/bootstrap";
const WEBSOCKET_TICKET_PATH = "/_devpc/ws-ticket";
const SESSION_RECOVERY_KEY = "devpc-managed-session-recovery-at";
const SESSION_RECOVERY_COOLDOWN_MS = 30_000;
let sessionRecoveryReloadScheduled = false;

function updateBootstrapMessage(message: string, failed = false): void {
const root = document.getElementById("root");
Expand Down Expand Up @@ -50,6 +53,31 @@ function pairingHash(token: string): string {
return new URLSearchParams([["token", token]]).toString();
}

function clearManagedSessionRecovery(): void {
try {
window.sessionStorage.removeItem(SESSION_RECOVERY_KEY);
} catch {
// Storage can be unavailable in privacy-restricted browser contexts.
}
}

function scheduleManagedSessionRecovery(): boolean {
if (sessionRecoveryReloadScheduled) return false;
const now = Date.now();
try {
const previous = Number(window.sessionStorage.getItem(SESSION_RECOVERY_KEY) ?? 0);
if (Number.isFinite(previous) && now - previous < SESSION_RECOVERY_COOLDOWN_MS) {
return false;
}
window.sessionStorage.setItem(SESSION_RECOVERY_KEY, String(now));
} catch {
// The in-memory guard still prevents a reload loop within this document.
}
sessionRecoveryReloadScheduled = true;
window.location.reload();
Comment on lines +73 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid reload loops when session storage is unavailable

In privacy-restricted contexts where sessionStorage throws, a persistent 401/403 still reaches window.location.reload(). Each reload creates a new document and resets sessionRecoveryReloadScheduled, so the next connection attempt reloads immediately again; the in-memory guard only prevents duplicate calls before the current document unloads. Skip automatic recovery when the cooldown cannot be persisted, or use a guard that survives navigation.

Useful? React with 👍 / 👎.

return true;
}

export async function prepareManagedDevPc(): Promise<void> {
if (!isManagedDevPc) return;

Expand Down Expand Up @@ -104,8 +132,12 @@ export async function prepareManagedWebSocketUrl(socketUrl: string): Promise<str
body: "{}",
});
if (!response.ok) {
if ([401, 403].includes(response.status) && scheduleManagedSessionRecovery()) {
throw new Error("Refreshing the managed workspace connection.");
}
throw new Error("The managed workspace connection could not be authorized.");
}
clearManagedSessionRecovery();

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

Clear recovery state only after the ticket and URL pass validation.

Line 140 removes the cooldown for any 2xx response. A malformed credential or invalid WebSocket URL after a 401 then permits the next 401 to trigger another reload. Move this call to immediately before the successful return.

Proposed fix
-  clearManagedSessionRecovery();
   const payload = (await response.json()) as {
@@
   resolved.searchParams.delete("gatewayTicket");
   resolved.searchParams.set(managedSocketUrl ? "wsTicket" : "gatewayTicket", payload.ticket);
+  clearManagedSessionRecovery();
   return resolved.toString();
📝 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
clearManagedSessionRecovery();
const payload = (await response.json()) as {
...
};
resolved.searchParams.delete("gatewayTicket");
resolved.searchParams.set(managedSocketUrl ? "wsTicket" : "gatewayTicket", payload.ticket);
clearManagedSessionRecovery();
return resolved.toString();
🤖 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 `@apps/web/src/managedDevPc.ts` at line 140, Move clearManagedSessionRecovery()
in the managed session response flow so it executes only after the ticket and
WebSocket URL validations succeed, immediately before the successful return. Do
not clear recovery state for merely 2xx responses that later fail credential or
URL validation.

const payload = (await response.json()) as {
ticket?: unknown;
websocketUrl?: unknown;
Expand Down
Loading