docs(devlog): record the admin-token local-UX unit - #3504
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds planning and delivery records for the admin-token local UX work. It documents authentication behavior, dialog repairs, localized token guidance, Windows test baselines, merged changes, and verification results. ChangesAdmin token local UX
Estimated code review effort: 2 (Simple) | ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
리뷰 · 우선순위 50 / 80이 PR은 이미
지금 체크아웃과 맞춰 보면, 라인 1 - 경로/030_windows_baseline.md - preload 가드 순서· 경로/000·040 - 세션 메타 예시가 경로/전체 - 제품 코드·테스트·워크플로 변경이 없고 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 200498e812
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 1. Arm `OCX_TEST_HOME_GUARD` before `acquireTestRunLock`, or make the SID lookup | ||
| fail closed instead of proceeding unguarded. Highest value: it is a safety | ||
| defect, not just a flake. |
There was a problem hiding this comment.
Remove the unfixed safety finding from tracked devlog
This publishes an explicit remediation for a defect that the document says is not fixed, after explaining how the failure leaves the test guard off and permits suites to reach the developer's live Task Scheduler. That is pre-disclosure bypass reasoning and a patch plan for an unfixed fail-open safety issue; keep these details in .tmp/ until the fix ships, then publish the closed outcome.
AGENTS.md reference: AGENTS.md:L103-L110
Useful? React with 👍 / 👎.
| @@ -0,0 +1,64 @@ | |||
| # 040 — Delivery record | |||
|
|
|||
| Work-phase `wp4`. All three PRs merged to `dev` with admin authority. | |||
There was a problem hiding this comment.
Move the completed unit to
_fin
This delivery record says every phase PR merged and the associated issues closed, so the unit has a terminal outcome rather than remaining open. Keeping the directory under _plan incorrectly advertises completed work as active; move the closeout record to _fin and split any genuinely open follow-up into its own plan.
AGENTS.md reference: AGENTS.md:L83-L86
Useful? React with 👍 / 👎.
| The rule: **the admin-token prompt is for a deployment that actually requires a | ||
| typed credential.** That is the non-loopback bind, which is the `hub` role. Any | ||
| other role — `standalone`, `client`, or an absent tag — must not prompt. |
There was a problem hiding this comment.
Replace the rejected role-only authentication design
The shipped gui/src/api-targets.ts:51-60 consumes opencodex-management-auth-required; a non-loopback standalone must prompt, while a loopback hub can mint a session. 040_delivery_record.md:58-64 also identifies this role-only rule as the rejected first implementation. Presenting it here as the phase's rule, code, and test plan makes the new record internally contradictory and gives future maintainers the wrong authentication invariant; rewrite this section around bind metadata or clearly mark the design as superseded.
Useful? React with 👍 / 👎.
Records PR #3507 (`663fdbb0a`) as the closure of carry-forward item 1 from the Windows baseline triage, and adds `031` explaining why that fix mattered independently of the failure count it removed. The count was the wrong headline. 161 red tests were the alarm; the damage was the handful that went green by doing something to the developer's machine, because the guard those suites rely on to refuse live elevation and Task Scheduler mutation had been skipped. A luckier run would have shown fewer failures and made the same writes. Also records what the plan audit changed — arm-then-lock became sandbox → arm → lock, because the first still leaves a worker able to read the real home — and the delivery-record rows for #3504 and #3507.
…ix (#3514) Records PR #3507 (`663fdbb0a`) as the closure of carry-forward item 1 from the Windows baseline triage, and adds `031` explaining why that fix mattered independently of the failure count it removed. The count was the wrong headline. 161 red tests were the alarm; the damage was the handful that went green by doing something to the developer's machine, because the guard those suites rely on to refuse live elevation and Task Scheduler mutation had been skipped. A luckier run would have shown fewer failures and made the same writes. Also records what the plan audit changed — arm-then-lock became sandbox → arm → lock, because the first still leaves a worker able to read the real home — and the delivery-record rows for #3504 and #3507. Co-authored-by: jun <jun@lidge.dev>
Summary
Records the devlog unit for the admin-token local-UX work merged in #3491, #3496, and #3493. Documentation only — no runtime, test, or build path reads from
devlog/.Five documents:
resolveTokenAfter401treated any definitive session refusal as "ask the human", but on loopback the only refusals are Host/Origin mismatches that no token can repair.isApiAuthRequired) rather thanruntimeRole.Verification
bun run privacy:scan— passed.tests/repo-hygiene.test.tsconstraints respected: no gitlink, no vendored reference clones, no pre-disclosure security material (every defect described here is already public in merged diffs, which is theAGENTS.mdtest for what may live indevlog/).Checklist
devSummary by CodeRabbit