Skip to content

Commit ebb4d55

Browse files
authored
Merge pull request #3073 from lidge-jun/codex/promote-main-2380
[WRONG BRANCH] promote dev onto main for v2.38.0
2 parents 54e2274 + a34e8b7 commit ebb4d55

192 files changed

Lines changed: 15139 additions & 749 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,11 @@ repository CI; a maintainer has to — so the gate never disproves it; a new
305305
push still resets every box. A disproved claim unticks the matching box and
306306
keeps the PR a draft.
307307
Authors with repository push permission skip the ancestry heuristic only. As with approval requirements in
308-
[`MAINTAINERS.md`](./MAINTAINERS.md), this is enforced by convention until
309-
branch protection is configured.
308+
[`MAINTAINERS.md`](./MAINTAINERS.md), the ancestry heuristic is a CI check
309+
rather than a branch rule. The branches themselves are protected: `dev`,
310+
`main`, and `preview` each carry an active ruleset requiring a reviewed pull
311+
request and blocking force-pushes and deletion, so a direct push to `dev` is
312+
rejected regardless of `--no-verify`.
310313

311314
[`MAINTAINERS.md`](./MAINTAINERS.md) is authoritative for review and merge
312315
policy (approvals, CI requirements, security review, promotion). This file

MAINTAINERS.md

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ when a maintainer steps down.
5252
a new push still resets every box. A disproved claim unticks the matching
5353
box and keeps the PR a draft.
5454
Authors with repository push permission skip the ancestry heuristic only. As
55-
with the approval requirement above, this is enforced by convention until
56-
branch protection is configured (see the note under the change log).
55+
with the approval requirement above, this part is enforced by convention;
56+
the ruleset does not check ancestry (see the note under the change log).
5757
- A pull request requires approval from at least one maintainer and successful required CI checks
5858
before merge.
5959
- Authors do not approve their own pull requests.
@@ -160,11 +160,21 @@ Adding or removing a maintainer requires:
160160
and release automation keep the two owners already listed for those paths, so
161161
this addition does not widen the review surface for them.
162162

163-
CODEOWNERS requests reviews rather than enforcing them — no branch protection
164-
rule is configured on this repository, so code-owner approval is a convention
165-
here, not a gate. The same is true of the approval requirement in the review
166-
and merge policy above. Widening the security boundary, or enforcing either
167-
of these through branch protection, is a separate decision.
163+
Code-owner approval and the maintainer-approval requirement above are both
164+
enforced, not conventions. `dev`, `main`, and `preview` each carry an active
165+
repository ruleset — the classic `/branches/{branch}/protection` endpoint
166+
returns 404 for them, which is why this file long described the repository as
167+
unprotected. `Protect dev` (id 20763889) requires a pull request with one
168+
approving review, code-owner review, and extra approval for unattributed
169+
changes, and it blocks deletion and non-fast-forward pushes. Allowed merge
170+
methods are merge and squash; rebase merges are off.
171+
172+
The one carve-out is that the `maintain`/`admin` repository role holds a
173+
`pull_request` bypass, so an owner can merge without the approval the rules
174+
otherwise require. That is a bypass, not an exemption: "Authors do not approve
175+
their own pull requests" above still governs, and an owner who uses the bypass
176+
should record it on the pull request rather than leave it to be inferred from
177+
a merge timestamp. Widening the security boundary is a separate decision.
168178

169179
## Security reports
170180

bin/ocx.mjs

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import {
2323
} from "../src/update/npm-cache-preflight.mjs";
2424
import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "../src/update/tray-update-plan.mjs";
2525
import { bootRestoreProbe, transactionalNpmUpdate } from "../src/update/transactional-install.mjs";
26+
import {
27+
CODEX_CLI_VERSION_MANAGER_ROOT_ENV_SLOTS,
28+
isCodexCliUpdateInspectionArgv,
29+
} from "../src/update/codex-cli-update-launch-policy.mjs";
2630

