Skip to content

Commit c05e88f

Browse files
committed
Merge origin/dev: take the landed login-URL parity work
#544 landed the same login-URL copy parity this branch had locally, so the overlapping GUI files resolve to the merged version rather than replaying a second copy of the work. The remaining local commits are unrelated. Verified after the resolution: typecheck clean, GUI eslint clean, gui 329 pass / 0 fail, full suite 5046 pass / 0 fail.
2 parents 325c8f3 + 0d17425 commit c05e88f

8 files changed

Lines changed: 317 additions & 9 deletions

File tree

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
# 070 — PR #544 Codex 리뷰 P2 2건 (wp1)
2+
3+
`chatgpt-codex-connector``171885ec29`에 남긴 인라인 지적 둘. 둘 다
4+
`useCopyFeedback` 도입으로 **새로 생긴** 결함이라 머지 전에 닫는다.
5+
6+
## P2-1 — 기기 코드 복사가 스코프를 안 쓴다
7+
8+
`gui/src/components/provider-workspace/ProviderAuthPanel.tsx`
9+
10+
```tsx
11+
const deviceCodeCopy = useCopyFeedback(); // K = void
12+
const deviceCodeOutcome = deviceCodeCopy.outcomeFor(undefined);
13+
onClick={() => deviceCodeCopy.copy(hintForThis.deviceCode ?? "", undefined)}
14+
```
15+
16+
훅은 "피드백이 자기 스코프를 들고 다녀 스코프가 바뀌면 idle로 읽힌다"는
17+
계약으로 만들었는데, 세 소비처 중 여기만 그 계약을 쓰지 않는다. URL은
18+
`url`을, doctor는 `account.id`를 스코프로 넘긴다. 기기 코드만 `undefined`다.
19+
20+
### 재현
21+
22+
1. 기기 코드 A가 뜬 상태에서 복사 → 라벨이 "코드 복사됨".
23+
2. 2.5초 안에 취소를 누른다. `cancelLoginOAuth`
24+
`setLoginInfo(current => current?.provider === provider ? null : current)`
25+
로 힌트만 비운다(`use-providers-oauth.ts:53`). **패널은 언마운트되지 않는다**
26+
`busy && hintForThis` 블록만 사라진다.
27+
3. 다시 로그인하면 `loginOAuth`가 새 코드 B로 `setLoginInfo`한다(`:84`).
28+
같은 컴포넌트 인스턴스, 같은 `undefined` 스코프.
29+
4. 코드 B 위에 A의 "복사됨"이 남는다. 클립보드에는 A가 들어 있다.
30+
31+
`Providers.tsx:210``key={item.name}`은 프로바이더가 바뀔 때만 리마운트하므로
32+
같은 프로바이더 재로그인에서는 보호가 없다. 이건 `LoginUrlBlock`이 이미
33+
막아둔 것과 **동일한 거짓 성공**이다.
34+
35+
### 수정
36+
37+
```tsx
38+
const deviceCodeCopy = useCopyFeedback<string>();
39+
const deviceCode = hintForThis?.deviceCode ?? "";
40+
const deviceCodeOutcome = deviceCodeCopy.outcomeFor(deviceCode);
41+
onClick={() => deviceCodeCopy.copy(deviceCode, deviceCode)}
42+
```
43+
44+
복사 대상 문자열이 곧 스코프다 — URL 복사와 같은 형태(`copy(url, url)`).
45+
46+
## P2-2 — 겹친 복사에서 오래된 완료가 최신 결과를 덮어쓴다
47+
48+
`gui/src/components/use-copy-feedback.ts:38-47`
49+
50+
```ts
51+
void copyTextToClipboard(text).then((ok) => {
52+
clearTimer();
53+
setFeedback({ scope, outcome: ok ? "copied" : "unavailable" });
54+
timerRef.current = setTimeout(...);
55+
});
56+
```
57+
58+
`copyTextToClipboard`는 async다. `navigator.clipboard.writeText`가 권한
59+
프롬프트나 포커스 대기로 지연되면 첫 시도가 둘째보다 늦게 resolve될 수 있다.
60+
그때 첫 시도의 `.then`이 무조건 상태와 타이머를 덮어쓴다.
61+
62+
- 스코프가 다르면(예: 코드 A→B) 현재 버튼이 남의 스코프 피드백을 받아 idle이 된다.
63+
- 스코프가 같으면 오래된 결과를 최신인 양 보고한다(A 실패→B 성공이면 "사용 불가").
64+
- 타이머도 늦은 쪽이 다시 걸어 피드백 수명이 어긋난다.
65+
66+
`clearTimer`는 이 경합을 못 막는다. 순서 문제이지 타이머 문제가 아니다.
67+
68+
### 수정
69+
70+
요청 세대 카운터를 둔다.
71+
72+
```ts
73+
const generationRef = useRef(0);
74+
75+
const copy = useCallback((text: string, scope: Scope) => {
76+
const generation = ++generationRef.current;
77+
void copyTextToClipboard(text).then((ok) => {
78+
if (generationRef.current !== generation) return; // 스테일 완료는 버린다
79+
clearTimer();
80+
setFeedback({ scope, outcome: ok ? "copied" : "unavailable" });
81+
timerRef.current = setTimeout(() => { ... }, FEEDBACK_MS);
82+
});
83+
}, [clearTimer]);
84+
```
85+
86+
만료 타이머도 자기 세대를 확인하게 해, 늦게 도착한 클릭이 앞 타이머의
87+
만료로 지워지지 않도록 한다. 이 저장소의 다른 경합 가드와 같은 방식이다
88+
(`use-providers-oauth.ts``oauthLoginGenerationRef`,
89+
`use-add-codex-account-oauth.ts``pollSession`).
90+
91+
## 회귀 테스트
92+
93+
### `gui/tests/use-copy-feedback-race.test.tsx` (신규)
94+
95+
클립보드 `writeText`를 테스트가 붙잡았다 놓는 스텁으로 순서를 고정한다.
96+
97+
1. **늦게 끝난 오래된 시도는 무시된다.** A를 클릭(보류) → B를 클릭(즉시 성공)
98+
→ A를 성공으로 해제. 라벨은 B의 결과를 유지한다.
99+
2. **결과가 갈려도 마찬가지.** A는 실패, B는 성공으로 두고 A를 나중에 해제해도
100+
"복사됨"이 남는다.
101+
3. **정상 순서는 그대로 동작한다.** 겹치지 않으면 마지막 클릭 결과가 뜬다.
102+
103+
### `gui/tests/provider-auth-device-code-copy.test.tsx` (기존에 추가)
104+
105+
1. **기기 코드가 바뀌면 라벨이 초기화된다.** 코드 A 복사 → 같은 패널에
106+
코드 B로 리렌더 → 라벨이 `prov.copyCode`로 돌아온다.
107+
108+
## 검증
109+
110+
- `bun run typecheck` exit 0, `cd gui && bun x tsc -b` exit 0
111+
- `cd gui && bun run lint` / `lint:i18n` exit 0
112+
- `cd gui && bun test tests` 전건 통과
113+
- `bun run test` (루트) 신규 실패 0, `bun run privacy:scan` 통과
114+
- 스코프를 `undefined`로 되돌리면 기기 코드 테스트가, 세대 가드를 지우면 경합 테스트가 실패한다

