diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b515d45..df4fb2ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Notable changes to house-rules. Versions follow [semantic versioning](https://se ## Unreleased +- `hr-code-review`'s seat client now passes `MODEL_PROVIDER` to a Codex seat (codex-acp reads it to choose the provider for new sessions, and the narrowed environment dropped it; no other seat, and not `DEFAULT_AUTH_REQUEST` or `INITIAL_AGENT_MODE`, gets it), and `references/reviewer-role.md` says the parent leaves protected paths (secrets, `.env` files, credentials) out of a large diff it saves into the review root and lists what it left out. - `hr-read-reddit` and the README say that Reddit announced RSS support ends November 13, 2026, so feeds can fail, and tell readers to use the partial search fallback when one does (on the cutoff day search feeds answered HTTP 403 while a thread feed still answered); the README's dark banner and overview image URLs carry `?v=1`, per the `web-image-formats` rule (bump it in the commit that changes either image's bytes). - Four wording fixes from review points on landed text: `web-image-formats.md` says to use a version parameter the image host does not own (and to leave a URL alone whose host already uses `v`) and that a branch-pointing URL shows the base branch's image until the pull request merges (check the asset in the pull request's files) and can keep the old bytes up to five minutes after it; `hr-diagnosing-bugs`' necessity test compares occasions with the other inputs and state alike; `hr-sqlite`'s rerun rows call their ratios indicative because the observer rejected those phases; `multi-agent.md`'s read-back clamps each start to 0 and stops when it reaches 0. - An unbuilt stage of work the user approved (a later phase, "PR N", a follow-up pull request of an approved design) is open work, never a parked item: `rules/core.md` § Session purpose says to write it as its own open ledger line that starts "Approved, not built:", and § End of every reply makes it a candidate for the offer when the stage before it lands and on every "next?". That line is the project's list, kept in the plan's Ledger with no issue. `hr-what-next` lists such lines in its open-work floor, `scripts/check-open-work.mjs` prints them as "approved, not built" and reports a deferred line that starts this way as a finding, and `hr-handoff`'s reconciliation reference says the checker carries them forward like any open line. diff --git a/skills/hr-code-review/references/acp-client.md b/skills/hr-code-review/references/acp-client.md index 74f621ff..9687eff3 100644 --- a/skills/hr-code-review/references/acp-client.md +++ b/skills/hr-code-review/references/acp-client.md @@ -138,7 +138,7 @@ What stays outside this: managed policy settings and an adapter's own hooks (Cla The seat starts with a narrowed environment, not the caller's: named locale, path, XDG, proxy and certificate variables (on Windows, also the variables a program needs to start), plus only the variables its own agent reads to log in -(`CODEX_` and `OPENAI_` for Codex, `ANTHROPIC_` and twelve `CLAUDE_` names for Claude, `DEVIN_`, +(`CODEX_`, `OPENAI_` and the name `MODEL_PROVIDER` for Codex, `ANTHROPIC_` and twelve `CLAUDE_` names for Claude, `DEVIN_`, `GROK_` and `XAI_`, `GEMINI_` and `GOOGLE_` for Antigravity, `CURSOR_`). A Claude seat keeps `ANTHROPIC_*` and only these `CLAUDE_` names, each read by the installed claude-agent-acp 0.102.0, its SDK or the CLI: diff --git a/skills/hr-code-review/references/reviewer-role.md b/skills/hr-code-review/references/reviewer-role.md index 2006732f..871494ce 100644 --- a/skills/hr-code-review/references/reviewer-role.md +++ b/skills/hr-code-review/references/reviewer-role.md @@ -3,7 +3,9 @@ Use this brief when a host lets you define a standing reviewer, or when you hand a review to a read-only sub-agent. The parent supplies intent, the diff (or, when it is too large to include, the head, the base and the path of a file inside the review root where the parent wrote the diff, because a reviewer -with only read and search tools cannot run a comparison command), changed paths and verification evidence, and names every +with only read and search tools cannot run a comparison command; the parent leaves out of that +file any path the review must not read (secrets, `.env` files, credentials) and lists the paths +it left out, because a saved diff is as readable to the reviewer as any file in the root), changed paths and verification evidence, and names every model that wrote any part of the reviewed change, committed or not (vendor, name, version, effort if known), or says that none did, so the reviewer can check that it is not one of them. The reviewer works within that scope. diff --git a/skills/hr-code-review/scripts/seat-launch.mjs b/skills/hr-code-review/scripts/seat-launch.mjs index f2694a30..320d9b40 100644 --- a/skills/hr-code-review/scripts/seat-launch.mjs +++ b/skills/hr-code-review/scripts/seat-launch.mjs @@ -46,6 +46,9 @@ export const LOGIN_PREFIXES = { // ../references/acp-client.md. /** @type {Record} */ export const LOGIN_NAMES = { + // codex-acp passes MODEL_PROVIDER (a provider name, not a path) to Codex for new sessions (its README, "Runtime + // options"; dist/index.js startAcpServer, 2.6.0); without it a seat cannot use the provider the caller selected. + codex: ["MODEL_PROVIDER"], claude: [ "CLAUDE_CONFIG_DIR", "CLAUDE_CODE_OAUTH_TOKEN", diff --git a/skills/hr-code-review/test/seat-launch.test.mjs b/skills/hr-code-review/test/seat-launch.test.mjs index f621dcc5..ae8db374 100644 --- a/skills/hr-code-review/test/seat-launch.test.mjs +++ b/skills/hr-code-review/test/seat-launch.test.mjs @@ -110,6 +110,24 @@ test( assert.deepEqual(Object.keys(env).sort(), ["FAKE_KEEP", "GROK_HOME"]); }, ); +test( + "a Codex seat keeps MODEL_PROVIDER, which codex-acp passes to Codex, and no other seat does", + POSIX, + () => { + const env = { + ...ENV, + MODEL_PROVIDER: "gateway", + DEFAULT_AUTH_REQUEST: "{}", + INITIAL_AGENT_MODE: "agent", + }; + assert.equal(seatEnv(env, "codex", []).MODEL_PROVIDER, "gateway"); + assert.equal(seatEnv(env, "claude", []).MODEL_PROVIDER, undefined); + assert.equal(seatEnv(env, "", []).MODEL_PROVIDER, undefined); + // The client sets the Codex seat's mode itself, and the auth request is not part of this change. + assert.equal(seatEnv(env, "codex", []).INITIAL_AGENT_MODE, undefined); + assert.equal(seatEnv(env, "codex", []).DEFAULT_AUTH_REQUEST, undefined); + }, +); test("each agent keeps only its own login variables", POSIX, () => { const claude = seatEnv(ENV, "claude", []); assert.equal(claude.ANTHROPIC_AUTH_TOKEN, "a");