2731
const PKG = "@bitkyc08/opencodex";
2832
const require = createRequire(import.meta.url);
@@ -155,10 +159,9 @@ function runNpmSelfUpdate() {
155159
process.platform === "win32" ? trayInstallState() : { installed: false, running: false },
156160
);
157161
/**
158-
* Refresh the existing service without re-registering it. `service repair` discovers
159-
* the installed backend itself and, on Windows scheduler installs, rewrites the wrapper
160-
* assets and restarts the existing task without `schtasks /create` — the elevation a
161-
* non-admin `ocx update` does not have.
162+
* Refresh the existing service in place. `service repair` discovers the installed backend;
163+
* healthy Windows scheduler registrations avoid `schtasks /create`, while stale definitions
164+
* may be re-registered and require elevation.
162165
*/
163166
function serviceRefreshArgs() {
164167
return [launcher, "service", "repair"];
@@ -290,7 +293,8 @@ function runNpmSelfUpdate() {
290293
}
291294
}
292295
if (needDirectStart) {
293-
// A repair needs no elevation, but it can still fail — or exit 0 while leaving
296+
// Repair normally avoids elevation for a healthy registration, but a stale Windows
297+
// scheduler definition can require it. It can also fail — or exit 0 while leaving
294298
// a non-viable manager. Fall back to a direct detached proxy start so the
295299
// update never leaves the user without a running proxy.
296300
console.warn(
@@ -468,7 +472,7 @@ function fail(msg) {
468472
process.exit(1);
469473
}
470474

471-
function resolveBun() {
475+
function resolveBun({ allowInstall = true } = {}) {
472476
// Keep direct npm-launcher starts aligned with durable service/shim installs:
473477
// a valid explicit runtime must win even when the bundled dependency exists.
474478
const override = process.env[BUN_OVERRIDE_ENV]?.trim();
@@ -493,7 +497,7 @@ function resolveBun() {
493497
// Lazy fallback: --ignore-scripts (or a failed postinstall) leaves the
494498
// ~450-byte placeholder stub. Run the bun package's own installer once.
495499
const installJs = join(bunDir, "install.js");
496-
if (existsSync(installJs)) {
500+
if (allowInstall && existsSync(installJs)) {
497501
const r = spawnSync(process.execPath, [installJs], { stdio: "inherit" });
498502
if (r.status === 0) bin = findBunBinary(bunDir);
499503
}
@@ -512,14 +516,20 @@ if (updateHelpRequested) {
512516
process.exit(0);
513517
}
514518

519+
const codexCliUpdateInspection = isCodexCliUpdateInspectionArgv(process.argv);
520+
if (codexCliUpdateInspection && typeof process.versions.bun === "string") {
521+
console.error("opencodex: codex-cli-update inspection must use the published Node launcher.");
522+
process.exit(1);
523+
}
524+
515525
if (process.argv[2] === "update" && isNodeModulesInstall() && !isBunGlobalInstall()) {
516526
runNpmSelfUpdate();
517527
}
518528

519529
// #1849 boot probe: a prior update that lost power (or double-faulted) mid-swap leaves a
520530
// backup sibling and a broken live tree. Restore before anything tries to run from the
521531
// broken tree; reap stale backups once the live tree verifies healthy.
522-
if (isNodeModulesInstall() && !isBunGlobalInstall()) {
532+
if (!codexCliUpdateInspection && isNodeModulesInstall() && !isBunGlobalInstall()) {
523533
try {
524534
const probe = bootRestoreProbe(resolve(here, ".."));
525535
if (probe.action === "restored") {
@@ -530,7 +540,7 @@ if (isNodeModulesInstall() && !isBunGlobalInstall()) {
530540
} catch { /* the probe must never block launch */ }
531541
}
532542

533-
const bunRuntime = resolveBun();
543+
const bunRuntime = resolveBun({ allowInstall: !codexCliUpdateInspection });
534544
const bun = bunRuntime.path;
535545

536546
// Run the Bun child asynchronously and FORWARD termination signals to it, then wait
@@ -554,20 +564,69 @@ const bun = bunRuntime.path;
554564
// interpolation and provider settings legitimately read the project environment.
555565
const preBunAnthropicSlots = ["ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_BASE_URL"]
556566
.filter(name => typeof process.env[name] === "string" && process.env[name] !== "");
567+
// A configured CODEX_CLI_PATH may legitimately be cwd-relative (`./tools/codex`), which the
568+
// ordinary runtime resolver accepts. Inspection only trusts absolute local paths, so capture
569+
// the absolute form here, in the launcher, while the original cwd is still authoritative;
570+
// resolving it later would silently reinterpret it against a different working directory.
571+
//
572+
// A bare command with no separator (`codex`) is NOT a relative path: the runtime resolver
573+
// deliberately hands those to executable lookup along PATH. Rewriting it to `<cwd>/codex`
574+
// would make the inspector treat it as an explicit path and stop searching PATH entirely.
575+
const configuredCodexCliPath = typeof process.env.CODEX_CLI_PATH === "string" && process.env.CODEX_CLI_PATH !== ""
576+
? process.env.CODEX_CLI_PATH
577+
: null;
578+
const preBunCodexCliPath = configuredCodexCliPath !== null
579+
&& (configuredCodexCliPath.includes("/") || configuredCodexCliPath.includes("\\") || /^[A-Za-z]:/.test(configuredCodexCliPath))
580+
? resolve(configuredCodexCliPath)
581+
: configuredCodexCliPath;
582+
const preBunPath = typeof process.env.PATH === "string" ? process.env.PATH : null;
583+
const preBunPathExt = typeof process.env.PATHEXT === "string" ? process.env.PATHEXT : null;
584+
const preBunCodexCliManagerRoots = Object.fromEntries(
585+
CODEX_CLI_VERSION_MANAGER_ROOT_ENV_SLOTS.flatMap(name => {
586+
const value = process.env[name];
587+
return typeof value === "string" && value !== "" ? [[name, value]] : [];
588+
}),
589+
);
557590
const launchProof = randomBytes(32).toString("base64url");
558591
const launchContext = JSON.stringify({
559592
version: 1,
560593
proof: launchProof,
561594
anthropicEnvSlots: preBunAnthropicSlots,
595+
codexCliInspectionEnv: codexCliUpdateInspection ? {
596+
codexCliPath: preBunCodexCliPath,
597+
path: preBunPath,
598+
pathExt: preBunPathExt,
599+
managerRoots: preBunCodexCliManagerRoots,
600+
configDir: configDir(),
601+
} : null,
562602
});
603+
// The inspection snapshot above already carries PATH, PATHEXT, and the manager-root slots as
604+
// proof-bound values, and `inspectCodexCliInstall` reads them from that snapshot rather than
605+
// from the live environment. Inheriting them again would spend the 32,767-character Windows
606+
// environment block twice, so a large-but-valid shell environment could stop the Bun child
607+
// from spawning and fail the command before it reports anything. Drop the duplicates for the
608+
// one-shot inspection launch only; every other launch inherits the environment unchanged.
609+
// Windows environment names are case-insensitive, but this spread produces an ordinary
610+
// case-sensitive object, and a real Windows environment commonly spells the variable `Path`.
611+
// Deleting only the canonical upper-case spelling would silently leave that copy behind and
612+
// reintroduce the duplication this block exists to prevent, so match on the lowercase form.
613+
const inheritedEnv = { ...process.env };
614+
if (codexCliUpdateInspection) {
615+
const snapshotted = new Set(
616+
["PATH", "PATHEXT", ...CODEX_CLI_VERSION_MANAGER_ROOT_ENV_SLOTS].map(name => name.toLowerCase()),
617+
);
618+
for (const name of Object.keys(inheritedEnv)) {
619+
if (snapshotted.has(name.toLowerCase())) delete inheritedEnv[name];
620+
}
621+
}
563622
const child = spawn(bun, [cliPath, `${NODE_LAUNCH_PROOF_PREFIX}${launchProof}`, ...process.argv.slice(2)], {
564623
stdio: "inherit",
565624
// A headless Windows parent (Task Scheduler, dashboard restart, shortcut) has no
566625
// console to inherit. Without this flag Windows allocates a visible console for
567626
// the long-running Bun child, and closing that window kills the proxy (#1236).
568627
windowsHide: true,
569628
env: {
570-
...process.env,
629+
...inheritedEnv,
571630
[NODE_LAUNCH_CONTEXT_ENV]: launchContext,
572631
[BUN_RUNTIME_SOURCE_ENV]: bunRuntime.source,
573632
[BUN_RUNTIME_PATH_ENV]: bunRuntime.path,
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Aside client + Integrations UX repair
2+
3+
Unit opened 2026-08-31. Two outcomes travel together because they land on the
4+
same page: Aside becomes an export/integration client, and the Integrations
5+
("연결") surface stops flooding itself with rollback rows.
6+
7+
They are one unit rather than two because the Aside work ADDS a twelfth card to
8+
a page that is already too crowded to absorb one. Shipping the client first
9+
would make the page measurably worse before it got better.
10+
11+
## The two problems
12+
13+
**Aside is unsupported.** Aside is a Chromium fork with a built-in browser
14+
agent. Its custom-provider catalog lives at `~/.aside/u/<accountId>/models.json`
15+
and its schema is the one Pi reads. The user on this machine already wired
16+
opencodex into it BY HAND: the live file carries a `providers.opencodex` block
17+
with 24 routed models, `api: "openai-completions"`, and
18+
`apiKey: "opencodex-loopback"` — byte-identical to what `buildPiClientConfig`
19+
emits. A hand-maintained integration is the strongest possible argument that
20+
the client belongs in the registry.
21+
22+
**The Integrations page floods.** The rollback journal renders up to 50 rows,
23+
each with its own border, at the bottom of the overview AND again on every file
24+
client tab. The user's words were "로그 밑에 막 다닥다닥 뜨는 히스토리" — the
25+
per-row borders are literally what produces that texture.
26+
27+
## Work phases
28+
29+
| Phase | Doc | Deliverable |
30+
|---|---|---|
31+
| wp1 | this unit | Research and roadmap (docs only) |
32+
| wp2 | 010 | Aside export client + integration registry |
33+
| wp3 | 020 | Aside GUI surface, marks entry, nine locales |
34+
| wp4 | 030 | Rollback surface redesign |
35+
| wp5 | 040 | Brand marks for the nine clients showing a monogram |
36+
| wp6 | 050 | Stacked PR chain |
37+
38+
Research docs: 001 (Aside contract), 002 (registration checklist),
39+
003 (Integrations UX diagnosis), 004 (brand mark provenance).
40+
41+
## Ordering constraint
42+
43+
wp4 and wp5 do not depend on wp2/wp3, and wp3 depends on wp2. The stack is
44+
therefore not a single line: the Aside pair (wp2 then wp3) and the page repair
45+
pair (wp4, wp5) are independent chains that both branch off `dev`. wp6 puts
46+
them in review order.

0 commit comments

Comments
 (0)