gui/src/components/codex-account-pool-cards.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export function CodexAccountPoolCards({
8383
)}
8484
{onCopyDoctor && oauthHealthShowsDoctor(healthStatus) && (
8585
<button type="button" className="btn btn-ghost btn-sm" onClick={() => onCopyDoctor(a.id)}>
86-
{doctorCopyButtonLabel(t, doctorCopyOutcomeFor?.(a.id))}
86+
<span aria-live="polite">{doctorCopyButtonLabel(t, doctorCopyOutcomeFor?.(a.id))}</span>
8787
</button>
8888
)}
8989
<button type="button" className="btn btn-ghost btn-sm" onClick={() => void onEditAlias(a)}>

gui/src/components/codex-account-pool-main-card.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export function CodexAccountPoolMainCard({
7979
)}
8080
{onCopyDoctor && oauthHealthShowsDoctor(main?.health?.status) && (
8181
<button type="button" className="btn btn-ghost btn-sm" onClick={() => onCopyDoctor(mainId)}>
82-
{doctorCopyButtonLabel(t, doctorCopyOutcomeFor?.(mainId))}
82+
<span aria-live="polite">{doctorCopyButtonLabel(t, doctorCopyOutcomeFor?.(mainId))}</span>
8383
</button>
8484
)}
8585
<span className="card-right"><IconLock width={14} /> {t("codexAuth.appLogin")}</span>

gui/src/components/provider-workspace/ProviderAuthPanel.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export default function ProviderAuthPanel({
4949
const [addingKey, setAddingKey] = useState(false);
5050
const [newKey, setNewKey] = useState("");
5151
const [keyBusy, setKeyBusy] = useState(false);
52-
const deviceCodeCopy = useCopyFeedback();
52+
const deviceCodeCopy = useCopyFeedback<string>();
5353
const doctorCopy = useCopyFeedback<string>();
5454

5555
const surface = providerAuthSurface({ ...item, hasApiKey: item.hasApiKey || keys.length > 0 });
@@ -75,7 +75,8 @@ export default function ProviderAuthPanel({
7575
if (!surface || !authHandlers) return null;
7676

7777
const hintForThis = loginHint?.provider === item.name ? loginHint : null;
78-
const deviceCodeOutcome = deviceCodeCopy.outcomeFor(undefined);
78+
const deviceCode = hintForThis?.deviceCode ?? "";
79+
const deviceCodeOutcome = deviceCodeCopy.outcomeFor(deviceCode);
7980
const deviceCodeCopyLabel = deviceCodeOutcome === "copied"
8081
? t("prov.codeCopied")
8182
: deviceCodeOutcome === "unavailable"
@@ -136,7 +137,7 @@ export default function ProviderAuthPanel({
136137
<span>{t("prov.deviceCode")}</span>
137138
<code className="pwi-device-code">{hintForThis.deviceCode}</code>
138139
<button type="button" className="btn btn-primary btn-sm"
139-
onClick={() => deviceCodeCopy.copy(hintForThis.deviceCode ?? "", undefined)}>
140+
onClick={() => deviceCodeCopy.copy(deviceCode, deviceCode)}>
140141
<span aria-live="polite">{deviceCodeCopyLabel}</span>
141142
</button>
142143
</div>
@@ -216,7 +217,7 @@ export default function ProviderAuthPanel({
216217
)}
217218
{showDoctor && (
218219
<button type="button" className="btn btn-ghost btn-sm" onClick={copyDoctor}>
219-
{doctorCopyButtonLabel(t, doctorCopy.outcomeFor(account.id))}
220+
<span aria-live="polite">{doctorCopyButtonLabel(t, doctorCopy.outcomeFor(account.id))}</span>
220221
</button>
221222
)}
222223
<button type="button" className="btn btn-ghost btn-sm"

gui/src/components/use-copy-feedback.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export function useCopyFeedback<Scope = void>(): {
2020
} {
2121
const [feedback, setFeedback] = useState<{ scope: Scope; outcome: CopyOutcome } | null>(null);
2222
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
23+
const generationRef = useRef(0);
2324

2425
const clearTimer = useCallback(() => {
2526
if (timerRef.current) {
@@ -36,11 +37,18 @@ export function useCopyFeedback<Scope = void>(): {
3637
);
3738

3839
const copy = useCallback((text: string, scope: Scope) => {
40+
// Writes settle out of order: a permission prompt can delay the first
41+
// attempt past a second one. Without a generation the older completion
42+
// overwrites the newer click's result — and its timer expires the wrong
43+
// feedback. Same guard shape the OAuth polling paths already use.
44+
const generation = ++generationRef.current;
3945
void copyTextToClipboard(text).then((ok) => {
46+
if (generationRef.current !== generation) return;
4047
clearTimer();
4148
setFeedback({ scope, outcome: ok ? "copied" : "unavailable" });
4249
timerRef.current = setTimeout(() => {
4350
timerRef.current = null;
51+
if (generationRef.current !== generation) return;
4452
setFeedback(null);
4553
}, FEEDBACK_MS);
4654
});

gui/tests/add-codex-account-login-url.test.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ let root: Root | null = null;
2222
let originalFetch: typeof globalThis.fetch;
2323
let clipboardWrites: string[] = [];
2424
let statusHolders: Array<{ resolve: (value: Response) => void }> = [];
25+
let loginHolders: Array<{ resolve: () => void }> = [];
2526

2627
function installClipboard(available: boolean) {
2728
clipboardWrites = [];
@@ -48,12 +49,16 @@ beforeEach(() => {
4849

4950
originalFetch = globalThis.fetch;
5051
statusHolders = [];
52+
loginHolders = [];
5153
Object.defineProperty(globalThis, "fetch", {
5254
configurable: true,
5355
value: async (input: RequestInfo | URL) => {
5456
const url = new URL(String(input), "http://localhost");
5557
if (url.pathname === "/api/codex-auth/login") {
56-
return Response.json({ url: AUTH_URL, flowId: "flow-1" });
58+
// Held so the test decides when the URL lands, instead of racing a timer.
59+
return await new Promise<Response>((resolve) => {
60+
loginHolders.push({ resolve: () => resolve(Response.json({ url: AUTH_URL, flowId: "flow-1" })) });
61+
});
5762
}
5863
if (url.pathname === "/api/codex-auth/login-status") {
5964
return await new Promise<Response>((resolve) => { statusHolders.push({ resolve }); });
@@ -67,6 +72,7 @@ beforeEach(() => {
6772
});
6873

6974
afterEach(async () => {
75+
for (const holder of loginHolders.splice(0)) holder.resolve();
7076
for (const holder of statusHolders.splice(0)) {
7177
holder.resolve(Response.json({ status: "pending" }));
7278
}
@@ -83,7 +89,11 @@ afterEach(async () => {
8389
await win.happyDOM?.close?.();
8490
});
8591

86-
/** Reauth enters the waiting step immediately, but authUrl arrives a tick later. */
92+
/**
93+
* Reauth enters the waiting step immediately, but authUrl arrives with the
94+
* login response. Release that response inside act() so the URL render is
95+
* flushed before any assertion — a fixed delay would race a slow worker.
96+
*/
8797
async function mountReauthModal() {
8898
const { createRoot } = await import("react-dom/client");
8999
await act(async () => {
@@ -94,7 +104,11 @@ async function mountReauthModal() {
94104
</LanguageProvider>,
95105
);
96106
});
97-
await act(async () => { await new Promise((r) => setTimeout(r, 40)); });
107+
await act(async () => {
108+
while (loginHolders.length === 0) await new Promise((r) => setTimeout(r, 0));
109+
for (const holder of loginHolders.splice(0)) holder.resolve();
110+
await new Promise((r) => setTimeout(r, 0));
111+
});
98112
}
99113

100114
function copyButton(): HTMLButtonElement {

gui/tests/provider-auth-device-code-copy.test.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,21 @@ test("reports an unusable clipboard instead of dying silently", async () => {
154154
expect(host.textContent).toContain(DEVICE_CODE);
155155
});
156156

157+
test("a new device code does not inherit the previous code's feedback", async () => {
158+
await mountPanel({ provider: "claude", deviceCode: DEVICE_CODE });
159+
await clickCopy();
160+
expect(host.textContent).toContain("Code copied");
161+
162+
// Cancel + restart replaces loginInfo while the panel stays mounted, so the
163+
// next code would otherwise show a copy it never received.
164+
await mountPanel({ provider: "claude", deviceCode: "QRST-UVWX" });
165+
166+
expect(host.textContent).toContain("QRST-UVWX");
167+
expect(host.textContent).not.toContain(DEVICE_CODE);
168+
expect(host.textContent).not.toContain("Code copied");
169+
expect(host.textContent).toContain("Copy code");
170+
});
171+
157172
test("keeps the latest feedback for its full window across repeated copies", async () => {
158173
await mountPanel({ provider: "claude", deviceCode: DEVICE_CODE });
159174

0 commit comments

Comments
 (0)