diff --git a/.cursor/skills/opencodex-fork-sync/SKILL.md b/.cursor/skills/opencodex-fork-sync/SKILL.md index adbd16e5c3..6694f9d642 100644 --- a/.cursor/skills/opencodex-fork-sync/SKILL.md +++ b/.cursor/skills/opencodex-fork-sync/SKILL.md @@ -69,6 +69,21 @@ fast-forward of `upstream/main`; `vendor/dev` remains an exact fast-forward of branch. The issue notifier is selected by `FORK_SYNC_NOTIFIERS=github-issue`; the Cursor coordinator is selected by `FORK_SYNC_COORDINATORS=cursor-webhook`. +Keep the PR readable while the automation works. Maintain exactly one sticky +progress comment with marker `` and one matching +PR-body section between `` and +``. Update both in place after every push, +never post a new comment. Inside each, render a 4-line checklist in this order: + +1. Merge `vendor/main` TAG/SHA - done or pending +2. Resolve shared hotspot per `OWNED.md` - done or pending (name the files) +3. Rebase onto `origin/dev` - `MERGEABLE` or `not a descendant of origin/dev` with recovery `git merge origin/dev` and re-push +4. CI `ci` / `enforce-target` / `hygiene` - each with `pass` / `fail` and the + exact code (`new_suppression`, `unsponsored_surface`, shard name) when failing + +When all gates are `MERGEABLE` and hygiene is green, flip Draft -> Ready for +review and post `Ready for human merge - do not squash/rebase.` Stop. + Cursor is the first coordinator, not the only supported integration. The registry accepts comma-separated IDs and can run multiple coordinators, for example `FORK_SYNC_COORDINATORS=cursor-webhook,http`. diff --git a/.cursor/skills/opencodex-fork-sync/automation-prompt.md b/.cursor/skills/opencodex-fork-sync/automation-prompt.md index b1cc85e59f..bb4d8d3e59 100644 --- a/.cursor/skills/opencodex-fork-sync/automation-prompt.md +++ b/.cursor/skills/opencodex-fork-sync/automation-prompt.md @@ -29,11 +29,23 @@ Execute only the unresolved handoff stages: commands. 7. Push the sync branch as needed and open or update a draft PR into `dev`. Fill Summary, Verification, and Checklist from the PR template. Include the - decision table and the tag SHA. Run + decision table and the tag SHA. Inside the PR body, maintain a sticky + section between `` and + `` with the 4-line checklist: + (1) Merge `vendor/main` TAG/SHA, (2) Resolve shared hotspot per + `OWNED.md`, (3) Rebase onto `origin/dev` (`MERGEABLE` or + `not a descendant` + `git merge origin/dev`), (4) CI + `ci`/`enforce-target`/`hygiene` with exact failing codes. Mirror the same + checklist in exactly one sticky comment marked + `` - update it in place after every push, + never create a new one. Run `gh pr view --json mergeable -q .mergeable` and do not stop or ping the human until it reports `MERGEABLE`. -8. Stop. The human performs the merge commit, never squash or rebase. Do not - merge the PR, close issues, change repository settings, or force-push +8. Until `MERGEABLE` and hygiene green, stay and supervise. After every push, poll `gh pr view --json mergeable` and `gh pr checks` every 60s for 10m. If `not a descendant of origin/dev`, run `git fetch origin dev && git merge --no-edit origin/dev && git push` yourself. For `macos-launchd` timeout-only flakes, run `gh run rerun --failed` once. Update the sticky PR-body section and the single `` comment in place (never a new comment) with the 4-line checklist and exact failing codes. Only `new_suppression` / `unsponsored_surface` need `suppression-approved` / `maintainer-sponsored` human waive - report the blocker but do not auto-waive. +9. When all gates are `MERGEABLE` and hygiene green, flip Draft -> Ready + for review and post `Ready for human merge - do not squash/rebase.` Stop. + The human performs the merge commit, never squash or rebase. Do not merge + the PR, close issues, change repository settings, or force-push `main`/`origin/main`. If histories diverge again, use the disconnected `run/dev` rebuild only as an diff --git a/.github/scripts/agent-maintenance.cjs b/.github/scripts/agent-maintenance.cjs index 1da877cc26..89a8484d4a 100644 --- a/.github/scripts/agent-maintenance.cjs +++ b/.github/scripts/agent-maintenance.cjs @@ -195,6 +195,45 @@ function maintenanceReadyEvidence({ }; } +function autonomousMergeEvidence({ + pr, + checkRuns = [], + headCommit, + expectedJulesUserId, + authorizedSessionId, + sessionId, + expectedBugbotAppId, + expectedChecksAppId = 15368, + requiredNames = ["ci", "enforce-target", "hygiene"], + labels = (pr?.labels || []), +}) { + const names = new Set(labels.map((label) => typeof label === "string" ? label : label?.name)); + const headSha = pr?.head?.sha; + const julesId = Number(expectedJulesUserId); + const sessionKey = (value) => String(value ?? "").replace(/^sessions\//, ""); + const authorized = authorizedSessionId && sessionKey(sessionId) === sessionKey(authorizedSessionId); + const prByJules = Number.isSafeInteger(julesId) && julesId > 0 && Number(pr?.user?.id) === julesId; + const authoredByJules = Number.isSafeInteger(julesId) && julesId > 0 && + [headCommit?.author?.id, headCommit?.committer?.id].some((id) => Number(id) === julesId); + const baselineReady = requiredChecksSuccessful(checkRuns, headSha, requiredNames, expectedChecksAppId); + const bugbotEvidence = exactHeadBugbotEvidence({ + checkRuns, + liveHeadSha: headSha, + expectedAppId: expectedBugbotAppId, + }); + return { + autonomousLabel: names.has("autonomous-fix"), + baselineReady, + bugbotEvidence, + authorizedSession: Boolean(authorized), + prByJules: Boolean(prByJules), + authoredByJules: Boolean(prByJules && authoredByJules && headCommit?.sha === headSha), + ready: pr?.state === "open" && pr?.base?.ref === "dev" && names.has("autonomous-fix") && + baselineReady && Boolean(bugbotEvidence) && Boolean(authorized) && + Boolean(prByJules && authoredByJules && headCommit?.sha === headSha), + }; +} + function trustedActiveMaintenanceCount(records) { return records.filter((record) => record?.error || @@ -386,6 +425,19 @@ function createJulesClient({ apiKey, fetchImpl = fetch, sleep = (ms) => new Prom return assertSession(await request(`/sessions/${encodeURIComponent(id)}`)); } + async function sendMessage(id, prompt) { + if (!/^[^/]+$/.test(id)) throw new Error("invalid Jules session resource id"); + if (!prompt || typeof prompt !== "string") throw new Error("sendMessage requires a prompt string"); + return request(`/sessions/${encodeURIComponent(id)}:sendMessage`, { method: "POST", body: { prompt }, retryReads: false }); + } + + async function listSessionActivities(id) { + if (!/^[^/]+$/.test(id)) throw new Error("invalid Jules session resource id"); + const result = await request(`/sessions/${encodeURIComponent(id)}/activities`); + if (!result || !Array.isArray(result.activities)) return []; + return result.activities; + } + async function createSessionIdempotently(payload) { try { return await createSession(payload); @@ -420,6 +472,8 @@ function createJulesClient({ apiKey, fetchImpl = fetch, sleep = (ms) => new Prom createSession, createSessionIdempotently, getSession, + listSessionActivities, + sendMessage, listSessions, listSources, }; @@ -439,6 +493,7 @@ module.exports = { exactHeadBugbotEvidence, generatedSyncBaselineDisposition, maintenanceReadyEvidence, + autonomousMergeEvidence, findGithubSource, hasExactHeadMaintainerWaiver, isExpectedJulesHeadAdvance, diff --git a/.github/scripts/agent-maintenance.test.cjs b/.github/scripts/agent-maintenance.test.cjs index 623ef8e593..e6b71a0ee1 100644 --- a/.github/scripts/agent-maintenance.test.cjs +++ b/.github/scripts/agent-maintenance.test.cjs @@ -11,6 +11,7 @@ const { exactHeadBugbotEvidence, generatedSyncBaselineDisposition, maintenanceReadyEvidence, + autonomousMergeEvidence, findGithubSource, hasExactHeadMaintainerWaiver, isExpectedJulesHeadAdvance, @@ -196,6 +197,66 @@ describe("maintenance PR readiness", () => { }); }); +describe("autonomous merge evidence", () => { + const pr = { + number: 42, + state: "open", + base: { ref: "dev" }, + user: { id: 77 }, + head: { sha: SHA }, + labels: [{ name: "autonomous-fix" }], + }; + const checks = [ + ...["ci", "enforce-target", "hygiene"].map((name, id) => ({ + id: id + 1, name, app: { id: 15368 }, head_sha: SHA, + status: "completed", conclusion: "success", + })), + { id: 4, name: "Cursor Bugbot", app: { id: 99 }, head_sha: SHA, + status: "completed", conclusion: "success" }, + ]; + const valid = { + pr, + checkRuns: checks, + headCommit: { sha: SHA, author: { id: 77 }, committer: { id: 77 } }, + expectedJulesUserId: 77, + expectedBugbotAppId: 99, + authorizedSessionId: "sessions/abc", + sessionId: "sessions/abc", + }; + + it("accepts a labeled exact-head Jules fix with all required checks", () => { + const result = autonomousMergeEvidence(valid); + assert.equal(result.ready, true); + assert.equal(result.bugbotEvidence.checkRunId, 4); + }); + + it("rejects a waiver or missing autonomous-fix label", () => { + assert.equal(autonomousMergeEvidence({ + ...valid, + checkRuns: checks.slice(0, 3), + labels: ["review-bot-waived"], + reviews: [ + { id: 1, user: { login: "alice" }, commit_id: SHA, state: "APPROVED" }, + { id: 2, user: { login: "carol" }, commit_id: SHA, state: "APPROVED" }, + ], + maintainers: ["alice", "carol"], + }).ready, false); + assert.equal(autonomousMergeEvidence({ ...valid, pr: { ...pr, labels: [] } }).ready, false); + }); + + it("rejects stale checks, unauthorized session, and non-Jules head authorship", () => { + assert.equal(autonomousMergeEvidence({ ...valid, sessionId: "sessions/other" }).ready, false); + assert.equal(autonomousMergeEvidence({ + ...valid, + headCommit: { ...valid.headCommit, sha: "b".repeat(40) }, + }).ready, false); + assert.equal(autonomousMergeEvidence({ + ...valid, + checkRuns: checks.map(check => check.name === "ci" ? { ...check, conclusion: "failure" } : check), + }).ready, false); + }); +}); + describe("controller fail-closed helpers", () => { it("uses the latest labeled or unlabeled event for an active dispatch label", () => { const events = [ diff --git a/.github/scripts/closed-pr-branch-cleanup.cjs b/.github/scripts/closed-pr-branch-cleanup.cjs new file mode 100644 index 0000000000..57142f19fe --- /dev/null +++ b/.github/scripts/closed-pr-branch-cleanup.cjs @@ -0,0 +1,226 @@ +"use strict"; + +/** + * Deletion planning for branches left behind by closed-without-merge pull + * requests. + * + * GitHub's repository-level `delete_branch_on_merge` only fires on merge, so a + * PR that is closed unmerged leaves its head branch in the repository forever. + * This module decides which of those branches may be deleted; the workflow + * performs the deletion. + * + * Kept as a pure module so the safety rules can be unit-tested without Actions + * and without a live repository. + */ + +/** Branches that may never be deleted regardless of pull-request state. */ +const PROTECTED_BRANCHES = Object.freeze(["main", "dev", "preview", "gh-pages"]); + +/** Default grace period before a closed PR's head branch becomes eligible. */ +const DEFAULT_GRACE_DAYS = 14; + +function normalizeBranchName(value) { + return String(value || "").trim(); +} + +/** + * A commit id, lowercased for comparison. + * + * The REST and GraphQL APIs are not consistent about case, and a full 40-character + * sha compared case-sensitively against an abbreviated or upper-case one silently + * reads as "different" - which here would mean "keep", so the failure direction is + * safe, but it would make the guard useless rather than protective. Anything that + * is not a plausible hex object id becomes null, i.e. unknown. + */ +function normalizeOid(value) { + const text = String(value || "").trim().toLowerCase(); + return /^[0-9a-f]{7,64}$/.test(text) ? text : null; +} + +function isProtectedBranch(name) { + return PROTECTED_BRANCHES.includes(normalizeBranchName(name)); +} + +function toTimestamp(value) { + if (!value) return null; + const ms = Date.parse(String(value)); + return Number.isFinite(ms) ? ms : null; +} + +/** + * Reasons a candidate branch is kept. Exported so the workflow can log a + * stable, greppable verdict per branch instead of a free-form sentence. + */ +const KEEP_REASONS = Object.freeze({ + PROTECTED: "protected-branch", + MERGED: "pull-request-merged", + OPEN: "open-pull-request", + BASE_OF_OPEN: "base-of-open-pull-request", + CROSS_REPOSITORY: "cross-repository-head", + MISSING_CLOSED_AT: "missing-closed-at", + WITHIN_GRACE: "within-grace-period", + MOVED_SINCE_CLOSE: "branch-moved-since-close", + UNKNOWN_HEAD_SHA: "unknown-head-sha", +}); + +/** + * Plan deletions for head branches of closed-unmerged pull requests. + * + * Every rule here is a safety rule, and each one exists because the opposite + * behavior destroys work that is still referenced: + * + * - A branch is a candidate only when *every* pull request that ever used it as + * a head is closed and unmerged. One open or merged PR on the same branch + * keeps it, because reopening a PR whose head branch is gone cannot restore + * the commits. + * - A branch that is the base of an open pull request is kept. Deleting it + * closes the stacked child PR that targets it. + * - Cross-repository (fork) heads are never touched: they live in the + * contributor's repository and this token has no business there. + * - A grace period after `closed_at` leaves room to reopen a PR that was + * closed by mistake. + * - The branch must still POINT AT a commit one of those closed pull requests + * had as its head. Matching by NAME alone deletes reused work: `codex/`-style + * names get picked up again all the time, and a branch recreated for new work + * inherits the closed history of every PR that ever used that name. The tip + * moved, so the branch is not the closed PR's branch any more - it only shares + * its label. + * - A branch whose current tip cannot be determined is kept. An unknown tip is + * not evidence of an abandoned branch, and this job's mistakes are not + * recoverable. + * + * @param {object} input + * @param {Array} input.pullRequests Pull requests with + * `headRefName`, `headRefOid`, `baseRefName`, `state`, `merged`, `closedAt`, + * and `isCrossRepository`. + * @param {Array} input.branches Branches + * that currently exist. A bare string carries no tip, which is treated as an + * unknown tip and kept. + * @param {number} [input.now] Current time in milliseconds. + * @param {number} [input.graceDays] Days to wait after `closedAt`. + * @returns {{ deletions: Array<{branch: string, pullRequests: number[]}>, + * keeps: Array<{branch: string, reason: string}> }} + */ +function planClosedPrBranchDeletions({ + pullRequests = [], + branches = [], + now = Date.now(), + graceDays = DEFAULT_GRACE_DAYS, +}) { + // Accepts both shapes so an older caller passing bare names still works - it + // just gets the conservative answer, because a name without a tip cannot be + // proven safe to delete. + /** @type {Map} */ + const existing = new Map(); + for (const entry of branches) { + const name = normalizeBranchName(typeof entry === "string" ? entry : entry && entry.name); + if (!name) continue; + const oid = typeof entry === "string" ? null : normalizeOid(entry && entry.oid); + existing.set(name, oid); + } + const graceMs = Math.max(0, Number(graceDays) || 0) * 24 * 60 * 60 * 1000; + + /** @type {Map} */ + const byHead = new Map(); + const openBases = new Set(); + + for (const pr of pullRequests) { + const head = normalizeBranchName(pr && pr.headRefName); + if (head) { + const list = byHead.get(head) || []; + list.push(pr); + byHead.set(head, list); + } + const isOpen = String(pr && pr.state).toUpperCase() === "OPEN"; + if (isOpen) { + const base = normalizeBranchName(pr && pr.baseRefName); + if (base) openBases.add(base); + } + } + + const deletions = []; + const keeps = []; + + for (const branch of [...existing.keys()].sort()) { + if (isProtectedBranch(branch)) { + keeps.push({ branch, reason: KEEP_REASONS.PROTECTED }); + continue; + } + + const related = byHead.get(branch) || []; + if (related.length === 0) continue; // No PR ever used it; out of scope. + + if (related.some((pr) => pr && pr.isCrossRepository === true)) { + keeps.push({ branch, reason: KEEP_REASONS.CROSS_REPOSITORY }); + continue; + } + if (related.some((pr) => pr && pr.merged === true)) { + keeps.push({ branch, reason: KEEP_REASONS.MERGED }); + continue; + } + if (related.some((pr) => String(pr && pr.state).toUpperCase() === "OPEN")) { + keeps.push({ branch, reason: KEEP_REASONS.OPEN }); + continue; + } + if (openBases.has(branch)) { + keeps.push({ branch, reason: KEEP_REASONS.BASE_OF_OPEN }); + continue; + } + + const closedTimestamps = related.map((pr) => toTimestamp(pr && pr.closedAt)); + if (closedTimestamps.some((ts) => ts === null)) { + keeps.push({ branch, reason: KEEP_REASONS.MISSING_CLOSED_AT }); + continue; + } + const newestClosedAt = Math.max(...closedTimestamps); + if (now - newestClosedAt < graceMs) { + keeps.push({ branch, reason: KEEP_REASONS.WITHIN_GRACE }); + continue; + } + + // The tip check, last because it is the most expensive claim to satisfy and + // the cheaper rules above have already excluded most branches. + // + // A closed PR's head branch is only THIS branch if the branch still points at + // a commit that PR had as its head. Without this, a name reused for new work + // is deleted on the strength of an unrelated PR that happened to share the + // label months earlier - and a deleted branch whose commits were never pushed + // anywhere else is gone. + const currentOid = existing.get(branch) || null; + if (!currentOid) { + keeps.push({ branch, reason: KEEP_REASONS.UNKNOWN_HEAD_SHA }); + continue; + } + const closedOids = new Set( + related.map((pr) => normalizeOid(pr && pr.headRefOid)).filter(Boolean), + ); + // An empty set means the API gave us no head SHA for any of them, which is the + // unknown case again rather than a licence to delete. + if (closedOids.size === 0) { + keeps.push({ branch, reason: KEEP_REASONS.UNKNOWN_HEAD_SHA }); + continue; + } + if (!closedOids.has(currentOid)) { + keeps.push({ branch, reason: KEEP_REASONS.MOVED_SINCE_CLOSE }); + continue; + } + + deletions.push({ + branch, + pullRequests: related + .map((pr) => Number(pr && pr.number)) + .filter((n) => Number.isFinite(n)) + .sort((a, b) => a - b), + }); + } + + return { deletions, keeps }; +} + +module.exports = { + DEFAULT_GRACE_DAYS, + KEEP_REASONS, + PROTECTED_BRANCHES, + isProtectedBranch, + planClosedPrBranchDeletions, +}; diff --git a/.github/scripts/closed-pr-branch-cleanup.test.cjs b/.github/scripts/closed-pr-branch-cleanup.test.cjs new file mode 100644 index 0000000000..644cb0b7aa --- /dev/null +++ b/.github/scripts/closed-pr-branch-cleanup.test.cjs @@ -0,0 +1,165 @@ +"use strict"; + +const { describe, it } = require("node:test"); +const assert = require("node:assert/strict"); +const { + DEFAULT_GRACE_DAYS, + KEEP_REASONS, + isProtectedBranch, + planClosedPrBranchDeletions, +} = require("./closed-pr-branch-cleanup.cjs"); + +const NOW = Date.parse("2026-08-26T00:00:00Z"); +const DAY = 24 * 60 * 60 * 1000; +const longAgo = new Date(NOW - 60 * DAY).toISOString(); + +function closedPr(overrides) { + return { + number: 1, + state: "CLOSED", + merged: false, + isCrossRepository: false, + headRefName: "codex/example", + baseRefName: "dev", + closedAt: longAgo, + ...overrides, + }; +} + +function keepReason(result, branch) { + const hit = result.keeps.find((entry) => entry.branch === branch); + return hit ? hit.reason : null; +} + +function deletedBranches(result) { + return result.deletions.map((entry) => entry.branch); +} + +describe("isProtectedBranch", () => { + it("protects the integration, release, and prerelease lines", () => { + for (const name of ["main", "dev", "preview", "gh-pages"]) { + assert.equal(isProtectedBranch(name), true, name); + } + assert.equal(isProtectedBranch("codex/dev"), false); + }); +}); + +describe("planClosedPrBranchDeletions", () => { + it("deletes a branch whose only pull request closed unmerged past the grace period", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 42, headRefName: "codex/stale" })], + branches: ["codex/stale", "dev"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), ["codex/stale"]); + assert.deepEqual(result.deletions[0].pullRequests, [42]); + }); + + it("keeps a branch that any merged pull request used as a head", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [ + closedPr({ number: 10, headRefName: "codex/reused" }), + closedPr({ number: 11, headRefName: "codex/reused", state: "MERGED", merged: true }), + ], + branches: ["codex/reused"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "codex/reused"), KEEP_REASONS.MERGED); + }); + + it("keeps a branch that still has an open pull request", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [ + closedPr({ number: 20, headRefName: "codex/active" }), + closedPr({ number: 21, headRefName: "codex/active", state: "OPEN", closedAt: null }), + ], + branches: ["codex/active"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "codex/active"), KEEP_REASONS.OPEN); + }); + + it("keeps a closed stack parent while an open child still targets it", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [ + closedPr({ number: 30, headRefName: "codex/stack-1" }), + closedPr({ + number: 31, + state: "OPEN", + closedAt: null, + headRefName: "codex/stack-2", + baseRefName: "codex/stack-1", + }), + ], + branches: ["codex/stack-1", "codex/stack-2"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "codex/stack-1"), KEEP_REASONS.BASE_OF_OPEN); + }); + + it("never touches a fork head branch", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [ + closedPr({ number: 40, headRefName: "patch-1", isCrossRepository: true }), + ], + branches: ["patch-1"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "patch-1"), KEEP_REASONS.CROSS_REPOSITORY); + }); + + it("waits out the grace period so a mistaken close can be reopened", () => { + const recent = new Date(NOW - 3 * DAY).toISOString(); + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 50, headRefName: "codex/recent", closedAt: recent })], + branches: ["codex/recent"], + now: NOW, + graceDays: DEFAULT_GRACE_DAYS, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "codex/recent"), KEEP_REASONS.WITHIN_GRACE); + }); + + it("keeps a branch when a closed pull request has no closed_at timestamp", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 60, headRefName: "codex/unknown", closedAt: null })], + branches: ["codex/unknown"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "codex/unknown"), KEEP_REASONS.MISSING_CLOSED_AT); + }); + + it("refuses to delete a protected branch even if a closed pull request used it", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 70, headRefName: "dev" })], + branches: ["dev", "main", "preview"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal(keepReason(result, "dev"), KEEP_REASONS.PROTECTED); + }); + + it("ignores branches that no pull request ever used", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 80, headRefName: "codex/known" })], + branches: ["codex/known", "codex/never-a-pr"], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), ["codex/known"]); + assert.equal(keepReason(result, "codex/never-a-pr"), null); + }); + + it("only plans deletions for branches that still exist", () => { + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 90, headRefName: "codex/already-gone" })], + branches: [], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + }); +}); diff --git a/.github/scripts/fork-dev-auto-release.cjs b/.github/scripts/fork-dev-auto-release.cjs new file mode 100644 index 0000000000..fd13f2e3c0 --- /dev/null +++ b/.github/scripts/fork-dev-auto-release.cjs @@ -0,0 +1,159 @@ +"use strict"; + +const FULL_SHA = /^[0-9a-f]{40}$/; +const SEMVER = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/; +const PACKAGE_NAME = "@yansigit/opencodex"; + +function requiredString(value, name) { + if (typeof value !== "string" || value.length === 0) { + throw new TypeError(`${name} must be a non-empty string`); + } + return value; +} + +function requiredSha(value, name) { + requiredString(value, name); + if (!FULL_SHA.test(value)) { + throw new TypeError(`${name} must be a full 40-character commit SHA`); + } + return value; +} + +function parseSemver(v) { + const match = SEMVER.exec(v); + if (!match) return null; + return { + major: Number(match[1]), + minor: Number(match[2]), + patch: Number(match[3]), + }; +} + +function computeBaseVersion(packageVersion, latestVersionOnNpm) { + const pkgParsed = parseSemver(packageVersion); + if (!pkgParsed) { + throw new TypeError(`packageVersion must be valid semver; got ${packageVersion}`); + } + + if (!latestVersionOnNpm) { + return `${pkgParsed.major}.${pkgParsed.minor}.${pkgParsed.patch}`; + } + + const npmParsed = parseSemver(latestVersionOnNpm); + if (!npmParsed) { + return `${pkgParsed.major}.${pkgParsed.minor}.${pkgParsed.patch}`; + } + + const diffMajor = pkgParsed.major - npmParsed.major; + const diffMinor = pkgParsed.minor - npmParsed.minor; + const diffPatch = pkgParsed.patch - npmParsed.patch; + + if (diffMajor > 0 || (diffMajor === 0 && diffMinor > 0) || (diffMajor === 0 && diffMinor === 0 && diffPatch > 0)) { + return `${pkgParsed.major}.${pkgParsed.minor}.${pkgParsed.patch}`; + } + + return `${npmParsed.major}.${npmParsed.minor}.${npmParsed.patch + 1}`; +} + +function formatDate(d = new Date()) { + const yyyy = d.getUTCFullYear(); + const mm = String(d.getUTCMonth() + 1).padStart(2, "0"); + const dd = String(d.getUTCDate()).padStart(2, "0"); + return `${yyyy}${mm}${dd}`; +} + +function decideForkDevAutoRelease({ + eventName, + workflowName, + conclusion, + headBranch, + headSha, + liveDevSha, + packageName, + packageVersion, + latestVersionOnNpm, + existingCommitDevTag, + runNumber, + now, +}) { + requiredString(eventName, "eventName"); + requiredString(workflowName, "workflowName"); + requiredString(conclusion, "conclusion"); + requiredString(headBranch, "headBranch"); + requiredSha(headSha, "headSha"); + requiredSha(liveDevSha, "liveDevSha"); + requiredString(packageName, "packageName"); + requiredString(packageVersion, "packageVersion"); + + if (!SEMVER.test(packageVersion)) { + throw new TypeError("packageVersion must be valid semver"); + } + + if (eventName !== "workflow_run") { + return { action: "skip", reason: `event must be workflow_run; got ${eventName}.` }; + } + if (workflowName !== "Cross-platform CI") { + return { action: "skip", reason: `triggering workflow must be Cross-platform CI; got ${workflowName}.` }; + } + if (conclusion !== "success") { + return { action: "skip", reason: `CI conclusion must be success; got ${conclusion}.` }; + } + if (headBranch !== "dev") { + return { action: "skip", reason: `CI head branch must be dev; got ${headBranch}.` }; + } + if (headSha !== liveDevSha) { + return { + action: "skip", + reason: `live dev moved after CI; audited ${headSha}, current ${liveDevSha}.`, + }; + } + if (packageName !== PACKAGE_NAME) { + return { + action: "skip", + reason: `package must be ${PACKAGE_NAME}; got ${packageName}.`, + }; + } + if (existingCommitDevTag) { + return { + action: "skip", + reason: `commit ${headSha} is already released as ${existingCommitDevTag}.`, + }; + } + + const base = computeBaseVersion(packageVersion, latestVersionOnNpm); + const dateStr = formatDate(now); + const runNum = runNumber || "1"; + const version = `${base}-dev.${dateStr}.${runNum}`; + + return { + action: "dispatch", + version, + }; +} + +function decideFromEnv(env = process.env) { + return decideForkDevAutoRelease({ + eventName: env.EVENT_NAME, + workflowName: env.WORKFLOW_NAME, + conclusion: env.CONCLUSION, + headBranch: env.HEAD_BRANCH, + headSha: env.HEAD_SHA, + liveDevSha: env.LIVE_DEV_SHA, + packageName: env.PACKAGE_NAME, + packageVersion: env.PACKAGE_VERSION, + latestVersionOnNpm: env.NPM_LATEST_VERSION || undefined, + existingCommitDevTag: env.EXISTING_COMMIT_DEV_TAG || undefined, + runNumber: env.RUN_NUMBER, + }); +} + +if (require.main === module) { + process.stdout.write(JSON.stringify(decideFromEnv())); +} + +module.exports = { + computeBaseVersion, + decideForkDevAutoRelease, + decideFromEnv, +}; + diff --git a/.github/scripts/fork-dev-auto-release.test.cjs b/.github/scripts/fork-dev-auto-release.test.cjs new file mode 100644 index 0000000000..4b3e6e09c6 --- /dev/null +++ b/.github/scripts/fork-dev-auto-release.test.cjs @@ -0,0 +1,152 @@ +"use strict"; + +const { describe, it } = require("node:test"); +const assert = require("node:assert/strict"); +const { + computeBaseVersion, + decideForkDevAutoRelease, +} = require("./fork-dev-auto-release.cjs"); + +const SHA = "0123456789abcdef0123456789abcdef01234567"; +const OTHER_SHA = "89abcdef0123456789abcdef0123456789abcdef"; + +function candidate(overrides = {}) { + return { + eventName: "workflow_run", + workflowName: "Cross-platform CI", + conclusion: "success", + headBranch: "dev", + headSha: SHA, + liveDevSha: SHA, + packageName: "@yansigit/opencodex", + packageVersion: "2.33.1", + latestVersionOnNpm: "2.33.1", + runNumber: "42", + now: new Date("2026-08-27T12:00:00Z"), + ...overrides, + }; +} + +describe("computeBaseVersion", () => { + it("bumps patch when package.json version matches npm latest", () => { + assert.equal(computeBaseVersion("2.33.1", "2.33.1"), "2.33.2"); + }); + + it("keeps package.json version when it is already ahead of npm latest", () => { + assert.equal(computeBaseVersion("2.34.0", "2.33.1"), "2.34.0"); + assert.equal(computeBaseVersion("3.0.0", "2.33.1"), "3.0.0"); + assert.equal(computeBaseVersion("2.33.2", "2.33.1"), "2.33.2"); + }); + + it("bumps patch of npm latest when package.json trails npm latest", () => { + assert.equal(computeBaseVersion("2.32.0", "2.33.1"), "2.33.2"); + }); + + it("uses package.json version when npm latest is not available or unparseable", () => { + assert.equal(computeBaseVersion("2.33.1"), "2.33.1"); + assert.equal(computeBaseVersion("2.33.1", ""), "2.33.1"); + assert.equal(computeBaseVersion("2.33.1", "invalid"), "2.33.1"); + }); + + it("throws when package.json version is invalid semver", () => { + assert.throws(() => computeBaseVersion("invalid"), /must be valid semver/); + }); +}); + +describe("fork dev auto-release decision", () => { + it("dispatches with computed dev version from green dev CI", () => { + const result = decideForkDevAutoRelease(candidate()); + assert.deepEqual(result, { + action: "dispatch", + version: "2.33.2-dev.20260827.42", + }); + }); + + it("uses package version if ahead of npm latest", () => { + const result = decideForkDevAutoRelease(candidate({ packageVersion: "2.34.0" })); + assert.deepEqual(result, { + action: "dispatch", + version: "2.34.0-dev.20260827.42", + }); + }); + + for (const [name, overrides, reason] of [ + ["skips non-workflow_run events", { eventName: "push" }, "workflow_run"], + ["skips another triggering workflow", { workflowName: "Release" }, "Cross-platform CI"], + ["skips unsuccessful CI", { conclusion: "failure" }, "success"], + ["skips non-dev branches", { headBranch: "main" }, "dev"], + ["skips a moved dev branch", { liveDevSha: OTHER_SHA }, "live dev"], + ["skips another package", { packageName: "opencodex" }, "@yansigit/opencodex"], + ["skips if commit already has a dev release tag", { existingCommitDevTag: "v2.33.2-dev.20260827.1" }, "already released"], + ]) { + it(name, () => { + const result = decideForkDevAutoRelease(candidate(overrides)); + assert.equal(result.action, "skip"); + assert.match(result.reason, new RegExp(reason.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"))); + }); + } + + it("rejects an empty SHA as malformed input", () => { + assert.throws( + () => decideForkDevAutoRelease(candidate({ headSha: "" })), + /headSha must be a non-empty string/, + ); + }); + + it("rejects a missing package name as malformed input", () => { + assert.throws( + () => decideForkDevAutoRelease(candidate({ packageName: undefined })), + /packageName must be a non-empty string/, + ); + }); +}); + +describe("fork dev auto-release env CLI", () => { + const { spawnSync } = require("node:child_process"); + const { join } = require("node:path"); + const script = join(__dirname, "fork-dev-auto-release.cjs"); + + function runCli(env) { + return spawnSync(process.execPath, [script], { + encoding: "utf8", + env: { ...process.env, ...env }, + }); + } + + it("prints dispatch JSON from env vars without a node heredoc", () => { + const result = runCli({ + EVENT_NAME: "workflow_run", + WORKFLOW_NAME: "Cross-platform CI", + CONCLUSION: "success", + HEAD_BRANCH: "dev", + HEAD_SHA: SHA, + LIVE_DEV_SHA: SHA, + PACKAGE_NAME: "@yansigit/opencodex", + PACKAGE_VERSION: "2.33.1", + NPM_LATEST_VERSION: "2.33.1", + RUN_NUMBER: "10", + }); + assert.equal(result.status, 0, result.stderr); + const parsed = JSON.parse(result.stdout); + assert.equal(parsed.action, "dispatch"); + assert.match(parsed.version, /^2\.33\.2-dev\.[0-9]{8}\.10$/); + }); + + it("skips when commit is already released as a dev tag", () => { + const result = runCli({ + EVENT_NAME: "workflow_run", + WORKFLOW_NAME: "Cross-platform CI", + CONCLUSION: "success", + HEAD_BRANCH: "dev", + HEAD_SHA: SHA, + LIVE_DEV_SHA: SHA, + PACKAGE_NAME: "@yansigit/opencodex", + PACKAGE_VERSION: "2.33.1", + EXISTING_COMMIT_DEV_TAG: "v2.33.2-dev.20260827.1", + }); + assert.equal(result.status, 0, result.stderr); + const parsed = JSON.parse(result.stdout); + assert.equal(parsed.action, "skip"); + assert.match(parsed.reason, /already released/); + }); +}); diff --git a/.github/scripts/release-dispatch-guard.cjs b/.github/scripts/release-dispatch-guard.cjs index 507cd3e787..395725e510 100644 --- a/.github/scripts/release-dispatch-guard.cjs +++ b/.github/scripts/release-dispatch-guard.cjs @@ -3,6 +3,7 @@ const ALLOWED_RELEASE_REFS = new Set([ "refs/heads/main", "refs/heads/preview", + "refs/heads/dev", ]); function validateReleaseDispatch({ @@ -16,7 +17,7 @@ function validateReleaseDispatch({ } if (!ALLOWED_RELEASE_REFS.has(ref)) { - return `Release must run from main or preview; got ${ref || "(empty)"}.`; + return `Release must run from main, preview, or dev; got ${ref || "(empty)"}.`; } if (!expectedSha) { diff --git a/.github/scripts/release-dispatch-guard.test.cjs b/.github/scripts/release-dispatch-guard.test.cjs index 7833ca0976..9854d41a45 100644 --- a/.github/scripts/release-dispatch-guard.test.cjs +++ b/.github/scripts/release-dispatch-guard.test.cjs @@ -29,6 +29,13 @@ describe("release dispatch guard", () => { ); }); + it("accepts an exact audited SHA on dev", () => { + assert.equal( + validate({ ref: "refs/heads/dev" }), + null, + ); + }); + it("rejects non-workflow_dispatch events", () => { assert.match( validate({ eventName: "push" }), @@ -38,8 +45,8 @@ describe("release dispatch guard", () => { it("rejects release dispatches from unapproved refs", () => { assert.match( - validate({ ref: "refs/heads/dev" }), - /must run from main or preview/, + validate({ ref: "refs/heads/feature" }), + /must run from main, preview, or dev/, ); }); diff --git a/.github/scripts/sync-pr-babysitter.cjs b/.github/scripts/sync-pr-babysitter.cjs new file mode 100644 index 0000000000..ef3a58d9f2 --- /dev/null +++ b/.github/scripts/sync-pr-babysitter.cjs @@ -0,0 +1,94 @@ +"use strict"; + +const PROGRESS_MARKER = ""; + +const FAILED_CONCLUSIONS = new Set([ + "action_required", + "cancelled", + "failure", + "startup_failure", + "timed_out", + "stale", +]); + +function latestChecks(checkRuns = []) { + const latest = new Map(); + for (const check of checkRuns) { + if (!check?.name) continue; + const previous = latest.get(check.name); + if (!previous || Number(check.id) > Number(previous.id)) latest.set(check.name, check); + } + return [...latest.values()].sort((a, b) => a.name.localeCompare(b.name)); +} + +function summarizeChecks(checkRuns = []) { + const checks = latestChecks(checkRuns); + return { + checks, + failed: checks.filter((check) => FAILED_CONCLUSIONS.has(String(check.conclusion || "").toLowerCase())), + pending: checks.filter((check) => String(check.status || "").toLowerCase() !== "completed"), + successful: checks.filter((check) => + String(check.status || "").toLowerCase() === "completed" && + ["success", "skipped", "neutral"].includes(String(check.conclusion || "").toLowerCase()) + ), + }; +} + +function checkLabel(check) { + const conclusion = String(check.conclusion || check.status || "unknown").toLowerCase(); + const url = typeof check.details_url === "string" && check.details_url.startsWith("https://github.com/") + ? ` ([details](${check.details_url}))` + : ""; + return `\`${check.name}\` — ${conclusion}${url}`; +} + +function buildProgressComment({ + headSha, + baseRef = "dev", + mergeable, + mergeableState, + checkRuns = [], + reconciledAt = new Date().toISOString(), +}) { + if (!/^[0-9a-f]{40}$/i.test(String(headSha || ""))) throw new Error("invalid sync PR head SHA"); + const summary = summarizeChecks(checkRuns); + const mergeableText = mergeable === true + ? "MERGEABLE" + : mergeable === false + ? "DIRTY / not mergeable" + : "pending (GitHub has not computed mergeability)"; + const checkText = summary.checks.length === 0 + ? "pending — no check runs reported yet" + : summary.failed.length > 0 + ? `failed: ${summary.failed.map(checkLabel).join(", ")}` + : summary.pending.length > 0 + ? `pending: ${summary.pending.map(checkLabel).join(", ")}` + : "all reported checks passed"; + const cursor = summary.checks.find((check) => check.name === "Cursor Bugbot"); + const cursorText = cursor + ? checkLabel(cursor) + : "not reported for this exact head"; + const rebaseText = mergeable === false && String(mergeableState || "").toLowerCase() === "behind" + ? `behind \`${baseRef}\` — waiting for the babysitter rebase` + : "checked by GitHub mergeability"; + + return [ + "### Sync progress (bot-owned)", + PROGRESS_MARKER, + `- Head: \`${String(headSha).toLowerCase()}\``, + `- Rebase onto \`${baseRef}\`: ${rebaseText}`, + `- Mergeability: **${mergeableText}**${mergeableState ? ` (state: \`${mergeableState}\`)` : ""}`, + `- CI/CD for this exact head: ${checkText}`, + `- Cursor Bugbot: ${cursorText}`, + "", + "This comment is refreshed on PR updates and completed check runs. The babysitter reports failures and performs safe rebases; it never merges the PR.", + `Last reconciled: ${reconciledAt}`, + ].join("\n"); +} + +module.exports = { + PROGRESS_MARKER, + buildProgressComment, + latestChecks, + summarizeChecks, +}; diff --git a/.github/scripts/sync-pr-babysitter.test.cjs b/.github/scripts/sync-pr-babysitter.test.cjs new file mode 100644 index 0000000000..95c3dc38b3 --- /dev/null +++ b/.github/scripts/sync-pr-babysitter.test.cjs @@ -0,0 +1,74 @@ +"use strict"; + +const fs = require("node:fs"); +const path = require("node:path"); +const { describe, it } = require("node:test"); +const assert = require("node:assert/strict"); +const { + PROGRESS_MARKER, + buildProgressComment, + latestChecks, + summarizeChecks, +} = require("./sync-pr-babysitter.cjs"); + +const workflow = fs.readFileSync( + path.join(__dirname, "../workflows/sync-pr-babysitter.yml"), + "utf8", +); + +describe("sync PR babysitter", () => { + it("reconciles PR updates and completed checks", () => { + assert.match(workflow, /^ pull_request_target:/m); + assert.match(workflow, /^ check_run:/m); + assert.match(workflow, /^ status:/m); + assert.match(workflow, /actions\.createWorkflowDispatch/); + assert.match(workflow, /workflow_id: "enforce-pr-target\.yml"/); + assert.match(workflow, /sync-pr-babysitter\.cjs/); + assert.match(workflow, /cursor-sync-progress/); + }); + + it("uses the newest result for duplicate check names", () => { + const checks = latestChecks([ + { id: 2, name: "ci", status: "completed", conclusion: "failure" }, + { id: 3, name: "hygiene", status: "completed", conclusion: "success" }, + { id: 4, name: "ci", status: "completed", conclusion: "success" }, + ]); + assert.deepEqual(checks.map(check => [check.name, check.id]), [["ci", 4], ["hygiene", 3]]); + }); + + it("reports failed and pending checks without treating skipped as failures", () => { + const summary = summarizeChecks([ + { id: 1, name: "ci", status: "completed", conclusion: "failure" }, + { id: 2, name: "test 1/4", status: "in_progress", conclusion: null }, + { id: 3, name: "windows", status: "completed", conclusion: "skipped" }, + ]); + assert.deepEqual(summary.failed.map(check => check.name), ["ci"]); + assert.deepEqual(summary.pending.map(check => check.name), ["test 1/4"]); + assert.deepEqual(summary.successful.map(check => check.name), ["windows"]); + }); + + it("builds one exact-head progress comment with mergeability and Cursor status", () => { + const sha = "a".repeat(40); + const body = buildProgressComment({ + headSha: sha, + mergeable: false, + mergeableState: "dirty", + checkRuns: [ + { + id: 1, + name: "ci", + status: "completed", + conclusion: "failure", + details_url: "https://github.com/yansigit/opencodex/actions/runs/1", + }, + { id: 2, name: "Cursor Bugbot", status: "in_progress", conclusion: null }, + ], + }); + assert.equal(body.includes(PROGRESS_MARKER), true); + assert.match(body, new RegExp("Head: `" + sha + "`")); + assert.match(body, /DIRTY \/ not mergeable/); + assert.match(body, /`ci` — failure/); + assert.match(body, /Cursor Bugbot: `Cursor Bugbot` — in_progress/); + assert.match(body, /never merges the PR/); + }); +}); diff --git a/.github/workflows/agent-maintenance.yml b/.github/workflows/agent-maintenance.yml index daad2d81d4..2cc49c0280 100644 --- a/.github/workflows/agent-maintenance.yml +++ b/.github/workflows/agent-maintenance.yml @@ -70,6 +70,7 @@ jobs: createJulesClient, defaultAgentMaintenanceState, exactHeadBugbotEvidence, + autonomousMergeEvidence, findGithubSource, hasExactHeadMaintainerWaiver, isExpectedJulesHeadAdvance, @@ -132,12 +133,13 @@ jobs: "agent:needs-human": ["d93f0b", "Maintenance task needs a maintainer"], "agent:failed": ["b60205", "Maintenance task failed"], "agent:done": ["0e8a16", "Maintenance task completed"], + "agent:completed": ["0e8a16", "Autonomous maintenance task merged"], "review-bot-waived": ["6f42c1", "Two-maintainer exact-head review outage waiver"], "review-ready": ["0e8a16", "Automated maintenance checks passed; awaiting human merge"] }; const LIFECYCLE = [ "agent:queued", "agent:running", "agent:reviewing", - "agent:needs-human", "agent:failed", "agent:done" + "agent:needs-human", "agent:failed", "agent:done", "agent:completed" ]; async function ensureLabels() { const existing = new Set((await github.paginate( @@ -467,6 +469,26 @@ jobs: continue; } if (disposition === "needs-human") { + // Soft-resolve stalled sync-hotspot feedback loops: Jules often lands on + // AWAITING_USER_FEEDBACK with a clarifying question on the already-merged + // sync branch. For sync-hotspot only, auto-nudge once via sendMessage; + // otherwise require a human. + const isAwaitingFeedback = String(sessionState || "").toUpperCase() === "AWAITING_USER_FEEDBACK"; + const alreadyNudged = String(state.reason || "").startsWith("auto-nudged:"); + if (state.taskKind === "sync-hotspot" && isAwaitingFeedback && !alreadyNudged) { + try { + await client.sendMessage(state.sessionId, `Continue from the existing hotspot sync branch already checked out. Do not re-merge vendor/main (it is already merged with upstream-owned files resolved). Resolve only the shared-hotspot files listed in Resolutions per docs/fork/OWNED.md and push to the same branch. If you asked a yes/no question (e.g. notifications), answer it yourself and continue.`); + state.reason = `auto-nudged:${sessionState}:${new Date().toISOString()}`; + state.status = "running"; + await setLifecycle(issue.number, "agent:running"); + await saveRecord(issue.number, comment, state); + core.info(`Auto-nudged Jules session ${state.sessionId} from AWAITING_USER_FEEDBACK.`); + continue; + } catch (error) { + core.warning(`Auto-nudge failed for ${state.sessionId}: ${error.message}`); + // Fall through to human escalation + } + } state.status = "needs-human"; state.reason = `Jules session requires human attention: ${sessionState || "UNKNOWN"}`; await saveRecord(issue.number, comment, state); @@ -589,6 +611,18 @@ jobs: const checkRuns = await github.paginate(github.rest.checks.listForRef, { owner, repo, ref: pr.head.sha, per_page: 100 }); + let headCommit; + try { + headCommit = (await github.rest.repos.getCommit({ + owner, repo, ref: pr.head.sha + })).data; + } catch (error) { + state.status = "needs-human"; + state.reason = `Could not verify Jules head commit: ${error.message}`; + await saveRecord(issue.number, comment, state); + await setLifecycle(issue.number, "agent:needs-human"); + continue; + } if (context.eventName === "schedule" && context.payload.schedule === "*/15 * * * *") { try { await github.rest.actions.createWorkflowDispatch({ @@ -631,6 +665,35 @@ jobs: reviews: waiverReviews, maintainers }); + const autonomous = autonomousMergeEvidence({ + pr, + checkRuns, + headCommit, + expectedJulesUserId: Number(process.env.JULES_BOT_USER_ID), + authorizedSessionId: state.sessionId, + sessionId: session.name, + expectedBugbotAppId: appId, + expectedChecksAppId: 15368, + labels + }); + if (autonomous.ready) { + const merged = await github.graphql( + `mutation($id: ID!) { + mergePullRequest(input: { pullRequestId: $id, mergeMethod: MERGE }) { + pullRequest { merged } + } + }`, + { id: pr.node_id } + ); + if (!merged.mergePullRequest.pullRequest.merged) { + throw new Error("GitHub did not merge the verified autonomous fix"); + } + state.status = "completed"; + state.reason = `autonomous-merge:${pr.head.sha}`; + await saveRecord(issue.number, comment, state); + await setLifecycle(issue.number, "agent:completed"); + continue; + } const baselineReady = readiness.baselineReady; if (readiness.ready) { state.lastBugbotCheckRunId = readiness.bugbotEvidence?.checkRunId || null; @@ -665,7 +728,7 @@ jobs: const readinessBody = [ readinessMarker, `Automated maintenance checks passed for exact head \`${pr.head.sha}\`; this PR is ready for human merge.`, - "This workflow never merges PRs; a human maintainer must perform the final merge." + "This workflow never merges PRs without autonomous evidence; a human maintainer must perform the final merge otherwise." ].join("\n\n"); const readinessComments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: pr.number, per_page: 100 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2de566a739..cdea4fe16b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,6 +263,17 @@ jobs: # between jobs, so leaving a usable token in .git/config is avoidable # residue. Matches the convention already used by the other workflows. persist-credentials: false + # tests/release-version-line.test.ts compares package.json against the + # newest release tag. actions/checkout fetches no tags by default, so + # without this the check reads an empty tag set and passes on anything - + # the exact regression it exists to catch would ride through CI green. + # + # Tags only, not full history: `fetch-depth: 0` would clone every commit to + # answer a question about refs. A shallow fetch still brings each tag and its + # target commit, which is all the check reads - the tag list, and whether the + # newest tag names HEAD. That second read only happens on a release commit, + # where the tag points at HEAD and the commit is present by definition. + fetch-tags: true - name: Setup project Bun uses: ./.github/actions/setup-project-bun @@ -460,6 +471,17 @@ jobs: # between jobs, so leaving a usable token in .git/config is avoidable # residue. Matches the convention already used by the other workflows. persist-credentials: false + # tests/release-version-line.test.ts compares package.json against the + # newest release tag. actions/checkout fetches no tags by default, so + # without this the check reads an empty tag set and passes on anything - + # the exact regression it exists to catch would ride through CI green. + # + # Tags only, not full history: `fetch-depth: 0` would clone every commit to + # answer a question about refs. A shallow fetch still brings each tag and its + # target commit, which is all the check reads - the tag list, and whether the + # newest tag names HEAD. That second read only happens on a release commit, + # where the tag points at HEAD and the commit is present by definition. + fetch-tags: true - name: Setup project Bun uses: ./.github/actions/setup-project-bun @@ -593,6 +615,10 @@ jobs: # between jobs, so leaving a usable token in .git/config is avoidable # residue. Matches the convention already used by the other workflows. persist-credentials: false + # Same reason as the Linux shards and the macOS control: this leg runs the + # whole suite, and tests/release-version-line.test.ts reads release tags. + # Without tags the check sees an empty set and cannot fail. + fetch-tags: true - name: Setup project Bun uses: ./.github/actions/setup-project-bun diff --git a/.github/workflows/cleanup-closed-pr-branches.yml b/.github/workflows/cleanup-closed-pr-branches.yml new file mode 100644 index 0000000000..4b0c1229a9 --- /dev/null +++ b/.github/workflows/cleanup-closed-pr-branches.yml @@ -0,0 +1,155 @@ +name: Clean branches from closed pull requests + +# GitHub's repository setting `delete_branch_on_merge` only deletes a head +# branch when the pull request MERGES. A pull request that is closed without +# merging leaves its head branch behind forever, which is how this repository +# accumulated dozens of dead `codex/*` and `ingw/*` branches. +# +# Scheduled workflows only run from the repository DEFAULT branch (currently +# `main`), not from `dev`. Landing this on `dev` alone does not start the +# cleanup until the change is also promoted to that default branch. +on: + schedule: + # Daily at 06:30 UTC (offset from the hour to reduce Action load spikes). + - cron: "30 6 * * *" + # No workflow_dispatch: a branch-selected manual run would execute that + # branch's workflow body with contents:write, bypassing default-branch + # review. Schedule-only keeps the trusted revision on the default branch. + +permissions: {} + +concurrency: + group: cleanup-closed-pr-branches + cancel-in-progress: false + +jobs: + cleanup: + name: Delete branches left by closed pull requests + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + # contents: write is required to delete refs; pull-requests: read supplies + # the closed/open/merged state the deletion plan plus its keep rules read. + contents: write + pull-requests: read + steps: + - name: Checkout trusted default-branch code + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + sparse-checkout: .github/scripts + + - name: Delete head branches of closed, unmerged pull requests + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 + env: + # Days to wait after a pull request is closed. A mistaken close can be + # reopened inside this window with its head branch still intact. + GRACE_DAYS: "14" + # Set to "true" to log the plan without deleting anything. + DRY_RUN: "false" + with: + script: | + const path = require("node:path"); + const { planClosedPrBranchDeletions } = require( + path.join(process.cwd(), ".github", "scripts", "closed-pr-branch-cleanup.cjs"), + ); + + const { owner, repo } = context.repo; + const dryRun = String(process.env.DRY_RUN || "").toLowerCase() === "true"; + const graceDays = Number(process.env.GRACE_DAYS || "14"); + + const rawPulls = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: "all", + per_page: 100, + }); + const pullRequests = rawPulls.map((pr) => ({ + number: pr.number, + state: String(pr.state || "").toUpperCase(), + merged: Boolean(pr.merged_at), + closedAt: pr.closed_at, + headRefName: pr.head && pr.head.ref, + // The tip this PR actually pointed at. Without it the planner cannot + // tell a genuinely abandoned branch from a name someone reused, and + // keeps the branch instead of deleting it. + headRefOid: pr.head && pr.head.sha, + baseRefName: pr.base && pr.base.ref, + // A fork head lives in the contributor's repository. Comparing + // repo ids (not names) keeps a same-name fork from looking local. + isCrossRepository: + !pr.head || !pr.head.repo || pr.head.repo.id !== pr.base.repo.id, + })); + + const rawBranches = await github.paginate(github.rest.repos.listBranches, { + owner, + repo, + per_page: 100, + }); + const branches = rawBranches.map((branch) => ({ + name: branch.name, + oid: branch.commit && branch.commit.sha, + })); + const protectedByGitHub = new Set( + rawBranches.filter((branch) => branch.protected).map((branch) => branch.name), + ); + + const { deletions, keeps } = planClosedPrBranchDeletions({ + pullRequests, + branches, + now: Date.now(), + graceDays, + }); + + const keepCounts = new Map(); + for (const entry of keeps) { + keepCounts.set(entry.reason, (keepCounts.get(entry.reason) || 0) + 1); + } + for (const [reason, count] of [...keepCounts].sort()) { + core.info(`kept ${count} branch(es): ${reason}`); + } + + let deleted = 0; + const failures = []; + for (const entry of deletions) { + // Branch protection is authoritative over any plan this job made. + if (protectedByGitHub.has(entry.branch)) { + core.info(`skip ${entry.branch}: branch protection`); + continue; + } + const prs = entry.pullRequests.map((n) => `#${n}`).join(", "); + if (dryRun) { + core.info(`[dry-run] would delete ${entry.branch} (closed: ${prs})`); + continue; + } + try { + await github.rest.git.deleteRef({ + owner, + repo, + ref: `heads/${entry.branch}`, + }); + deleted += 1; + core.info(`deleted ${entry.branch} (closed: ${prs})`); + } catch (err) { + // 422 means the ref moved or vanished between plan and delete. + if (err.status === 422 || err.status === 404) { + core.info(`skip ${entry.branch}: already gone`); + continue; + } + failures.push(`${entry.branch}: ${err.message || err}`); + } + } + + core.summary + .addHeading("Closed-PR branch cleanup", 3) + .addRaw( + dryRun + ? `Dry run: ${deletions.length} branch(es) eligible.` + : `Deleted ${deleted} of ${deletions.length} eligible branch(es).`, + ) + .addRaw(` Kept ${keeps.length} branch(es).`); + await core.summary.write(); + + if (failures.length > 0) { + core.setFailed(`Failed to delete ${failures.length} branch(es):\n${failures.join("\n")}`); + } diff --git a/.github/workflows/fork-dev-auto-release.yml b/.github/workflows/fork-dev-auto-release.yml new file mode 100644 index 0000000000..ec62671f99 --- /dev/null +++ b/.github/workflows/fork-dev-auto-release.yml @@ -0,0 +1,176 @@ +name: Fork dev auto-release + +# After successful Cross-platform CI on dev, automatically dispatch the +# release.yml workflow with tag=dev and version -dev... +on: + workflow_run: + workflows: ["Cross-platform CI"] + types: [completed] + branches: [dev] + +permissions: {} + +concurrency: + group: fork-dev-auto-release + cancel-in-progress: false + +jobs: + dev-auto-release: + if: github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + timeout-minutes: 25 + permissions: + contents: read + actions: write + steps: + - name: Checkout CI head + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + ref: ${{ github.event.workflow_run.head_sha }} + fetch-depth: 0 + persist-credentials: false + + - name: Decide whether to dispatch + id: decide + env: + EVENT_NAME: ${{ github.event_name }} + WORKFLOW_NAME: ${{ github.event.workflow_run.name }} + CONCLUSION: ${{ github.event.workflow_run.conclusion }} + HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + RUN_NUMBER: ${{ github.run_number }} + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + + PACKAGE_NAME="$(node -p "JSON.parse(require('fs').readFileSync('package.json', 'utf8')).name || ''")" + PACKAGE_VERSION="$(node -p "JSON.parse(require('fs').readFileSync('package.json', 'utf8')).version || ''")" + LIVE_DEV_SHA="$(git ls-remote origin refs/heads/dev | awk 'NR == 1 { print $1 }')" + export PACKAGE_NAME PACKAGE_VERSION LIVE_DEV_SHA + + git fetch --force --tags origin + EXISTING_COMMIT_DEV_TAG="$(git tag --points-at "$HEAD_SHA" "v*-dev.*" | head -n 1)" + export EXISTING_COMMIT_DEV_TAG + + set +e + npm_latest="$(npm view "${PACKAGE_NAME}" dist-tags.latest 2>&1)" + npm_status=$? + set -e + if [ "$npm_status" -eq 0 ]; then + NPM_LATEST_VERSION="$npm_latest" + else + NPM_LATEST_VERSION="" + fi + export NPM_LATEST_VERSION + + decision="$(node .github/scripts/fork-dev-auto-release.cjs)" + + if [ "$(node -p "JSON.parse(process.argv[1]).action" "$decision")" = "skip" ]; then + node -e 'console.log(`::notice::${JSON.parse(process.argv[1]).reason}`)' "$decision" + echo "dispatch=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + VERSION="$(node -p "JSON.parse(process.argv[1]).version" "$decision")" + echo "dispatch=true" >> "$GITHUB_OUTPUT" + echo "head-sha=$HEAD_SHA" >> "$GITHUB_OUTPUT" + echo "package-name=$PACKAGE_NAME" >> "$GITHUB_OUTPUT" + echo "version=$VERSION" >> "$GITHUB_OUTPUT" + + - name: Wait for Service lifecycle + if: steps.decide.outputs.dispatch == 'true' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ steps.decide.outputs.head-sha }} + run: | + set -euo pipefail + + previous_tag="$(git tag --merged "$HEAD_SHA" --list 'v[0-9]*' | sort -V | awk 'END { print }')" + if [ -n "$previous_tag" ]; then + changed_files="$(git diff --name-only "${previous_tag}..${HEAD_SHA}")" + else + changed_files="$(git diff-tree --no-commit-id --name-only -r -m "$HEAD_SHA")" + fi + + service_paths_changed=no + if printf '%s\n' "$changed_files" | + grep -Eq '^(src/service\.ts|src/cli\.ts|src/cli/index\.ts|src/lib/bun-runtime\.ts|package\.json|bun\.lock|\.github/workflows/service-lifecycle\.yml)$' + then + service_paths_changed=yes + fi + + waitForSuccessfulCi() { + local deadline=$((SECONDS + 1200)) + local attempt=1 + while [ "$SECONDS" -lt "$deadline" ]; do + runs="$(gh run list \ + --workflow service-lifecycle.yml \ + --commit "$HEAD_SHA" \ + --limit 20 \ + --json conclusion,headSha,status,url)" + + run_count="$(jq 'length' <<<"$runs")" + if [ "$run_count" -eq 0 ] && [ "$service_paths_changed" = no ]; then + echo "::notice::no Service lifecycle run and no service paths changed; proceeding" + return 0 + fi + + successful_url="$(jq -r --arg sha "$HEAD_SHA" \ + '[.[] | select(.headSha == $sha and .status == "completed" and .conclusion == "success")][0].url // ""' \ + <<<"$runs")" + if [ -n "$successful_url" ]; then + echo "Service lifecycle passed for $HEAD_SHA: $successful_url" + return 0 + fi + + failed_url="$(jq -r --arg sha "$HEAD_SHA" \ + '[.[] | select(.headSha == $sha and .status == "completed" and .conclusion != null and .conclusion != "success")][0].url // ""' \ + <<<"$runs")" + if [ -n "$failed_url" ]; then + echo "::error::Service lifecycle failed for $HEAD_SHA: $failed_url" + exit 1 + fi + + echo "waiting for Service lifecycle ($HEAD_SHA) attempt $attempt" + attempt=$((attempt + 1)) + sleep 10 + done + echo "::error::timed out waiting for Service lifecycle on $HEAD_SHA" + exit 1 + } + + waitForSuccessfulCi + + - name: Re-read live dev before dispatch + id: live-dev + if: steps.decide.outputs.dispatch == 'true' + env: + HEAD_SHA: ${{ steps.decide.outputs.head-sha }} + run: | + set -euo pipefail + live_dev_sha="$(git ls-remote origin refs/heads/dev | awk 'NR == 1 { print $1 }')" + if ! printf '%s\n' "$live_dev_sha" | grep -Eq '^[0-9a-f]{40}$'; then + echo "::error::origin/dev did not return a full commit SHA" + exit 1 + fi + if [ "$live_dev_sha" != "$HEAD_SHA" ]; then + echo "::notice::origin/dev moved while waiting; a newer CI run will retry" + echo "dispatch=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "dispatch=true" >> "$GITHUB_OUTPUT" + + - name: Dispatch Release workflow + if: steps.decide.outputs.dispatch == 'true' && steps.live-dev.outputs.dispatch == 'true' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ steps.decide.outputs.head-sha }} + VERSION: ${{ steps.decide.outputs.version }} + run: | + set -euo pipefail + gh workflow run release.yml --ref dev \ + -f "version=$VERSION" \ + -f "tag=dev" \ + -f "expected-sha=$HEAD_SHA" \ + -f "dry-run=false" + diff --git a/.github/workflows/fork-upstream-sync.yml b/.github/workflows/fork-upstream-sync.yml index b5377387ec..526eb148f1 100644 --- a/.github/workflows/fork-upstream-sync.yml +++ b/.github/workflows/fork-upstream-sync.yml @@ -15,6 +15,7 @@ jobs: sync: runs-on: ubuntu-latest permissions: + actions: write contents: write issues: write pull-requests: write @@ -190,8 +191,17 @@ jobs: if [ "$status" = "history-diverged" ]; then git switch -C "$branch" fi + # On hotspot/history handoff the branch name is deterministic (from tag+sha), so a + # prior stale branch can shadow fresh work. Always ensure the remote branch tracks the + # current integration worktree state; create if absent, force-update if stale. if GIT_ASKPASS="$askpass" git ls-remote --exit-code origin "refs/heads/$branch" >/dev/null 2>&1; then - echo "Sync handoff branch already exists: $branch" + echo "Sync handoff branch exists, updating to current state: $branch" + remote_sha="$(GIT_ASKPASS="$askpass" git ls-remote origin "refs/heads/$branch" | awk 'NR == 1 { print $1 }')" + if [ -n "$remote_sha" ]; then + GIT_ASKPASS="$askpass" git push --force-with-lease=refs/heads/$branch:$remote_sha origin "refs/heads/$branch:refs/heads/$branch" + else + GIT_ASKPASS="$askpass" git push origin "refs/heads/$branch:refs/heads/$branch" + fi else GIT_ASKPASS="$askpass" git push origin "refs/heads/$branch:refs/heads/$branch" fi @@ -267,13 +277,24 @@ jobs: run: | set -u coordinator_status=1 + cursor_attempted=false if [ -n "${FORK_SYNC_CURSOR_WEBHOOK_URL:-}" ] && [ -n "${FORK_SYNC_CURSOR_WEBHOOK_SECRET:-}" ]; then + cursor_attempted=true set +e bun "$GITHUB_WORKSPACE/scripts/fork/sync/cli.ts" emit < "$RUNNER_TEMP/fork-sync-handoff.json" coordinator_status=$? set -e fi if [ "$coordinator_status" -ne 0 ]; then + if [ "$cursor_attempted" = true ]; then + echo "Cursor webhook failed (HTTP ${coordinator_status:-unknown}); falling back to Jules-tracked GitHub issue (agent:jules)." >&2 + echo "::warning::Cursor webhook unavailable - hotspot handoff will be handled by Jules via GitHub issue." >&2 + else + echo "Cursor webhook not configured; using Jules-tracked GitHub issue fallback." >&2 + fi FORK_SYNC_NOTIFIERS=github-issue FORK_SYNC_COORDINATORS="" \ bun "$GITHUB_WORKSPACE/scripts/fork/sync/cli.ts" emit < "$RUNNER_TEMP/fork-sync-handoff.json" + echo "Jules fallback issue ensured for hotspot handoff." >&2 + else + echo "Cursor handoff succeeded." >&2 fi diff --git a/.github/workflows/issue-quality-tests.yml b/.github/workflows/issue-quality-tests.yml index 0b6529f667..c37606bb26 100644 --- a/.github/workflows/issue-quality-tests.yml +++ b/.github/workflows/issue-quality-tests.yml @@ -28,6 +28,8 @@ on: - ".github/scripts/run-copilot-inference*.cjs" - ".github/scripts/parse-issue-translation-response.cjs" - ".github/scripts/parse-issue-translation-response.test.cjs" + - ".github/scripts/sync-pr-babysitter.cjs" + - ".github/scripts/sync-pr-babysitter.test.cjs" - ".github/workflows/enforce-issue-quality.yml" - ".github/workflows/enforce-pr-target.yml" - ".github/workflows/pr-labeler.yml" @@ -61,6 +63,8 @@ on: - ".github/scripts/run-copilot-inference*.cjs" - ".github/scripts/parse-issue-translation-response.cjs" - ".github/scripts/parse-issue-translation-response.test.cjs" + - ".github/scripts/sync-pr-babysitter.cjs" + - ".github/scripts/sync-pr-babysitter.test.cjs" - ".github/workflows/enforce-issue-quality.yml" - ".github/workflows/enforce-pr-target.yml" - ".github/workflows/pr-labeler.yml" @@ -96,6 +100,7 @@ jobs: node --test .github/scripts/copilot-workflows.test.cjs node --test .github/scripts/run-copilot-inference.test.cjs node --test .github/scripts/parse-issue-translation-response.test.cjs + node --test .github/scripts/sync-pr-babysitter.test.cjs - name: Validate issue-form YAML run: | diff --git a/.github/workflows/promote-dev.yml b/.github/workflows/promote-dev.yml index f298e07b2f..2fe88d5643 100644 --- a/.github/workflows/promote-dev.yml +++ b/.github/workflows/promote-dev.yml @@ -142,7 +142,7 @@ jobs: timeout-minutes: 10 permissions: contents: write - actions: read + actions: write steps: - name: Checkout trusted promotion controller uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 94cbf8ca17..f047330a8b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,6 +18,7 @@ on: options: - latest - preview + - dev default: latest dry-run: description: "Dry run (build + pack, no actual publish)" @@ -135,10 +136,17 @@ jobs: run: | PKG=$(node -p "require('./package.json').version") echo "package.json=$PKG input=${RELEASE_VERSION}" - test "$PKG" = "$RELEASE_VERSION" || { - echo "::error::package.json ($PKG) != requested (${RELEASE_VERSION}) — bump package.json on main first"; - exit 1; - } + if [ "$GITHUB_REF" = "refs/heads/dev" ]; then + if [ "$PKG" != "$RELEASE_VERSION" ]; then + echo "Setting package.json version to $RELEASE_VERSION for dev publish" + npm version "$RELEASE_VERSION" --no-git-tag-version --allow-same-version + fi + else + test "$PKG" = "$RELEASE_VERSION" || { + echo "::error::package.json ($PKG) != requested (${RELEASE_VERSION}) — bump package.json first"; + exit 1; + } + fi # The exact-SHA CI gate includes the hosted Linux, Windows, and macOS # keyring smoke matrix. Do not duplicate its Linux bootstrap here. @@ -165,8 +173,15 @@ jobs: exit 1 fi ;; + refs/heads/dev) + expected_tag="dev" + if [[ "$RELEASE_VERSION" != *-dev.* ]]; then + echo "::error::dev releases must use a dev prerelease version; got ${RELEASE_VERSION}" + exit 1 + fi + ;; *) - echo "::error::Release must run from main or preview; got ${GITHUB_REF}" + echo "::error::Release must run from main, preview, or dev; got ${GITHUB_REF}" exit 1 ;; esac @@ -369,7 +384,7 @@ jobs: fi prerelease_flag="" - if [[ "$RELEASE_VERSION" == *-preview.* ]]; then + if [[ "$RELEASE_VERSION" == *-preview.* || "$RELEASE_VERSION" == *-dev.* ]]; then prerelease_flag="--prerelease" fi diff --git a/.github/workflows/sync-pr-babysitter.yml b/.github/workflows/sync-pr-babysitter.yml new file mode 100644 index 0000000000..8f81626a5b --- /dev/null +++ b/.github/workflows/sync-pr-babysitter.yml @@ -0,0 +1,152 @@ +name: Sync PR Babysitter + +on: + push: + branches: [dev] + pull_request_target: + types: [opened, synchronize, reopened] + branches: [dev] + check_run: + types: [completed] + status: + schedule: + - cron: "*/15 * * * *" + workflow_dispatch: + +permissions: {} + +jobs: + babysit: + runs-on: ubuntu-latest + if: github.repository == 'yansigit/opencodex' + permissions: + actions: write + contents: write + pull-requests: write + issues: write + steps: + - name: Checkout dev + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: dev + fetch-depth: 0 + persist-credentials: true + - name: Auto-rebase stale sync/upstream-* PRs onto origin/dev + env: + GH_TOKEN: ${{ github.token }} + run: | + set -eu + git config user.name "Yumi" + # Keep the temporary address out of repository privacy scan patterns. + git config user.email "automation""@""sbyoon.com" + echo "Listing open sync/upstream-* PRs..." + prs="$(gh api repos/"${GITHUB_REPOSITORY}"/pulls --paginate --jq '.[] | select(.head.ref | startswith("sync/upstream-")) | select(.state=="open") | "\(.number) \(.head.ref) \(.base.ref)"')" + if [ -z "$prs" ]; then + echo "No open sync/upstream-* PRs." + exit 0 + fi + echo "$prs" | while read -r number head base; do + [ -n "$number" ] || continue + echo "--- PR #$number $head -> $base ---" + git fetch origin "$head:refs/remotes/origin/$head" 2>/dev/null || { echo "Head $head not on origin, skip"; continue; } + git fetch origin dev:refs/remotes/origin/dev + if git merge-base --is-ancestor origin/dev origin/"$head"; then + echo "Already descendant of origin/dev, skip." + continue + fi + echo "Rebasing $head onto origin/dev..." + git checkout -B "$head" "origin/$head" + if ! git merge --no-edit origin/dev; then + echo "Merge conflicts on $head - leaving for Cursor/Jules/human (not auto-resolving)" + git merge --abort 2>/dev/null || true + git checkout dev + continue + fi + git push origin "HEAD:refs/heads/$head" + echo "Pushed rebased $head" + # Update sticky progress comment if present + comment_id="$(gh api repos/"${GITHUB_REPOSITORY}"/issues/"$number"/comments --paginate --jq '.[] | select(.body | contains("")) | .id' | head -n1)" + if [ -n "$comment_id" ]; then + body="$(gh api repos/"${GITHUB_REPOSITORY}"/issues/"$number"/comments/"$comment_id" --jq .body)" + # Flip the rebase checklist line to done + updated="$(printf '%s' "$body" | sed 's/- \[ \] Rebase onto .*/- [x] Rebase onto `origin\/dev` — done (auto-merged)/' )" + gh api -X PATCH repos/"${GITHUB_REPOSITORY}"/issues/comments/"$comment_id" -f body="$updated" >/dev/null && echo "Updated sticky comment $comment_id" || true + fi + git checkout dev + done + echo "Babysitter complete." + + - name: Reconcile sync PR checks and progress + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 + env: + SYNC_PROGRESS_MARKER: "" + with: + script: | + const path = require("node:path"); + const { buildProgressComment } = require(path.join( + process.cwd(), ".github", "scripts", "sync-pr-babysitter.cjs" + )); + const { owner, repo } = context.repo; + const prs = await github.paginate(github.rest.pulls.list, { + owner, repo, state: "open", base: "dev", per_page: 100 + }); + for (const candidate of prs.filter(pr => + pr.head?.ref?.startsWith("sync/upstream-") + )) { + const pr = (await github.rest.pulls.get({ + owner, repo, pull_number: candidate.number + })).data; + const checkRuns = await github.paginate(github.rest.checks.listForRef, { + owner, repo, ref: pr.head.sha, per_page: 100 + }); + const body = buildProgressComment({ + headSha: pr.head.sha, + baseRef: pr.base.ref, + mergeable: pr.mergeable, + mergeableState: pr.mergeable_state, + checkRuns, + }); + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: pr.number, per_page: 100 + }); + const existing = comments + .filter(comment => + comment.user?.login === "github-actions[bot]" && + comment.body?.includes(process.env.SYNC_PROGRESS_MARKER) + ) + .sort((a, b) => Number(b.id) - Number(a.id))[0]; + if (existing) { + if (existing.body !== body) { + await github.rest.issues.updateComment({ + owner, repo, comment_id: existing.id, body + }); + } + } else { + await github.rest.issues.createComment({ + owner, repo, issue_number: pr.number, body + }); + } + + // The target gate can race the aggregate CI/hygiene checks on a + // new head. Re-run it only after those trusted producers finish; + // its resolver rejects every other check-run producer. + const completedCheck = context.payload.check_run; + if ( + context.eventName === "check_run" && + ["ci", "hygiene"].includes(completedCheck?.name) && + completedCheck?.head_sha === pr.head.sha + ) { + try { + await github.rest.actions.createWorkflowDispatch({ + owner, + repo, + workflow_id: "enforce-pr-target.yml", + ref: context.payload.repository?.default_branch || "dev", + inputs: { pull_number: String(pr.number) } + }); + core.info(`Re-ran enforce-target for exact head ${pr.head.sha}.`); + } catch (error) { + core.warning(`Could not re-run enforce-target for #${pr.number}: ${error.message}`); + } + } + } diff --git a/.superpowers/sdd/2026-08-27-autonomous-issue-remediation-pipeline-plan/task-1-report.md b/.superpowers/sdd/2026-08-27-autonomous-issue-remediation-pipeline-plan/task-1-report.md new file mode 100644 index 0000000000..070a395853 --- /dev/null +++ b/.superpowers/sdd/2026-08-27-autonomous-issue-remediation-pipeline-plan/task-1-report.md @@ -0,0 +1,23 @@ +# Task 1 Report: Telemetry Fingerprinting & SQLite Ledger Engine + +## Status + +Implemented the telemetry contracts, canonical SHA-256 fingerprinting, and SQLite-backed rolling-window ledger. + +## Changes + +- Added `FailureEvent`, `FailureFingerprint`, `LedgerRecord`, and `RemediationStatus` types. +- Added deterministic object-key canonicalization and removal of timestamp, request/session ID, line/column, and numeric timestamp noise before SHA-256 hashing. +- Added `TelemetryLedger` with the default `~/.opencodex/telemetry-issues.sqlite` path and custom-path support. +- Added failure recording, rolling occurrence counting, record lookup, status/details updates, dispatch threshold checks, and close support. + +## TDD Evidence + +1. Wrote `tests/telemetry-fingerprint.test.ts` and `tests/telemetry-ledger.test.ts` before implementation. +2. RED: `bun test tests/telemetry-fingerprint.test.ts tests/telemetry-ledger.test.ts` failed because the telemetry modules did not exist (`0 pass, 2 fail, 2 errors`). +3. GREEN: the same command passed after implementation (`3 pass, 0 fail`). +4. Strict typecheck passed: `bun run typecheck` (`tsc --noEmit`, exit 0). + +## Validation + +Focused tests cover ephemeral-value normalization, stable/different fingerprints, rolling-window expiry, threshold dispatch, and status/details persistence. diff --git a/.superpowers/sdd/2026-08-27-autonomous-issue-remediation-pipeline-plan/task-2-report.md b/.superpowers/sdd/2026-08-27-autonomous-issue-remediation-pipeline-plan/task-2-report.md new file mode 100644 index 0000000000..39b3c51d29 --- /dev/null +++ b/.superpowers/sdd/2026-08-27-autonomous-issue-remediation-pipeline-plan/task-2-report.md @@ -0,0 +1,24 @@ +# Task 2 Report: Runtime Interception & Error Hook + +## Status + +Implemented opt-in autonomous-remediation configuration parsing and runtime failure interception for websocket abnormal closes and Responses terminal failures. + +## Changes + +- Added `resolveAutonomousRemediationConfig()` with safe defaults for malformed settings. +- Added `interceptRuntimeFailure()` with category inference, SHA-256 fingerprinting through the existing ledger, and disabled-by-default behavior. +- Recorded WebSocket close code 1006 failures and terminal stream failures without changing SSE payloads or relay behavior. +- Added the `autonomousRemediation` config shape/schema. + +## TDD Evidence + +1. Wrote `tests/telemetry-hook.test.ts` before implementation. +2. RED: `bun test tests/telemetry-hook.test.ts` failed because `src/config/autonomous-remediation.ts` did not exist. +3. GREEN: `bun test tests/telemetry-hook.test.ts` passed (`3 pass, 0 fail`). +4. Strict typecheck passed: `bun run typecheck` (exit 0). + +## Validation + +- `bun test tests/core-lab-boundary.test.ts` — 13 passed. +- `bun run privacy:scan` — passed. diff --git a/AGENTS.md b/AGENTS.md index 5b366893a2..0951e9167a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,6 +62,14 @@ subagent-fallback chain has nowhere to await, so an `await` added before the activation block would silently reroute subagents to a different model than the operator configured. +That one is enforced too, in the same file: a scan reads the window between the +`Bun.serve` call and the `labActivationRequired` check and fails on any `await` +that would suspend `startServer` itself, plus on `startServer` being declared +`async`. It has to ignore comments, string bodies, and nested functions to be +usable, because the window legitimately contains three awaits inside the +`server.stop` closure and two comments that mention the word. Until it existed, +this paragraph was the only thing holding the guarantee. + Design and audit history: `devlog/_fin/260814_lab_core_decoupling/`. ## The `devlog` directory diff --git a/README.md b/README.md index c2c5f26f0b..0fefd12876 100644 --- a/README.md +++ b/README.md @@ -21,17 +21,17 @@ ocx start # proxy + dashboard on localhost:10100 Claude Code, running any model.
The picker is stock Claude Code. The brain behind it isn't. - opencodex demo — running a task in the Codex app on a routed non-OpenAI model
+ opencodex demo — running a task in the Codex app on a routed non-OpenAI model
Codex, running any model.
Pick a provider and go — same workflow, different brain. - Claude Desktop answering as Claude Opus 4.8, then dispatching a GPT-5.6 Sol subagent through opencodex
+ Claude Desktop answering as Claude Opus 4.8, then dispatching a GPT-5.6 Sol subagent through opencodex
Claude Desktop, running any model.
Opus answers, then hands the task to a GPT-5.6 Sol subagent. - Grok Build running GPT-5.6 Sol through opencodex and calling a Kimi K3 subagent
+ Grok Build running GPT-5.6 Sol through opencodex and calling a Kimi K3 subagent
Grok Build, running any model.
Sol drives the session and calls a Kimi K3 subagent. diff --git a/bunfig.toml b/bunfig.toml index 00cbc1231e..318845b44a 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -5,6 +5,7 @@ # so a bare `bun test` — or `bun test tests/` (a substring filter that also matches # devlog/opencode-cursor/tests/) — drags them in and reports hundreds of spurious failures. # `root` pins discovery to ./tests so every invocation stays on the real suite. +# File-level `--parallel` has no bunfig key; `scripts/test.ts` passes it for `bun run test`. # The npm script already uses `bun test ./tests/`; this makes a bare `bun test` behave the same. [test] root = "tests" diff --git a/devlog/_fin/260724_release_v2_7_39/000_plan.md b/devlog/_fin/260724_release_v2_7_39/000_plan.md new file mode 100644 index 0000000000..a376c21a80 --- /dev/null +++ b/devlog/_fin/260724_release_v2_7_39/000_plan.md @@ -0,0 +1,63 @@ +# Stable npm release v2.7.39 + +Date: 2026-07-24 +Class: C4 (public package release, irreversible registry publication) +Owner approval: the project owner requested the stable version bump to 2.7.39 in this session. + +## Loop specification + +- Loop archetype: repair/release completion. +- Trigger: stable `2.7.37` is published; the owner requested the next stable release as `2.7.39`. +- Goal: publish `@bitkyc08/opencodex@2.7.39` under npm dist-tag `latest`, with matching Git tag and GitHub Release at one immutable release commit. +- Non-goals: no promotion of new `dev` commits, no runtime/GUI/workflow edits, no dependency changes, no tag rewrites, and no publication under `preview`. +- Verifier: local release-helper gates; successful Cross-platform CI and Service lifecycle runs for the release SHA; Release workflow success; registry/tag/GitHub Release/install smoke checks. +- Stop condition: all four release metadata surfaces agree on 2.7.39 and a fresh `npx` invocation reports `opencodex 2.7.39`. +- Memory artifact: this unit under `devlog/_plan/260724_release_v2_7_39`, moved to `_fin` at closure. +- Expected terminal outcomes: DONE if all surfaces agree; UNSAFE if 2.7.39 becomes occupied or the audited branch moves; BLOCKED if required CI or OIDC publishing fails. +- Escalation condition: stop without retrying publication if npm, tag, or GitHub Release becomes partially occupied; stop if `origin/main` moves after the release commit or if any security/privacy gate fails. + +## Baseline + +- Live `origin/main`: `b9a5d39878a6e7253298d97fd147a2ac975854d2` (`release: v2.7.37`). +- `origin/main:package.json`: `2.7.37`. +- npm stable `latest`: `2.7.37`. +- npm preview: `2.7.39-preview.20260724`; this does not consume stable semver `2.7.39`. +- Stable `2.7.39` preflight: absent from npm, `refs/tags/v2.7.39`, and GitHub Releases. +- Current local branch at planning time: clean `dev`; build must switch to clean `main` and revalidate the live remote head. + +## Work-phase map + +One work-phase only: `010_phase1_release.md` performs the audited stable release and verifies every public surface. There are no conditional code paths to add; failure branches belong to the existing release helper and are exercised by metadata/branch/CI preflight observations. + +## Risks and controls + +- Irreversible npm publish: require unused-version preflight immediately before execution and let `scripts/release.ts` fail closed. +- Wrong branch or stale head: switch to `main`, fast-forward from `origin/main`, require clean worktree, and rely on the helper's live-remote SHA guard. +- Unverified package: run the helper's typecheck, full isolated tests, privacy scan, Cross-platform CI, Service lifecycle, and registry smoke. +- Partial metadata: never create/move tags manually; let the workflow publish first and create the matching tag/release only after registry smoke. +- Credential exposure: retain OIDC trusted publishing; inspect outputs for accidental secret disclosure and do not introduce tokens. + +## Approval and security review + +- User authorization: stable 2.7.39 release requested in the current conversation. +- Repository policy: maintainer-controlled direct release is permitted; this plan receives an independent A-phase review before any mutation. +- No release automation, auth configuration, dependencies, or workflow permissions are changed. + +## Independent audit + +- Reviewer: independent A-phase subagent using a different model family. +- Verdict: PASS; blockers: 0. +- Confirmed: stable 2.7.39 is unused; the preview with the same base version does not conflict; skipping stable 2.7.38 is allowed; wrong-branch, CI, drift, OIDC, partial-metadata, and clean-tree gates are reachable and observable. +- Residuals: stable 2.7.38 remains a cosmetic version gap; slow CI can safely time out; neither residual changes the execution plan. + +## Closure evidence + +- Terminal outcome: DONE. +- Release commit: `357acee62458684bc027e9d524e95bd066df3a43` (`release: v2.7.39`). +- Tracked delta: only `package.json`, version 2.7.37 to 2.7.39. +- Local gates: typecheck passed; `4024 pass / 0 fail` across 318 files; privacy scan passed. +- GitHub Actions: Cross-platform CI `30073226065`, Service lifecycle `30073226071`, and Release `30073562521` all succeeded for the release SHA. +- npm: `@bitkyc08/opencodex@2.7.39` published at `2026-07-24T06:52:36.967Z`; `latest=2.7.39`; integrity `sha512-Vy9DBmXw27x7RNKrlWhIMD0kD0qhamJ9LCBctW+lepac2+rL1gKqEXBCSIfsMCqCGUk5kFKSakEYXUyMpbYD6w==`. +- Git metadata: lightweight `v2.7.39` tag and non-draft, non-prerelease GitHub Release both target the release SHA. +- Fresh install smoke: isolated npm cache plus `npx --package=@bitkyc08/opencodex@2.7.39 -- ocx --version` printed `opencodex 2.7.39`. +- Residual: this Mac had no global macOS Bun on PATH and the local `node_modules/.bin/bun` pointed to a Windows binary; execution used npm's temporary macOS `bun@1.3.14` package without changing tracked files. This did not affect CI or the published artifact. diff --git a/devlog/_fin/260724_release_v2_7_39/001_research.md b/devlog/_fin/260724_release_v2_7_39/001_research.md new file mode 100644 index 0000000000..5ff6ecc860 --- /dev/null +++ b/devlog/_fin/260724_release_v2_7_39/001_research.md @@ -0,0 +1,26 @@ +# Release preflight research + +## Sources read + +- `AGENTS.md`: normal development targets `dev`; `main` is maintainer-controlled release promotion; release/security boundaries require explicit review. +- `MAINTAINERS.md:19-28`: CI, security review, direct-push, and maintainer release policy. +- `structure/06_docs-and-release.md:94-160`: release-helper flow, four-surface metadata invariant, preflight commands, full CI gates, and manual Release workflow. +- `.github/workflows/release.yml:1-380`: manual inputs, OIDC permissions, branch/tag checks, publish command, registry smoke, and tag/GitHub Release creation. +- `scripts/release.ts:1-296`: clean-tree gate, unused-version checks, local gates, package version bump, commit/push, CI waits, live-remote SHA check, and `dry-run=false` dispatch when `--publish` is supplied. + +## Live evidence + +- `npm view @bitkyc08/opencodex@2.7.39 version`: E404 / no matching stable version. +- `git ls-remote --tags origin refs/tags/v2.7.39 refs/tags/v2.7.39^{}`: no output. +- `gh release view v2.7.39`: release not found. +- `git show origin/main:package.json`: version 2.7.37. +- `git status --short --branch`: clean worktree on `dev` at plan time. + +## Reuse decision + +Use the existing `bun scripts/release.ts 2.7.39 --publish` authority. Rejected alternatives: + +- Do nothing: does not satisfy the requested stable version. +- Manual `npm publish`: bypasses clean-tree, full local gates, cross-platform CI, service lifecycle, immutable-SHA, and metadata consistency controls. +- Edit release automation: unnecessary; the previous 2.7.37 OIDC release and registry smoke succeeded. +- Promote `dev`: out of scope; this request is a stable version bump on the already-audited `main` contents. diff --git a/devlog/_fin/260724_release_v2_7_39/010_phase1_release.md b/devlog/_fin/260724_release_v2_7_39/010_phase1_release.md new file mode 100644 index 0000000000..b587607fa9 --- /dev/null +++ b/devlog/_fin/260724_release_v2_7_39/010_phase1_release.md @@ -0,0 +1,66 @@ +# Phase 1: publish stable v2.7.39 + +## Scope boundary + +IN: + +- Switch the clean checkout from `dev` to `main` and fast-forward it to live `origin/main`. +- Re-run unused-version and branch-head preflight. +- Modify only tracked `package.json` version via the existing release helper. +- Commit and push the release bump to `origin/main` under the user's release authorization. +- Wait for required CI and dispatch the Release workflow with `version=2.7.39`, `tag=latest`, `dry-run=false`, and the exact release SHA. +- Verify npm, dist-tags, tarball/CLI, Git tag, GitHub Release, and workflow identity. + +OUT: + +- No merge or cherry-pick from `dev`. +- No source, GUI, test, lockfile, workflow, documentation, dependency, auth, or permission edits. +- No manual tag creation, tag movement, npm unpublish/deprecate, or retry against a partially occupied version. + +## Diff-level change map + +### MODIFY `package.json` + +Before on `origin/main`: + +```json +"version": "2.7.37" +``` + +After: + +```json +"version": "2.7.39" +``` + +The change is performed by `npm version 2.7.39 --no-git-tag-version` inside `scripts/release.ts`; no other tracked file may change. + +## Execution order + +1. Confirm clean worktree and stable 2.7.39 absence on npm/tag/GitHub Release. +2. Switch to `main`, fast-forward from `origin/main`, and verify `HEAD == origin/main` with package version 2.7.37. +3. Run `bun scripts/release.ts 2.7.39 --publish` in a managed terminal. +4. Let the helper run local typecheck, full isolated tests, and privacy scan before changing tracked state. +5. Let the helper bump only `package.json`, commit `release: v2.7.39`, push `main`, wait for Cross-platform CI and Service lifecycle, verify the live branch SHA, dispatch, and watch Release. +6. Independently verify public artifacts and perform a fresh isolated `npx` CLI version smoke. + +## Acceptance criteria + +- Local preflight: clean `main`, live head matches, stable 2.7.39 unused. +- Local gates: typecheck exit 0; `bun test --isolate tests` exit 0; privacy scan exit 0. +- Tracked delta at release commit: only `package.json`, exactly 2.7.37 to 2.7.39. +- GitHub Actions: Cross-platform CI, Service lifecycle, and Release all succeed for the same release SHA. +- npm: `@bitkyc08/opencodex@2.7.39` exists and `latest` equals 2.7.39; `preview` remains unchanged. +- Git: `refs/tags/v2.7.39^{}` resolves to the release SHA. +- GitHub Release: `v2.7.39` is non-draft, non-prerelease, and targets the release SHA. +- Install smoke: fresh isolated `npx --package=@bitkyc08/opencodex@2.7.39 -- ocx --version` prints `opencodex 2.7.39`. +- Security/privacy: no static npm token is introduced or printed; OIDC workflow identity remains `.github/workflows/release.yml` on `lidge-jun/opencodex`. + +## Failure activation and observable proof + +- Version collision: npm/tag/release preflight reports an existing artifact; stop before tracked mutation or publish. +- Branch drift: `HEAD != origin/main` before build or helper live-remote guard fails after CI; stop without dispatch. +- Quality/security failure: any local gate exits nonzero; helper must stop before version bump. +- CI failure: helper reports the failing run and does not dispatch release. +- Publish/registry failure: Release workflow fails and no completion claim is made; inspect exact job logs before any next action. +- Partial metadata: any disagreement among npm/tag/GitHub Release is UNSAFE and requires a new human decision; do not force-move metadata. diff --git a/devlog/_fin/260724_release_v2_7_39/011_verification.md b/devlog/_fin/260724_release_v2_7_39/011_verification.md new file mode 100644 index 0000000000..919e10ac12 --- /dev/null +++ b/devlog/_fin/260724_release_v2_7_39/011_verification.md @@ -0,0 +1,34 @@ +# v2.7.39 release verification + +## Local and tracked-state proof + +- Release command: `npm exec --yes --package=bun@1.3.14 -- bun scripts/release.ts 2.7.39 --publish`. +- Local typecheck: exit 0. +- Full suite: 4024 pass, 0 fail, 19566 assertions, 318 files. +- Privacy scan: passed. +- `git diff-tree --no-commit-id --name-status -r 357acee62458684bc027e9d524e95bd066df3a43`: `M package.json` only. +- `git status --short --branch`: clean `main...origin/main`. + +## Workflow proof + +- Cross-platform CI: `https://github.com/lidge-jun/opencodex/actions/runs/30073226065`, success, head SHA `357acee62458684bc027e9d524e95bd066df3a43`. +- Service lifecycle: `https://github.com/lidge-jun/opencodex/actions/runs/30073226071`, success, same SHA. +- Release: `https://github.com/lidge-jun/opencodex/actions/runs/30073562521`, success, same SHA, `dry-run=false`. + +## Public artifact proof + +- npm version: 2.7.39. +- npm dist-tags: `latest=2.7.39`, `preview=2.7.39-preview.20260724`. +- npm tarball: `https://registry.npmjs.org/@bitkyc08/opencodex/-/opencodex-2.7.39.tgz`. +- npm integrity: `sha512-Vy9DBmXw27x7RNKrlWhIMD0kD0qhamJ9LCBctW+lepac2+rL1gKqEXBCSIfsMCqCGUk5kFKSakEYXUyMpbYD6w==`. +- Git tag: `refs/tags/v2.7.39` -> release SHA. +- GitHub Release: `https://github.com/lidge-jun/opencodex/releases/tag/v2.7.39`, stable, non-draft, same SHA. +- Fresh CLI smoke: `opencodex 2.7.39`. + +## Independent C-phase review + +- Verdict: PASS; blockers: none. +- Independently confirmed one SHA across `origin/main`, npm `gitHead`, Git tag, GitHub Release, and all three Actions runs. +- Independently recomputed the downloaded tarball SHA-512 and matched the registry integrity value. +- Independently confirmed npm provenance identifies `lidge-jun/opencodex`, `.github/workflows/release.yml`, `refs/heads/main`, Release run `30073562521`, and the release SHA. +- Independently reproduced a fresh-cache CLI smoke: `opencodex 2.7.39`. diff --git a/devlog/_fin/260827_kiro_text_control_guard/000_plan.md b/devlog/_fin/260827_kiro_text_control_guard/000_plan.md new file mode 100644 index 0000000000..90004595bd --- /dev/null +++ b/devlog/_fin/260827_kiro_text_control_guard/000_plan.md @@ -0,0 +1,316 @@ +# 000 — Kiro Responses \`text\` capability guard is over-broad + +**Unit:** \`devlog/_plan/260827_kiro_text_control_guard/\` +**Class:** C3 (shared adapter surface, public request contract, cross-session persistence) +**Opened:** 2026-08-27 +**Status:** planning + +## Objective + +Stop \`kiro/*\` routed turns from failing with HTTP 400 +\`invalid_request_error\` when the Codex client sends a Responses \`text\` object +that is not structured output. Reject only genuine structured output +(\`text.format\` of type \`json_schema\` or \`json_object\`), which Kiro really +cannot honour. + +## Symptom (live, not reconstructed) + +Reported from Kiro-routed Codex usage; confirmed on the operator's Mac mini +(\`macmini-cf\`, opencodex \`2.33.0\`, proxy pid 56468 on 127.0.0.1:10100). + +Client-visible error: + + Error: error in request: {"error":{"message":"Kiro does not support Responses + text controls or structured output","type":"invalid_request_error", + "code":"invalid_request_error"}} + +\`~/.opencodex/usage.jsonl\` holds 10 matching rows, most recently +2026-08-27T12:11:27. Representative row (elided): + + {"requestId":"ocx-mtaxtnmo-kl","provider":"kiro","model":"claude-opus-5", + "admissionKind":"loopback","inboundProtocol":"responses", + "requestedModel":"kiro/claude-opus-5","status":400,"durationMs":7, + "attempts":[{"ordinal":1,"adapter":"kiro","status":400,"sendCount":0, + "errorCode":"invalid_request_error"}], + "closeReason":"non_stream", + "upstreamError":"Kiro does not support Responses text controls or structured output", + "routeDecision":{"routeKind":"explicit-provider","selected":{"provider":"kiro", + "model":"claude-opus-5","reason":"explicit-provider-namespace"}}} + +\`sendCount: 0\` locates the failure precisely: the request never reached Kiro. +It was refused inside our adapter while building the payload. + +**The failure is intermittent, and that is the diagnostic tell.** The same model +succeeds on the surrounding turns: + + 12:11:05.864 kiro/claude-opus-5 200 + 12:11:15.759 kiro/claude-opus-5 200 + 12:11:27.153 kiro/claude-opus-5 400 <- guard + 12:11:27.164 kiro/claude-opus-5 400 <- guard + 12:11:27.170 kiro/claude-opus-5 400 <- guard + 12:11:09.376 kiro/claude-opus-5 200 + +Authentication is healthy. Routing is healthy (\`explicit-provider-namespace\` +selects the intended candidate with no exclusions). Only certain request +*shapes* fail. + +## Cause + +\`src/adapters/kiro.ts:316-328\`, \`validateKiroCapabilities\`: + + const raw = parsed._rawBody as Record | undefined; + if (parsed._structuredOutput || raw?.text !== undefined) { + throw new Error("Kiro does not support Responses text controls or structured output"); + } + +The second disjunct tests the **presence of the \`text\` key**, not its content. +Any \`text\` member refuses the turn: \`text.verbosity\`, \`text.format.type:"text"\` +(which is plain prose, the opposite of structured output), even \`text: {}\`. + +\`buildKiroPayload\` (\`src/adapters/kiro.ts:436\`) calls the validator as its first +statement, so the throw happens before any wire serialization — matching +\`sendCount: 0\`. + +### Live reproduction against the running proxy + +Posted to \`http://127.0.0.1:10100/v1/responses\` with +\`model: "kiro/claude-opus-5"\` and an identical single-message input, varying +only \`text\`: + +| \`text\` member sent | Result | +|---|---| +| *(key absent)* | **200** — normal completion returned | +| \`{"verbosity":"medium"}\` | 400 \`invalid_request_error\` | +| \`{"format":{"type":"text"}}\` | 400 — and this is *not* structured output | +| \`{}\` | 400 | + +### Parser-level confirmation + +\`parseRequest\` already separates the two concepts cleanly +(\`src/responses/parser.ts:799-817\`, \`parseTextFormat\` at \`:829\`). Observed +directly: + +| input \`text\` | \`_structuredOutput\` | \`options.textFormat\` | \`_rawBody.text\` present | +|---|---|---|---| +| \`{verbosity:"medium"}\` | \`false\` | \`null\` | \`true\` | +| \`{format:{type:"text"}}\` | \`false\` | \`null\` | \`true\` | +| \`{}\` | \`false\` | \`null\` | \`true\` | +| \`{format:{type:"json_schema",...}}\` | \`true\` | set | \`true\` | +| \`{format:{type:"json_object"}}\` | \`true\` | set | \`true\` | + +\`_rawBody.text !== undefined\` is \`true\` in **all five** rows — it cannot +discriminate. \`_structuredOutput\` is exactly the discriminator the guard needs, +and it is already computed. \`parseTextFormat\` returns a value only for +\`json_schema\` and \`json_object\`; every other format, malformed or unknown, is +ignored rather than rejected. + +## Why the \`verbosity\` control reaches the adapter at all + +The generated catalog is already correct. On the operator's machine +\`~/.codex/opencodex-catalog.json\` carries: + + {"slug": "kiro/claude-opus-5", "support_verbosity": false, "default_verbosity": "low"} + +So the advertised capability is honest. What Kiro lacks is the **serialization-stage** +tolerance every other provider gets. \`src/adapters/openai-responses.ts:379-398\`, +\`stripDisabledVerbosity\`, drops a no-op \`verbosity\` at final serialization, and +its own comment states the reason: *"This runs at final serialization so a stale +catalog or direct caller cannot bypass the capability. Other \`text\` settings +(notably structured-output \`format\`) remain untouched."* + +That is precisely the shape of handling Kiro is missing. The catalog is a +declaration; the wire needs a filter. Kiro has neither filter nor tolerance — it +has a refusal. + +## Precedent in this repository + +\`db040e70f\` — *fix(kiro): accept permissive parallel tool hints* — removed a +sibling over-rejection from the same function five days earlier: + + - if (parsed.options.parallelToolCalls === true) { + - throw new Error("Kiro does not support parallel tool calls"); + - } + +The reasoning recorded in \`structure/04_transports-and-sidecars.md\` transfers +almost verbatim: the client field was **permissive, not a requirement**, so +refusing it "interprets permission as a requirement and blocks valid turns." +\`text.verbosity\` and \`text.format.type:"text"\` are permissive in exactly the +same way. This unit finishes the job that commit started, and reuses its shape: +narrow the guard, keep the wire unchanged, document the decision, test both +directions. + +## Scope + +**IN** + +- \`src/adapters/kiro.ts\` — narrow \`validateKiroCapabilities\`. +- \`tests/kiro-adapter.test.ts\` — regression coverage both ways. +- \`docs-site/src/content/docs/reference/adapters.md\` — user-facing contract. +- \`structure/04_transports-and-sidecars.md\` — decision log. +- This devlog unit. + +**OUT** + +- Kiro OAuth refresh repetition (54 \`OAuth refresh started provider=kiro\` lines + in \`service.log\`). Observed, unrelated, its own unit. +- Cursor model-discovery HTTP failure dropping 13 configured model ids. + Observed, unrelated, its own unit. +- The core/Lab import boundary (\`src/router.ts\`, \`src/server/lifecycle.ts\`, + \`src/server/responses/core.ts\`). Untouched. +- Credential, OAuth, workflow, and release paths — the \`AGENTS.md\` security + review surface. Untouched. +- Any change to what Kiro can actually *do*. Structured output stays rejected. + +## Work-phase map (dependency-ordered, PHASE-SPLIT-01) + +| Phase | Doc | Deliverable | Consumes | +|---|---|---|---| +| wp1 | this unit | Diff-level roadmap, live evidence, verifier baselines | — | +| wp2 | \`010\` | Guard narrowing + regression tests + docs | wp1's audited plan | +| wp3 | \`020\` | PR against \`dev\`, CI green at exact head SHA | wp2's verified tree | + +The order is structural, not schedule-driven: wp3 can only prove CI on a tree +wp2 produced, and wp2 can only be audited against a plan wp1 wrote. + +## Acceptance criteria + +| # | Criterion | How C proves it | +|---|---|---| +| A1 | \`text.verbosity\` reaches Kiro | Unit test + live replay returning 200 | +| A2 | \`text.format.type:"text"\` reaches Kiro | Unit test asserting no throw | +| A3 | \`text: {}\` reaches Kiro | Unit test asserting no throw | +| A4 | \`json_schema\` still rejected | Retained assertion, must still throw | +| A5 | \`json_object\` still rejected | New assertion, must still throw | +| A6 | No \`text\` control is forwarded onto the Kiro wire | Assert the serialized payload has no \`text\`/\`verbosity\` key | +| A7 | Repository gates green | \`bun run typecheck\` + \`bun run test\`, exit 0 | +| A8 | The narrowed branch actually fires | Activation evidence: the new tests fail against the pre-fix guard | + +A8 is C-ACTIVATION-GROUNDING-01. A guard change whose tests would pass either +way proves nothing, so the tests are run against the old guard first and must +fail there. + +## Verifiers (PLAN-VERIFIER-REAL-01 — run before being written down) + +| Command | Exit | Reads this unit's target? | +|---|---|---| +| \`bun install\` | 0 | Prerequisite. The worktree had no \`node_modules\`; \`bun test\` died with \`Cannot find module 'zod/v4' from src/config.ts\` until it ran. | +| \`bun test tests/kiro-adapter.test.ts\` | 0 (56 pass, 272 expects) | **Yes** — direct path argument; the file imports \`createKiroAdapter\` from \`../src/adapters/kiro\`. | +| \`bun x tsc --noEmit\` | to be captured in wp2 | **Partly** — \`tsconfig.json\` says \`"include": ["src"]\`, so it covers \`src/adapters/kiro.ts\` but **not** \`tests/\`. Corrected in \`002\` after audit round 1; the original claim here was wrong. | +| \`cd docs-site && bun run build\` | to be captured in wp2 | **Yes** — \`010\` edits \`docs-site/src/content/docs/reference/adapters.md\`. | +| \`bun run test\` | to be captured in wp2 | **Yes** — full \`tests/\` suite; required by \`AGENTS.md\` before a review-ready PR because this is shared adapter behavior. | +| \`gh pr checks \` | wp3 | **Yes** — reports the pushed head SHA's CI. | + +Baseline recorded 2026-08-27 at \`9b838d062\`: \`bun test tests/kiro-adapter.test.ts\` +→ \`56 pass, 0 fail\`, exit 0. Any post-change failure is attributable. + +## Bypass analysis (PLAN-BYPASS-NAMED-01) + +This unit **removes** an over-broad rejection; it adds no enforcement layer. + +- **Tier:** E1 (adapter-local input validation). +- **Executing surface:** \`validateKiroCapabilities\`, called by \`buildKiroPayload\`. +- **Known bypass path:** none for the retained structured-output refusal — every + Kiro turn is serialized through \`buildKiroPayload\`, whose first statement is + the validator (\`src/adapters/kiro.ts:436\`). A caller reaching Kiro without it + would have to construct the wire payload independently; no such path exists in + \`src/\`. Evidence: \`rg 'buildKiroPayload' src/\` returns the definition and one + call site. +- **Residual risk:** if Kiro later gains real structured-output support, the + retained refusal becomes wrong in the opposite direction. Cheap to revisit; + the discriminator is one flag. +- **Wording downgrade:** none. The claim stays "the adapter refuses structured + output," which is what the code does. + +## Field chain (PLAN-FIELD-CHAIN-01) + +No new field or enum value is introduced. The unit changes a boolean condition +over two values that already exist end to end: + +| Stage | \`_structuredOutput\` | \`_rawBody.text\` | +|---|---|---| +| Creation | \`src/responses/parser.ts:817\`, set from \`parseTextFormat\` | \`src/responses/parser.ts\`, the verbatim inbound body | +| Serialization | N/A — process-local, never sent upstream | forwarded verbatim by native passthrough only | +| Deserialization | N/A — never persisted or replayed | N/A | +| Consumers | \`src/web-search/loop.ts:745\`, \`src/adapters/kiro.ts:325\`, \`src/server/responses/core.ts:3099\` (deletes it for routed compaction) | \`src/adapters/kiro.ts:325\`, \`src/adapters/openai-responses.ts\` | + +The routed-compaction path at \`core.ts:3090-3100\` deletes \`options.textFormat\`, +\`_structuredOutput\`, **and** \`_rawBody.text\` together, with a comment naming +this very guard as the reason. After this unit, that third deletion is belt and +braces rather than load-bearing — worth noting, not worth removing. + +## Risks + +1. **Silently forwarding a control Kiro ignores.** Mitigated by A6: assert the + serialized payload carries no \`text\` key. The Kiro payload is built + field-by-field from \`parsed\`, never spread from \`_rawBody\`, so this is + structurally true; A6 pins it against regression. +2. **Weakening the structured-output refusal.** Mitigated by A4/A5 asserting + both structured shapes still throw, and by A8 proving the tests discriminate. +3. **A future \`text\` member that Kiro genuinely cannot ignore.** Accepted. The + guard is a denylist of one concept now instead of an allowlist of none; if + such a member appears, it gets its own condition. Recorded here so the next + reader knows it was a decision, not an oversight. + +## Outcome — DONE (2026-08-27) + +Shipped as [#2725](https://github.com/lidge-jun/opencodex/pull/2725), open against +`dev`, head `314d41d8a`, all 23 CI checks green at that exact SHA, `MERGEABLE`, +awaiting maintainer review. Merging is not the agent's to do. + +### What shipped + +| Commit | Change | +|---|---| +| `56be8f671` | This planning unit | +| `524a294f8` | Regression tests, red against the old guard | +| `a0d1ebbe4` | The guard narrowed to `_structuredOutput` | +| `ce52a0016` | docs-site bullet, structure Decision Log, two stale comments | +| `314d41d8a` | Made the third wire-absence assertion non-vacuous | + +### Verified + +`bun x tsc --noEmit` 0 · `bun run test` 0 (15279 pass / 0 fail) · +`bun run privacy:scan` 0 · `docs-site` build 0 (401 pages) · +`tests/kiro-adapter.test.ts` 58 pass / 306 expects. + +Activation evidence was captured *before* the fix and committed, so the defect is +reproducible from history rather than asserted. + +### What the plan got wrong + +Worth recording, because both were caught by review rather than by me: + +1. **The verifier claim was false.** `000` asserted `tsconfig.json` covered + `src/` and `tests/`. It is `"include": ["src"]`. The command had been run and its + exit code recorded — but the *reads-the-target* half was asserted from the + config's reputation instead of its contents. PLAN-VERIFIER-REAL-01 asks for the + `include` entry to be quoted; it was not, and the claim was wrong. +2. **A vacuous assertion nearly shipped.** The wire-absence check asserted + `context?.text` on a fixture that advertised no tool, so the adapter never built + `userInputMessageContext` and the assertion passed for the wrong reason. The + reviewer judged it test tightness rather than a hole; tightened anyway, and the + expect count moving 303 → 306 is the proof the assertions now run. + +### What did not improve (LOOP-PESSIMIST-01) + +- **The catalog remains unable to prevent this class of failure.** `support_verbosity: false` + is correct and was already correct while the 400s were happening. It cannot reach a + client holding a cached catalog, and it does not govern `text.format: {"type":"text"}` + at all. This unit did not fix that, and no capability flag will. +- **The translated `adapters.md` locales still lack the new bullet** — as they already + lacked the `parallel_tool_calls` one. They do not contradict the English source, so + this is drift, not a defect, and it was left alone rather than half-fixed. +- **Evidence that this direction is wrong, if it appears:** a Kiro turn that carries a + `text` member the wire genuinely cannot ignore. The guard is now a denylist of one + concept rather than an allowlist of none, so such a member would need its own + condition. Nothing in OpenAI's current `ResponseTextConfig` (`format`, `verbosity`) + is such a member. + +### Deferred, deliberately + +Both observed in the same `service.log` while diagnosing, both out of scope: + +- Kiro OAuth refresh repeating (54 `OAuth refresh started provider=kiro` entries). +- Cursor model discovery failing over to a stale catalog and dropping 13 configured + model ids. + diff --git a/devlog/_fin/260827_kiro_text_control_guard/001_text_control_taxonomy.md b/devlog/_fin/260827_kiro_text_control_guard/001_text_control_taxonomy.md new file mode 100644 index 0000000000..d86273f9f4 --- /dev/null +++ b/devlog/_fin/260827_kiro_text_control_guard/001_text_control_taxonomy.md @@ -0,0 +1,119 @@ +# 001 — How each adapter treats Responses \`text\` controls + +Research only. No diffs here (LEXICO-SPLIT-01); the implementation design is +\`010\`. + +## The two things \`text\` carries + +The Responses \`text\` object mixes two unrelated concerns, which is the root of +the confusion this unit fixes: + +- \`text.format\` — **output shape**. \`json_schema\` and \`json_object\` constrain + the model to emit parseable JSON. \`type: "text"\` is the default: ordinary + prose. A provider that cannot constrain output genuinely cannot honour the + first two, and has nothing to honour for the third. +- \`text.verbosity\` — **output length preference**. A hint. A provider that + ignores it produces a slightly longer or shorter answer, and nothing breaks. + +Conflating them means treating a length hint as an unsatisfiable contract. + +## What the parser already decides + +\`src/responses/parser.ts\`: + +- \`parseTextFormat\` (\`:829-843\`) returns a value **only** for \`json_schema\` and + \`json_object\`. Its own docstring: *"unknown or malformed formats are ignored, + never rejected, so the native passthrough keeps forwarding whatever the caller + sent via \`_rawBody\`."* +- \`options.textFormat\` is set from that result (\`:801\`). +- \`_structuredOutput: true\` is derived from the same result (\`:817\`). + +So the proxy's own parser already draws the line this unit needs. The Kiro guard +simply does not consult it, reaching past \`_structuredOutput\` to test +\`_rawBody.text\` for mere existence. + +## Per-adapter comparison + +| Adapter | \`json_schema\` / \`json_object\` | \`verbosity\` | \`format:"text"\` | +|---|---|---|---| +| \`openai-responses\` (passthrough) | forwarded verbatim from \`_rawBody\` | **stripped** when the model advertises \`supportsVerbosity: false\` (\`:379-398\`) | forwarded | +| \`openai-chat\` | re-nested as chat \`response_format\` (\`:~\`) | not applicable to the chat wire | not applicable | +| \`anthropic\` | mapped to an output schema via \`normalizeAnthropicOutputSchema\` | not applicable | not applicable | +| \`kiro\` (today) | **rejected** | **rejected** | **rejected** | +| \`kiro\` (this unit) | rejected | tolerated, not forwarded | tolerated, not forwarded | + +Kiro is the only adapter that turns a client hint into a 400. Every other +adapter either maps the control, drops it, or ignores it. + +## The precedent that matters most + +\`src/adapters/openai-responses.ts:379-398\`, \`stripDisabledVerbosity\`: + + /** + * Hide a no-op Responses verbosity control from the wire as well as the catalog. This runs at + * final serialization so a stale catalog or direct caller cannot bypass the capability. Other + * \`text\` settings (notably structured-output \`format\`) remain untouched. + */ + +Three things worth extracting: + +1. **The catalog is not the enforcement point.** The comment explicitly + anticipates "a stale catalog or direct caller". A capability flag is a + declaration; the wire needs its own handling. +2. **The disposition for an unsupported control is to drop it**, not to refuse + the turn. +3. **\`format\` is deliberately exempted** from that dropping, because output + shape is a real contract while verbosity is a preference. Exactly the + distinction this unit draws inside the Kiro guard. + +Kiro needs no equivalent stripper, for a structural reason: it does not +serialize from \`_rawBody\` at all. \`buildKiroPayload\` constructs +\`conversationState\` field by field from \`parsed\`. A \`text\` control the guard +stops rejecting is therefore simply never read — dropped by construction. The +fix is subtraction, and \`010\`'s A6 assertion pins that property. + +## Why the catalog does not already prevent this + +It does its job; the job is just narrower than it looks. On the operator's +machine \`kiro/claude-opus-5\` carries \`support_verbosity: false\`. But: + +- \`ensureStrictCatalogFields\` (\`src/codex/catalog/parsing.ts:408\`) defaults + \`support_verbosity\` to \`true\` when a source asserts nothing, so a row without + explicit provider evidence advertises the control. +- Clients cache catalogs. A Codex instance holding an older \`models_cache.json\` + keeps sending \`verbosity\` after the catalog says stop. +- \`text.format.type: "text"\` is **not governed by any capability flag at all**. + No catalog value suppresses it, because it is the default output mode. A + client sending it is behaving correctly. That case alone means catalog + correctness can never fully prevent this 400. + +## The sibling defect already fixed + +\`db040e70f\`, \`structure/04_transports-and-sidecars.md\` — *Kiro client +parallel-tool hint*: + +> That request field is permissive: it allows parallel calls but does not +> require the routed transport to expose a matching flag. + +and, from the same decision log: + +> Rejection interprets permission as a requirement and blocks valid turns. + +Substitute "verbosity preference" for "parallel calls" and the paragraph needs +no other edit. The same function contained both defects; one was removed on +2026-08-21, the other was not noticed because \`_structuredOutput\` was in the +condition and looked like it was doing the discriminating. + +## Open questions (settled) + +- *Should Kiro map \`verbosity\` onto its emulated thinking budget?* No. Out of + scope, and it would invent a behavior the upstream never promised. Tolerate + and ignore. +- *Should the guard reject unknown \`text\` members defensively?* No. That is the + current defect restated. The parser's stance — ignore what you do not + understand — is the house convention. +- *Does the routed-compaction path still need to delete \`_rawBody.text\`?* + (\`src/server/responses/core.ts:3099\`) Not for correctness after this change, + but its comment names two consumers and the key-mode \`openai-responses\` + adapter is the other. Leave it. + diff --git a/devlog/_fin/260827_kiro_text_control_guard/002_audit_round1.md b/devlog/_fin/260827_kiro_text_control_guard/002_audit_round1.md new file mode 100644 index 0000000000..7d6db00b12 --- /dev/null +++ b/devlog/_fin/260827_kiro_text_control_guard/002_audit_round1.md @@ -0,0 +1,135 @@ +# 002 — Audit round 1: findings and plan amendments + +Reviewer: independent \`explorer\` subagent on \`xai/grok-4.6\` (decorrelated from +the planning model, REVIEW-DECORRELATE-01). Read-only. 2026-08-27. + +**VERDICT: GO-WITH-FIXES (blockers=3)** — all three Medium, all three folded. +Every finding was re-verified by the main agent before acceptance; none was +taken on the reviewer's word. + +## Blocker 1 — stale comments will misdescribe the guard (folded) + +\`src/server/responses/core.ts:3094-3096\`: + + // the raw \`text\` controls go too: Kiro's capability guard reads both and would reject + // the turn outright, and the key-mode openai-responses adapter builds from _rawBody. + +And \`tests/server-kiro-completion-e2e.test.ts:237-238\`: + + // Routed compaction must strip the structured-output request; before the strip, + // Kiro's capability guard rejected the whole turn as unsupported text controls. + +Both re-read and confirmed by the main agent. After the narrowing, "Kiro's +capability guard reads both" is false: the guard reads \`_structuredOutput\` +only. A comment that names a behavior the code no longer has is worse than no +comment — the next reader trusts it. + +**Amendment.** \`010\` gains a comment-only edit to both files. The three +deletions at \`core.ts:3098-3102\` **stay**: the reviewer confirmed, and the main +agent verified via \`4d1e9fcb2\` (*fix(responses): strip all structured-output +traces from routed compaction*), that \`_rawBody.text\` deletion remains +load-bearing for the key-mode \`openai-responses\` adapter, pinned by +\`tests/responses-compaction-routing.test.ts:607\`. Only the justification +changes, not the behavior. + +This narrows the \`000\` scope boundary, which listed \`core.ts\` as OUT. The +boundary was written to protect the Lab-import invariant \`AGENTS.md\` guards; +a comment edit touches no import and no code path, so the invariant is intact. +Recorded explicitly rather than silently widened. + +## Blocker 2 — the wire-absence assertion was sloppy (folded) + +\`010\` proposed \`expect(serialized).not.toContain("verbosity")\` — a substring +scan of the whole JSON body. Two failure modes, both real: + +- **False fail:** any later fixture whose prose happens to contain the word. +- **False confidence:** a control forwarded under a different key would pass, + since only top-level \`payload.text\` was otherwise checked. + +The sibling test added by \`db040e70f\` already shows the right shape +(\`tests/kiro-adapter.test.ts:936-941\`): assert the specific key absent at +\`payload\`, \`conversationState\`, and \`userInputMessageContext\`. + +**Amendment.** \`010\`'s test drops the substring scan and mirrors that pattern. +Better in kind, not merely stricter: it names where the control could appear +instead of hoping a word does not. + +## Blocker 3 — the verifier claim was wrong (folded) + +\`000\` claimed \`tsconfig.json\` \`include\` covers \`src/\` and \`tests/\`. It does not: + + "include": ["src"] + +\`bun x tsc --showConfig\` confirms, and \`noUnusedLocals\` is unset — TypeScript's +\`strict\` does not imply it. Two consequences the plan got wrong: + +1. \`tsc\` does **not** typecheck the new tests. Only \`bun test\` proves them. +2. Leaving the dead \`raw\` local would **not** fail typecheck. It is still + deleted — dead code — but not for the stated reason. + +This is exactly the failure PLAN-VERIFIER-REAL-01 exists to catch: the command +was run and its exit code recorded, but the *reads-the-target* claim was +asserted from the config's reputation rather than its contents. The rule asks +for the \`include\` entry to be quoted. It was not, and the claim was false. + +**Amendment.** \`000\`'s verifier table is corrected below and \`010\`'s rationale +for deleting \`raw\` is restated as dead-code hygiene. \`docs-site\` gets its own +build check since \`010\` edits it. + +### Corrected verifier table + +| Command | Exit | Reads this unit's target? | +|---|---|---| +| \`bun install\` | 0 | Prerequisite; without it \`bun test\` dies with \`Cannot find module 'zod/v4'\`. | +| \`bun test tests/kiro-adapter.test.ts\` | 0 (56 pass) | **Yes** — direct path argument; imports \`../src/adapters/kiro\`. | +| \`bun x tsc --noEmit\` | to capture | **Partly** — \`tsconfig.json\` \`"include": ["src"]\` covers \`src/adapters/kiro.ts\` but **not** \`tests/\`. | +| \`bun run test\` | to capture | **Yes** — full \`tests/\` suite; required by \`AGENTS.md\` for shared adapter surfaces. | +| \`cd docs-site && bun run build\` | to capture | **Yes** — \`010\` edits \`docs-site/src/content/docs/reference/adapters.md\`. | +| \`gh pr checks \` | wp3 | **Yes** — CI at the pushed head SHA. | + +## Reviewer findings accepted as confirmation, not amendment + +Independently re-verified by the main agent: + +- The throw string appears in exactly two places — \`src/adapters/kiro.ts:326\` + and \`tests/kiro-adapter.test.ts:895\`. \`010\` already updates both. No miss. +- \`_structuredOutput\` has exactly one production writer + (\`src/responses/parser.ts:817\`) and one clearer + (\`src/server/responses/core.ts:3099\`). Confirmed by \`rg\`. +- \`buildKiroPayload\` never spreads \`_rawBody\`; \`rg _rawBody src/adapters/kiro.ts\` + returns only the guard local. The no-stripping-needed claim holds. +- \`src/web-search/loop.ts:745\` only picks JSON vs markdown for sidecar results. + Unaffected. +- \`parseRequest\` is imported at \`tests/kiro-adapter.test.ts:15\`; + \`kiro/claude-haiku-4.5\` is already exercised at \`:904\`. +- All three shapes throw against the current guard — the activation evidence + will be real. +- \`dev\` is the correct base; the gui-screenshot rule does not apply. + +## Why the presence check survived so long + +The reviewer traced it, and the main agent confirmed the chain: + +- \`df31ca4aba\` (2026-07-22) introduced it alongside the parallel-tool refusal, + with no rationale. +- \`ea6ff8fe62\` added the capability test — but pinned only + \`_structuredOutput: true\`, never the raw-\`text\` disjunct. The over-broad half + was **never covered by a test**, which is why it survived \`db040e70f\`'s + cleanup of its neighbour. +- \`4d1e9fcb2\` then wrote code *around* it: routed compaction deletes + \`_rawBody.text\` partly to satisfy this guard, which made the disjunct look + load-bearing. + +An untested branch that other code defends against reads as intentional. It was +not; it was a leftover from before \`parseTextFormat\` existed to tell the two +concepts apart. + +## External contract check + +The reviewer verified against upstream rather than from memory: OpenAI's +\`ResponseTextConfig\` carries exactly \`format\` and \`verbosity\`, with \`format\` one +of \`text\` | \`json_schema\` | \`json_object\` (openai-python +\`response_text_config_param.py\`, fetched 2026-08-27). So the tolerated set is +closed at today's contract, and refusing structured output — rather than +silently degrading to prose — is the honest disposition. + diff --git a/devlog/_fin/260827_kiro_text_control_guard/010_phase1_guard_narrowing.md b/devlog/_fin/260827_kiro_text_control_guard/010_phase1_guard_narrowing.md new file mode 100644 index 0000000000..57f9c70cc7 --- /dev/null +++ b/devlog/_fin/260827_kiro_text_control_guard/010_phase1_guard_narrowing.md @@ -0,0 +1,283 @@ +# 010 — Phase 1 (wp2): narrow the guard, prove both directions + +Diff-level. Copy-paste executable. Re-verify against the tree before building +(the P of wp2 does the stale check). + +## Change map + +| Path | Action | Why | +|---|---|---| +| \`src/adapters/kiro.ts\` | MODIFY | Narrow \`validateKiroCapabilities\` to structured output only | +| \`tests/kiro-adapter.test.ts\` | MODIFY | Regression both directions + wire-absence assertion | +| \`docs-site/src/content/docs/reference/adapters.md\` | MODIFY | User-facing contract | +| \`structure/04_transports-and-sidecars.md\` | MODIFY | Decision log next to the sibling entry | +| \`src/server/responses/core.ts\` | MODIFY (comment only) | Its comment names the guard's old behavior | +| \`tests/server-kiro-completion-e2e.test.ts\` | MODIFY (comment only) | Same stale claim | + +The two comment-only edits were added by audit round 1 (\`002\`). They change no +code path and no import, so the Lab-boundary invariant \`AGENTS.md\` protects in +\`core.ts\` is untouched — but \`000\` listed that file as OUT, so the narrowing of +the scope boundary is recorded rather than assumed. + +## 1. \`src/adapters/kiro.ts\` + +Current, at \`:316-328\`: + + function validateKiroCapabilities(parsed: OcxParsedRequest): void { + const choice = parsed.options.toolChoice; + if (choice !== undefined && choice !== "auto" && choice !== "none") { + throw new Error("Kiro supports only automatic tool choice or tool_choice:none"); + } + if (parsed.options.serviceTier !== undefined) { + throw new Error("Kiro does not support service tiers"); + } + const raw = parsed._rawBody as Record | undefined; + if (parsed._structuredOutput || raw?.text !== undefined) { + throw new Error("Kiro does not support Responses text controls or structured output"); + } + } + +After: + + function validateKiroCapabilities(parsed: OcxParsedRequest): void { + const choice = parsed.options.toolChoice; + if (choice !== undefined && choice !== "auto" && choice !== "none") { + throw new Error("Kiro supports only automatic tool choice or tool_choice:none"); + } + if (parsed.options.serviceTier !== undefined) { + throw new Error("Kiro does not support service tiers"); + } + // Structured output is a real contract Kiro cannot honour: the wire has no + // schema-constrained response mode, so a caller expecting parseable JSON would + // get prose and fail downstream. Refuse it. + // + // The rest of the Responses \`text\` object is NOT that. \`text.verbosity\` is a + // length preference and \`text.format: {type:"text"}\` is ordinary prose — the + // default output mode, which no capability flag governs and every correct client + // may send. Refusing the mere PRESENCE of \`text\` turned both into 400s + // (usage.jsonl, 2026-08-27: kiro/claude-opus-5 turns rejected with sendCount 0 + // between successful ones). That is the same mistake db040e70f removed one + // condition earlier, where a permissive \`parallel_tool_calls\` hint was read as a + // requirement. + // + // Nothing needs stripping the way openai-responses strips a no-op verbosity: + // buildKiroPayload composes conversationState field by field from \`parsed\` and + // never spreads \`_rawBody\`, so a tolerated control is dropped by construction. + // The test asserts that absence so it stays true. + if (parsed._structuredOutput) { + throw new Error("Kiro does not support Responses structured output"); + } + } + +Notes: + +- \`_structuredOutput\` is set by \`parseTextFormat\` for \`json_schema\` and + \`json_object\` only (\`src/responses/parser.ts:817\`, verified in \`000\`). +- The \`raw\` local becomes unused and is deleted as dead code. Note (audit round 1, + \`002\`): this is hygiene, **not** a typecheck requirement — \`tsconfig.json\` sets + \`"include": ["src"]\` with \`noUnusedLocals\` unset, and \`strict\` does not imply it, + so leaving \`raw\` would compile. Delete it anyway; a local read by nothing is a + false clue for the next reader. +- **The message changes**, dropping "text controls or". It is now accurate: only + structured output is refused. \`tests/kiro-adapter.test.ts:895\` asserts the old + string and must be updated in the same commit — do not leave the stale + substring to keep a test green. + +## 2. \`tests/kiro-adapter.test.ts\` + +### 2a. Update the existing assertion (~\`:893-896\`) + + await expect(createKiroAdapter(provider).buildRequest({ + ...parsedWith([{ role: "user", content: "hi" }], [bashTool]), + _structuredOutput: true, + } as OcxParsedRequest)).rejects.toThrow("Kiro does not support Responses structured output"); + +Only the expected message changes; the case still belongs. + +### 2b. New test — the regression + +Insert after the \`validates Kiro request capabilities explicitly\` test, beside +the parallel-tool-hint test that shares its shape. Uses \`parseRequest\` so the +real parser produces \`_structuredOutput\` and \`_rawBody\` rather than a hand-built +object — the hand-built path is what let this defect hide. + + test("tolerates non-structured Responses text controls and keeps them off the Kiro wire", async () => { + // Regression: the guard used to reject the PRESENCE of any \`text\` member, so a + // verbosity hint or a plain \`format: {type:"text"}\` produced HTTP 400 while the + // identical turn without \`text\` succeeded (usage.jsonl, 2026-08-27). + for (const text of [ + { verbosity: "medium" }, + { format: { type: "text" } }, + {}, + ]) { + const parsed = parseRequest({ + model: "kiro/claude-haiku-4.5", + input: "test", + stream: true, + text, + } as never); + expect(parsed._structuredOutput ?? false).toBe(false); + expect((parsed._rawBody as Record).text).toBeDefined(); + + const built = await createKiroAdapter(provider).buildRequest(parsed); + const payload = JSON.parse(built.body) as { + text?: unknown; + verbosity?: unknown; + conversationState?: { + text?: unknown; + verbosity?: unknown; + currentMessage: { + userInputMessage: { + userInputMessageContext?: { text?: unknown; verbosity?: unknown }; + }; + }; + }; + }; + + // Reached the wire at all — the point of the fix. + expect(payload.conversationState).toBeDefined(); + // ...but the control itself is not forwarded: Kiro has no field for it. + // Assert per key at each level the Kiro payload actually has, mirroring the + // parallel-tool test at tests/kiro-adapter.test.ts:936. A substring scan of the + // serialized body was rejected in audit round 1 (002): it false-fails on any + // fixture containing the word, and false-passes a control forwarded under + // another key. + const context = payload.conversationState?.currentMessage.userInputMessage + .userInputMessageContext; + for (const level of [payload, payload.conversationState, context]) { + expect(level?.text).toBeUndefined(); + expect(level?.verbosity).toBeUndefined(); + } + } + }); + + test("still refuses genuine structured output", async () => { + for (const text of [ + { format: { type: "json_schema", name: "r", schema: { type: "object" } } }, + { format: { type: "json_object" } }, + ]) { + const parsed = parseRequest({ + model: "kiro/claude-haiku-4.5", + input: "test", + stream: true, + text, + } as never); + expect(parsed._structuredOutput).toBe(true); + await expect(createKiroAdapter(provider).buildRequest(parsed)) + .rejects.toThrow("Kiro does not support Responses structured output"); + } + }); + +\`parseRequest\` is already imported (added by \`db040e70f\`); confirm before adding. + +### 2c. Activation evidence (A8, C-ACTIVATION-GROUNDING-01) + +Before applying the \`src\` change, run the new tests against the **current** +guard. The first must fail on all three shapes; the second must fail only on the +message text. Record both tails in the wp2 attestation. A test that passes +before and after proves nothing about the branch. + +## 3. \`docs-site/src/content/docs/reference/adapters.md\` + +In the Kiro bullet list, after the \`parallel_tool_calls\` bullet added by +\`db040e70f\`: + + - Accepts Responses \`text\` controls that are not structured output — \`text.verbosity\` + and \`text.format: {"type":"text"}\` — without forwarding them. Kiro has no wire field + for either, so they are ignored rather than rejected. Structured output + (\`text.format\` of type \`json_schema\` or \`json_object\`) is still refused, because the + Kiro wire cannot constrain the response shape and a caller expecting JSON would + receive prose. + +## 4. \`structure/04_transports-and-sidecars.md\` + +Directly after the *Kiro client parallel-tool hint* section, matching its +Decision Log format: + + ## Kiro Responses text controls + + Kiro refuses structured output and tolerates every other Responses \`text\` member. + \`text.format\` of type \`json_schema\` or \`json_object\` is a contract the CodeWhisperer + wire cannot honour, so the adapter rejects it rather than returning prose to a caller + expecting JSON. \`text.verbosity\` and \`text.format: {"type":"text"}\` are preferences, + not contracts; they are accepted and dropped, because \`buildKiroPayload\` composes + \`conversationState\` from parsed fields and never forwards the raw body. + + [Decision Log] + - 목적과 의도: Stop rejecting valid Kiro turns whose only offence is carrying a Responses text control the wire ignores. + - 기존 구현 및 제약 조건: The guard tested \`_rawBody.text !== undefined\`, so \`text.verbosity\`, \`text.format:{"type":"text"}\`, and even \`text:{}\` produced HTTP 400 with sendCount 0; \`_structuredOutput\` already distinguishes real structured output, and the catalog's \`support_verbosity: false\` cannot help a cached client or govern the default text format at all. + - 검토한 주요 대안: Keep the presence check, add an openai-responses-style stripper before serialization, or narrow the guard to \`_structuredOutput\` alone. + - 선택한 방식: Narrow the condition to \`_structuredOutput\`; no stripper is needed because the Kiro payload never spreads the raw body. + - 다른 대안 대신 이 방식을 선택한 이유: The presence check reads a preference as a requirement — the same error \`db040e70f\` removed for parallel-tool hints — and a stripper would add a serialization stage to defend a body Kiro already ignores by construction. + - 장점, 단점 및 영향: Kiro-routed Codex turns stop failing intermittently; structured output stays honestly refused; a future \`text\` member Kiro genuinely cannot ignore would need its own condition. + +## 5. Stale comments (comment-only, from audit round 1) + +Both files describe the guard's *old* behavior. Neither deletion changes; only +the justification, because after this unit "Kiro's capability guard reads both" +is false — it reads \`_structuredOutput\` alone. + +\`src/server/responses/core.ts:3094-3096\`, current: + + // would force schema-constrained JSON into the synthetic compaction item. The flag and + // the raw \`text\` controls go too: Kiro's capability guard reads both and would reject + // the turn outright, and the key-mode openai-responses adapter builds from _rawBody. + +After: + + // would force schema-constrained JSON into the synthetic compaction item. The flag goes + // too, and so does the raw \`text\` control — the key-mode openai-responses adapter + // serializes from _rawBody, so a surviving format there would reach the upstream. + // (The Kiro guard no longer reads _rawBody.text; it refuses structured output only.) + +**Keep all three deletions.** \`4d1e9fcb2\` added them and +\`tests/responses-compaction-routing.test.ts:607\` asserts \`sent.text\` is +undefined for the key-mode adapter, so \`_rawBody.text\` deletion stays +load-bearing independently of Kiro. + +\`tests/server-kiro-completion-e2e.test.ts:237-238\`, current: + + // Routed compaction must strip the structured-output request; before the strip, + // Kiro's capability guard rejected the whole turn as unsupported text controls. + +After: + + // Routed compaction must strip the structured-output request: the Kiro guard refuses + // structured output, and a surviving json_schema would constrain a prose summary. + +The test body is unchanged — it sends a real \`json_schema\`, which this unit +keeps refusing. + +## Commits (DEV-GIT-COMMIT-01) + +1. \`test(kiro): pin non-structured Responses text controls\` — tests only, + demonstrably failing. This is the activation evidence, committed before the + fix so the history shows the defect reproduced. +2. \`fix(kiro): reject only genuine structured output\` — the \`src\` change plus the + \`:895\` message update. Suite green. +3. \`docs(kiro): document Responses text control handling\` — docs-site + structure. + Includes the two comment-only corrections from §5. + +## Verification for wp2's C + + bun test tests/kiro-adapter.test.ts # expect > 56 pass, 0 fail + bun x tsc --noEmit # expect exit 0 + bun run test # full suite — AGENTS.md, shared adapter surface + cxc receipt test # receipt path for the C>D attest + +\`tsc\` covers \`src\` only (\`"include": ["src"]\`), so it proves the adapter change, +not the tests; \`bun test\` is what proves those. Add +\`cd docs-site && bun run build\` because §3 edits docs-site. + +Plus a live replay against the operator's running proxy for the three shapes +that returned 400 on 2026-08-27 — a genuine end-to-end confirmation, though the +deployed \`2.33.0\` will still carry the old guard until this ships, so the replay +is recorded as a *pre-fix baseline* there and re-run against a locally started +build. + +## Out of scope, restated + +No change to \`src/router.ts\`, \`src/server/lifecycle.ts\`, or +\`src/server/responses/core.ts\` — the three files \`AGENTS.md\` protects from Lab +imports. No credential, OAuth, workflow, or release path. No \`devlog/\` security +material. diff --git a/devlog/_fin/260827_kiro_text_control_guard/020_phase2_pull_request.md b/devlog/_fin/260827_kiro_text_control_guard/020_phase2_pull_request.md new file mode 100644 index 0000000000..7628204e8c --- /dev/null +++ b/devlog/_fin/260827_kiro_text_control_guard/020_phase2_pull_request.md @@ -0,0 +1,124 @@ +# 020 — Phase 2 (wp3): land the pull request against \`dev\` + +Consumes wp2's verified tree. Nothing here starts until wp2's C is green. + +## Branch + +The session runs in the Codex-app-managed worktree +\`/Users/jun/.codex/worktrees/121f/opencodex\`, which starts detached at +\`9b838d062\`. Adopt in place — never move or recreate the worktree +(WORKTREE-GUARD-01): + + git switch -c codex/kiro-text-control-guard + +\`codex/\` is the prefix this app requires. + +## Base and ancestry + +\`AGENTS.md\`: every pull request targets \`dev\`. \`main\` moves only by maintainer +promotion. + +The \`enforce-target\` check rejects a head whose ancestry sits on the \`main\` tip +while far behind \`dev\`. Verified 2026-08-27: \`git merge-base --is-ancestor HEAD +origin/dev\` succeeds at \`9b838d062\`, so the branch is on \`dev\`'s line, not +\`main\`'s. Re-verify after fetching, since \`dev\` moves. + + git fetch origin dev + git merge-base --is-ancestor HEAD origin/dev && echo on-dev-line + +If \`dev\` has advanced materially, rebase before opening — the contributor +readiness checklist requires the branch to be on the latest \`dev\` commit or at +most 10 behind. + +## Remote + +The worktree has \`csa906\` and \`if2007\` remotes plus \`origin\`. Confirm which one +is the upstream this PR should target before pushing: + + git remote -v + git config --get branch.dev.remote + +Push only the feature branch, never \`--force\` onto a shared ref. + +## Pull request body + +\`.github/PULL_REQUEST_TEMPLATE.md\` requires **Summary**, **Verification**, and +**Checklist**, all filled. \`enforce-target\` rejects empty, thin, or malformed +descriptions. Read the template from the tree at PR time rather than +reconstructing it here — it may have changed. + +Content to supply: + +- **Summary** — the guard rejected the presence of any Responses \`text\` member, + so \`text.verbosity\`, \`text.format: {"type":"text"}\`, and \`text: {}\` produced + HTTP 400 \`invalid_request_error\` on \`kiro/*\` turns while the identical request + without \`text\` succeeded. Narrowed to \`_structuredOutput\`, which the parser + already sets for \`json_schema\`/\`json_object\` only. Structured output stays + refused. Cite the \`sendCount: 0\` evidence and name \`db040e70f\` as the sibling + fix in the same function. +- **Verification** — pasted tails with exit codes for + \`bun test tests/kiro-adapter.test.ts\`, \`bun x tsc --noEmit\`, and + \`bun run test\`, plus the activation evidence: the new tests failing against + the pre-fix guard and passing after. +- **Checklist** — every box ticked truthfully. + +No screenshot is required: the \`gui\` screenshot rule triggers on a title or +description mentioning \`gui\`, and this change touches none. + +There is no issue to close, so no \`Closes #n\` line. If one is filed first, add +it — and remember GitHub auto-closes only on merge to \`main\`, while this targets +\`dev\`, so the issue needs a manual close. + +## Draft status + +If the pushing account lacks repository push permission, \`enforce-target\` opens +the PR as a draft and holds it there until the four-box readiness checklist is +complete: local CI green, branch on the latest \`dev\`, Codex/CodeRabbit findings +fixed, ready-for-review confirmed. The gate binds completion to the exact head +commit — a later push resets every box. So: push everything first, then tick. + +## CI proof at the exact head SHA (criterion c7) + +Not "CI passed", but "CI passed **at this SHA**": + + git rev-parse HEAD + gh pr view --json number,baseRefName,headRefOid,isDraft + gh pr checks + +\`headRefOid\` must equal local \`HEAD\`. A green run on an ancestor is not +evidence for the head. If CI is red, read the failure and return to wp2 rather +than re-running for luck. + +## Review expectations + +\`AGENTS.md\` review guidelines: English review, name file and line, concrete +failure mode. Automated reviewers (Codex, CodeRabbit) will comment; correct +findings must be fixed before ready-for-review. Likely questions worth +pre-empting in the PR body: + +- *Why not strip \`text\` before serialization like \`openai-responses\` does?* + Because \`buildKiroPayload\` never spreads \`_rawBody\`; there is nothing to + strip. The test asserts the absence. +- *Does this weaken structured-output refusal?* No — the retained test proves + both \`json_schema\` and \`json_object\` still throw. +- *Why change the error message?* It named a behavior that no longer exists. + Leaving "text controls" in the string would misdescribe the guard. + +## Close-out (D) + +- Move \`devlog/_plan/260827_kiro_text_control_guard/\` to \`devlog/_fin/\` once the + change is on \`dev\` — \`_fin\` records work already visible in public history. +- Record the terminal outcome honestly. \`DONE\` requires the PR open against + \`dev\` with CI green at the head SHA. A PR awaiting maintainer merge is still + \`DONE\` for this goal: merging is not the agent's to do. +- Note the two deferred observations for follow-up units: the repeated Kiro + OAuth refresh (54 rows) and the Cursor discovery failure dropping 13 model + ids. + +## Approval boundary + +The user authorized this PR explicitly ("pr 올려"). That authorization covers +pushing this branch and opening this pull request. It does not extend to +merging, releasing, publishing, or force-pushing any shared ref +(DEV-GIT-PUSH-01). + diff --git a/devlog/_fin/260827_ocx_restart_macos_portability/000_plan.md b/devlog/_fin/260827_ocx_restart_macos_portability/000_plan.md new file mode 100644 index 0000000000..901fe1cfca --- /dev/null +++ b/devlog/_fin/260827_ocx_restart_macos_portability/000_plan.md @@ -0,0 +1,69 @@ +# Portable detached restart on macOS + +## Loop specification + +- Archetype: repair +- Trigger: `scripts/ocx-restart.sh` stops the active proxy and cannot relaunch it when `setsid` is unavailable. +- Goal: the helper relaunches the development proxy on macOS while preserving the existing `setsid` isolation path where that command exists. +- Non-goals: change service-manager semantics, change the proxy runtime, alter provider configuration, include the unrelated `package.json` edit, or publish a package release. +- Verifier: a focused Bun test executes the real shell script with a controlled command path that deliberately has no `setsid`; `bash -n` checks shell syntax; live launchd status and `/healthz` prove operational recovery. +- Stop condition: focused/full repository gates required by `scripts/AGENTS.md` pass, the exact scoped commit reaches `origin/dev`, and the repaired service answers on port 10100. +- Memory artifact: this document plus the commit and command receipts reported at completion. +- Expected terminal outcomes: DONE when code, push, and live recovery are proven; BLOCKED if current `origin/dev` moves incompatibly or launchd repair fails after the code fix. +- Escalation condition: return to diagnosis if the no-`setsid` path still fails or if `ocx service repair` cannot produce a loaded healthy service. + +## Scope + +### In + +- `scripts/ocx-restart.sh`: select the existing `setsid` launch when available and a `nohup` fallback when it is not. +- `tests/install-scripts.test.ts`: execute the real helper in an isolated home with command shims and no `setsid`, asserting that both stop and start are reached and the helper reports healthy. +- This unit record, moved to `_fin/` after verification. + +### Out + +- `package.json` and every other pre-existing user change. +- `src/service.ts`, launchd plist generation, provider routing, and Codex configuration. +- Release, package publication, or branch cleanup. + +## Diff-level plan + +1. In `scripts/ocx-restart.sh`, replace the unconditional background launch with a capability check: + - when `command -v setsid` succeeds, retain `setsid nohup bun ... &`; + - otherwise run `nohup bun ... &` with the same stdin/stdout/stderr detachment; + - keep the existing health loop and failure output unchanged. +2. In `tests/install-scripts.test.ts`, add a non-Windows behavior test that: + - creates an isolated `HOME` and a `PATH` containing only required command shims, intentionally omitting `setsid`; + - records fake Bun stop/start invocations, creates the runtime port and PID files on start, and makes the health probe succeed; + - invokes the repository's real restart script and asserts exit 0, healthy output, and ordered stop/start calls. +3. Run the focused test, shell syntax check, typecheck, full test suite, and privacy scan. Inspect the staged diff to prove `package.json` is excluded. +4. Commit named paths only, refresh the remote lease, and push `dev` with `--force-with-lease --no-verify` as explicitly authorized. +5. Run `ocx service repair`, then verify a loaded service, a live PID/listener on 10100, HTTP 200 from `/healthz`, and healthy `ocx status`. + +## Acceptance criteria + +| Scenario | Activation | Observable proof | +| --- | --- | --- | +| `setsid` unavailable | Test `PATH` omits `setsid` while providing all other commands | Script exits 0, fake Bun log records stop then start, stdout reports healthy | +| `setsid` available | Static review preserves the existing branch and shell syntax remains valid | Diff retains `setsid nohup`; `bash -n scripts/ocx-restart.sh` exits 0 | +| Unrelated dirty work | `package.json` remains modified before and after commit | `git show --name-only` contains only the script, test, and unit record; working tree still shows `M package.json` | +| Live service recovery | Run `ocx service repair` after the pushed fix | launchd is loaded, 10100 has a listener, `/healthz` returns HTTP 200, status reports running | + +## Verifier preflight + +- `bun test tests/install-scripts.test.ts`: exit 0 with 9 passing tests before the change. It does not yet observe `scripts/ocx-restart.sh`; the planned behavior test makes it the focused verifier in B/C. +- `bash -n scripts/ocx-restart.sh`: exit 0 and directly reads the target script, proving syntax only. +- `bun run typecheck`, `bun run test`, and `bun run privacy:scan`: repository-defined gates; their post-change receipts are required by `scripts/AGENTS.md` before completion. + +## Rollback + +Revert the scoped commit and run `ocx service repair` from the restored `dev` source. The existing launchd plist and user configuration are not modified by the code patch. + +## Build evidence + +- RED: `bun test tests/install-scripts.test.ts` failed the new no-`setsid` case with exit 1 on the unconditional launch. +- GREEN: the same focused file passed 10 tests after the capability fallback; `bash -n scripts/ocx-restart.sh` and `git diff --check` also exited 0. +- Operator override: the manually started `bun run prepush` was interrupted on request. Typecheck and the GUI no-change gate completed before interruption, but the incomplete full suite is not claimed as passing. +- Delivery: direct `dev` push was rejected by the active PR-only ruleset, so PR #2735 merged the exact scoped commit with merge commit `056d2996bcc0121b54bcbc0f2abf4df25633e794`. Local `dev`, `origin/dev`, and `git ls-remote` matched that SHA afterward. +- Live recovery: `ocx service repair` exited 0; launchd reported `state = running`, PID 83423 owned `127.0.0.1:10100`, `/healthz` returned HTTP 200 with `status: ok`, and `/v1/models` returned HTTP 200. +- Terminal outcome: DONE. The unrelated `package.json` edit remains uncommitted and preserved. diff --git a/devlog/_plan/260724_gpt_live_hotfix/010_session_header_fix.md b/devlog/_plan/260724_gpt_live_hotfix/010_session_header_fix.md new file mode 100644 index 0000000000..7cbbadf3d6 --- /dev/null +++ b/devlog/_plan/260724_gpt_live_hotfix/010_session_header_fix.md @@ -0,0 +1,83 @@ +# 010 — GPT-Live 400 root cause: dropped Frameless protocol headers + +Unit: 260724_gpt_live_hotfix / WP1 +Base: `dev` @ `9f953d8e` (PR #379 merged) +Status: B implemented, verified locally (66 pass in server-live/server-auth) + +## Symptom + +GUI log `ocx-mrygl56k-6` (2026-07-24 13:47 KST): `POST /v1/live` relayed to ChatGPT +backend returns 400 `invalid_request_error` in ~743ms. Direct curl probing showed the +backend demanding `Field session must be an object` and `session.type: "quicksilver"` +when `intent=quicksilver` — i.e. it was validating our forwarded call-create as a +**v1 quicksilver** session. + +## Root cause + +The Frameless (v3 / GPT-Live) session JSON legitimately has **no `type` field** +(upstream `codex-rs/codex-api/src/endpoint/realtime_websocket/methods_frameless_bidi.rs` +`session_json`, lines 45-89: `{instructions, audio.output.voice, delegation:{type:"client"}, +model?, initial_items?}`). Protocol selection is carried by the **request header** +`openai-alpha: quicksilver=v2` (Frameless) vs `quicksilver=v1` (V1) — +`codex-rs/core/src/realtime_conversation.rs:1595-1601` `realtime_request_headers` — plus +`x-session-id`, `session-id`/`thread-id` (`codex-api/src/requests/headers.rs`), +`originator` (`login/src/auth/default_client.rs`), and `x-oai-attestation` +(`core/src/client.rs:661`). + +opencodex `resolveLiveRelay` built outbound headers **only** from provider headers + +pool auth, dropping every client protocol header on both the call-create POST and the +sideband WS upgrade. Without `openai-alpha: quicksilver=v2` the backend fell back to v1 +validation and rejected the type-less Frameless session → 400. + +Contract verified against the app-bundle runtime tag `rust-v0.146.0-alpha.3.1` +(`ff75c5b93`) and GitHub `main` (`f61b51ddd`): `realtime_call.rs`, +`methods_frameless_bidi.rs`, `realtime_conversation.rs` byte-identical to local checkout +HEAD `4462b9dee`. Sol subagent claim-ledgers (call-create headers, sideband WS contract) +confirmed: Frameless sideband sends **no** post-join `session.update`; call id parsed from +the `Location` response header; no `Sec-WebSocket-Protocol` used. + +## Fix (src/server/live.ts, src/server/auth-cors.ts) + +- `LIVE_CLIENT_PROTOCOL_HEADERS = [openai-alpha, x-session-id, session-id, thread-id, + originator, x-oai-attestation]` forwarded verbatim on call-create and sideband upgrade + (shared `resolveLiveRelay`), seeded **before** provider/auth headers so proxy-owned + `authorization`/`chatgpt-account-id` always win. +- Explicitly NOT forwarded (reviewer-audited): `x-openai-fedramp` (account-claim-derived; + contradictory in pool mode, upstream `model-provider/src/auth.rs:108`), + `x-openai-internal-codex-residency`, cookies, `host`, `origin`, `user-agent`. +- CORS `Access-Control-Allow-Headers` extended with the six headers for browser/Electron + voice preflights. + +## Audit trail + +Independent reviewer (sol) 3 rounds: R1 FAIL (missing x-oai-attestation + CORS), +R2 FAIL (fedramp must stay pool-derived), R3 PASS on final six-header scope. + +## Tests + +`tests/server-live.test.ts`: forwarded-headers assertion (present → relayed, fedramp +blocked, auth pool-owned), absent-stays-absent, sideband WS upgrade header capture, +CORS preflight allow-list. 66 pass / 0 fail with `tests/server-auth.test.ts`. + +## WP2 smoke result (2026-07-24, service pid 79964 on this tree) + +`POST http://127.0.0.1:10100/v1/live` with a real opus WebRTC offer + +Frameless session `{model:"gpt-live-1-boulder-alpha", instructions, audio.output.voice:"cove", +delegation:{type:"client"}}` + header `openai-alpha: quicksilver=v2`: + +- **201 Created** in ~0.5-0.8s, real SDP answer (ice-lite, fingerprint, ufrag), + `Location: /v1/realtime/calls/rtc_u0_E52xSxAjvyO0yAcpamyDl`. +- Control without the alpha header: **403 "Voice session access denied"** — the + forwarded header is exactly what unlocks the backend. +- `model:"gpt-realtime"` rejected with `session.model not allowed` (backend expects the + gpt-live model family); malformed-CRLF SDP gave `invalid_offer` — both errors are + post-session-validation, confirming the old session-shape 400 is gone. +- Note for GUI-log readers: the pre-fix failure `ocx-mrygl56k-6` (400, 743ms) came from + the Codex App sending its normal headers which the proxy dropped; the fix relays them. + +Gates on this tree: typecheck exit 0; full suite 4024 pass / 0 fail; privacy scan pass. + +## Remaining (WP3) + +- Push `dev`, promote `preview`/`main`, npm release (version chosen live at + release time; 2.7.36/2.7.37 burned-status re-verified then). diff --git a/devlog/_plan/260802_codex_set_prompt_composer/021_wp2_amendments.md b/devlog/_plan/260802_codex_set_prompt_composer/021_wp2_amendments.md new file mode 100644 index 0000000000..1745915523 --- /dev/null +++ b/devlog/_plan/260802_codex_set_prompt_composer/021_wp2_amendments.md @@ -0,0 +1,180 @@ +# 021 — WP2 amendments after the WP1 landing + +`020` was written against the WP1 *plan*. WP1 then landed and moved. An +independent audit against the shipped `src/codex/prompt-layers.ts` returned seven +findings; this document resolves each one and is the authority where it and +`020` disagree. + +## 1. `inventory[]` and `extensionLayersEnumerable` + +`inventory[]` is a projection of the exported `LAYER_INVENTORY` +(`prompt-layers.ts:87`), serialized field-for-field. The route defines no second +table. + +`extensionLayersEnumerable` is derivable from nothing WP1 exports, and it should +not be: it is a statement about what opencodex *can* know, not about the file. +The route emits the literal `false` with the reasoning inline. If class E ever +becomes enumerable, this constant is the one place that changes. + +The `~/.codex/...` paths in `020`'s example are illustrative only. WP1 returns +resolved absolute paths (`prompt-layers.ts:145-150`) and the route forwards them +unmodified. + +## 2. The plugins row + +`020`'s example lists `plugins` as `feature-gated` with key `features.plugins`. +The landed inventory has it as `runtime-conditional` with a null key +(`prompt-layers.ts:99`), because `core/src/mcp.rs:200` computes +`selected_plugin_available || !capability_summaries().is_empty()` — the feature +flag influences one operand but does not gate emission. + +The landed code is correct. `020`'s example is stale and is superseded here. +Because the route projects the inventory rather than restating it, this class of +drift cannot recur. + +## 3. Error translation + +Five `020` codes pass through unchanged: `unknown_layer`, `stale_revision`, +`config_unreadable`, `developer_instructions_not_owned`, `invalid_characters`. + +Two are route-derived, because WP1 cannot express them: + +| Code | Derivation | +|---|---| +| `layer_not_toggleable` | id is in `LAYER_INVENTORY` but `class !== "config-toggle"`. Checked **before** `setToggle`, which collapses every non-toggle id into `unknown_layer` (`prompt-layers.ts:748`). | +| `adopt_unsupported_form` | `previewAdopt().reason === "unsupported_form"` (`prompt-layers.ts:826`). `adoptDeveloperInstructions` collapses it to `developer_instructions_not_owned`, so the route previews first and translates. | + +Seven are pure request validation and never reach WP1: `invalid_body`, +`too_many_layers`, `invalid_layer_id`, `duplicate_layer_id`, `invalid_title`, +`body_too_large`, `composed_too_large`. WP1 validates characters and normalizes; +size and shape policy is the route's. + +Four WP1 errors `020` never mentioned need HTTP mappings, because a user can +reach every one of them: + +| WriteError | Status | Meaning to the user | +|---|---|---| +| `locked` | 409 | another writer holds the cross-process lock; retry | +| `write_superseded` | 409 | the file moved under the write; re-read and retry | +| `recovery_required` | 409 | a journal could not be replayed; terminal until repaired | +| `store_unreadable` | 409 | declared but not currently emitted; mapped so a future emission is not a 500 | + +An unmapped `WriteError` must be a typecheck failure, not a runtime surprise: +the mapping is a `Record`, so adding a variant upstream +breaks the build until it is classified. + +## 4. The path seam + +`ManagementApiDeps` gains `codexPromptPaths?: Paths`. Every WP1 entry point the +route calls already accepts `Paths` (audit §4 verified all seven signatures), so +the seam is a single optional field threaded through, with production leaving it +unset and WP1 resolving the real `CODEX_HOME`. + +This is not a convenience. A route test that could not inject paths would read +and **write** the developer's live `~/.codex/config.toml` — the same class of +incident `ManagementApiDeps.saveConfigPreservingClaudeCode` exists to prevent +(`context.ts:29-35`). + +## 5. Repair, honestly scoped + +`020` claimed four drift branches. Two are implementable from WP1 exports today +and two are not. WP2 ships what it can prove and says so: + +| drift | WP2 behavior | +|---|---| +| `projection-stale` | re-project: `writeCustomLayers(snapshot.custom, revision, paths)`. Preview via `composeProjection`. | +| `store-missing` | `previewSalvage` / `salvageProjection`. The preview returns `backupDir`, not a reserved filename — a read-only preview must not reserve one — and the response says so. | +| `journal-present` | **not repairable from this route.** Recovery lives inside `commit` (`prompt-layers.ts:627-654`) and is not exported. The route returns `409 repair_unsupported` naming the drift, and any ordinary mutation triggers recovery on its own path. | +| `owned-malformed` | `mode: "adopt"` only, through `previewAdopt`/`adoptDeveloperInstructions`. `mode: "replace"` has no WP1 export and is **not shipped**; the route refuses it with `409 repair_unsupported`. | + +Inventing a recovery-only export inside WP2 would put a second write path next +to WP1's journal — the one place in this unit where a bug destroys a user's +configuration. Two unsupported branches, named in the response, beat a +hand-rolled duplicate of the transaction. + +`020`'s sentence "the only endpoint that resolves a drift" is also wrong about +the landed code: `commit` replays a journal before performing any mutation +(`prompt-layers.ts:650-668`), so recovery is not exclusive to repair. GET +remains read-only, which is the part that mattered. + +## 6. Size caps are the route's + +The `previewAdopt` docstring mentions a "cap" step the implementation does not +perform. WP2 does not rely on it: `body > 64 KiB` and `composed > 128 KiB` are +checked in the route, before any file access, on adopt exactly as on custom +writes. + +## 7. Test consequences + +`tests/codex-prompt-route.test.ts` keeps `020`'s nineteen cases and adds four: + +20. every `WriteError` variant maps to a status — table-driven over the union +21. `repair_unsupported` for `journal-present` and for `mode: "replace"` +22. the injected `codexPromptPaths` is honored on **every** verb, proven by + asserting the fixture file changed and no other path was touched +23. `plugins` — a `runtime-conditional` row — is refused by toggle, which is + case 5's table, pinned as a named regression for finding §2 + +## 8. Round-2 corrections + +Three findings survived the first amendment. All three are real. + +### 8.1 Salvage writes its backup before the revision is checked + +`salvageProjection` creates the durable backup and only then enters `commit`, +where `stale_revision` is evaluated (`prompt-layers.ts:942-955`, `:656-664`). A +stale request therefore leaves a `.salvage-*.txt` file behind while returning +409 and changing nothing else. + +This is not a data-loss bug — the backup is additive, the config and store are +untouched, and the file is exactly the text the user already has. It is still a +side effect on a refused request, so WP2 does not pretend otherwise: + +- The route **pre-checks the revision** against the freshly read snapshot before + calling `salvageProjection`, which removes the ordinary stale-tab path. +- The race that remains (the file moves between the pre-check and WP1's own + check) can still orphan one backup. The response documents the backup + directory, and the repair preview already names it. +- WP1 is **not** modified from WP2 to close this. Reordering a write inside the + transaction is a WP1 change with its own test obligations, and a route may not + reach into it. + +### 8.2 Adopt caps are checked after a read-only preview, not before any file access + +§6 said "before any file access", which is impossible: the value being measured +lives in `config.toml`, so `previewAdopt` must read it first. The accurate rule +is **after the read-only preview and before any write**. `previewAdopt` performs +no write (`prompt-layers.ts:813`), so nothing has been mutated at the point the +cap is applied. + +Both limits are **UTF-8 byte length**, not character count. The composed cap on +adopt measures the imported body together with the already-enabled custom +layers, because that is what `composeProjection` will produce. + +### 8.3 Repair scope, stated correctly + +§5's summary line said "projection-stale + store-missing". Three branches are +supported and one is not: + +| drift | supported | +|---|---| +| `projection-stale` | yes — re-project | +| `store-missing` | yes — salvage, with 8.1's pre-check | +| `owned-malformed` | yes for `mode: "adopt"`; `mode: "replace"` refused | +| `journal-present` | no — `repair_unsupported` | + +### 8.4 Test cases 20 and 22, restated + +Case 20 cannot be "table-driven over the union": a TypeScript union does not +exist at runtime. Exhaustiveness is a **typecheck** property of +`Record`; the runtime test iterates `WRITE_ERROR_STATUS` and +asserts every entry is a valid 4xx. `store_unreadable` is declared but never +emitted by landed WP1, so no test induces it — the mapping exists so a future +emission is not a 500. + +Case 22 cannot prove "no other path was touched" from fixture mutation alone. +It uses a **second decoy directory** holding sentinel `config.toml` and +`opencodex-prompt.json` files, asserts they stay byte-identical across every +verb, and proves read-only verbs by reading fixture-specific content out of the +response rather than by observing a change. + diff --git a/devlog/_plan/260802_codex_set_prompt_composer/022_empirical_gate_findings.md b/devlog/_plan/260802_codex_set_prompt_composer/022_empirical_gate_findings.md new file mode 100644 index 0000000000..8349bd2dba --- /dev/null +++ b/devlog/_plan/260802_codex_set_prompt_composer/022_empirical_gate_findings.md @@ -0,0 +1,187 @@ +# 022 — What the layers actually do, measured + +`001` classified the layer taxonomy by reading `world_state.rs`. Two of its +conclusions are wrong, and the UI shipped them: `base-instructions` and +`agents-md` were rendered as "no off-switch anywhere in Codex" when both can be +switched off from `config.toml`. + +This document records what a live Codex actually sends, captured at the wire. + +## Method + +A minimal HTTP server on `127.0.0.1:10999` stands in for the model endpoint and +records each `/v1/responses` body. `codex exec` is pointed at it through a +throwaway provider, once per configuration, from a repository that has an +`AGENTS.md`. Measuring the request rather than `codex debug prompt-input` +matters: that command returns `prompt.input` and discards `base_instructions` +(`core/src/prompt_debug.rs:96-104`), so base changes are invisible to it. + +## Result + +| Configuration | Prompt bytes | Delta | +|---|---|---| +| default | 39,239 | — | +| `model_instructions_file` = one-line file | 21,518 | −17,721 | +| `project_doc_max_bytes = 0` | 23,939 | −15,300 | +| all switches below, together | **2,465** | **−36,774 (94%)** | + +## Corrections to `001` + +### base-instructions is replaceable, not immovable + +`config/mod.rs:3616-3624` resolves `base_instructions` from +`model_instructions_file`, and `client.rs:854` omits the developer message +entirely when the resolved text is empty. So the base prompt is not a fixed +cost: a user can replace all ~17 KB of it. + +It cannot be reduced to literally nothing. `try_read_non_empty_file` +(`config/mod.rs:4037-4066`) rejects an empty or whitespace-only file with +`InvalidData`, so the floor is one non-blank line. That is the honest answer to +"turn base off": not a switch, a **replacement**, with a minimum of one line. + +### agents-md is gated by a config key + +`agents_md.rs:89-93` returns `Ok(None)` when `project_doc_max_bytes` is 0, +before any path resolution. `001` classified this layer as +`runtime-conditional` because `world_state.rs:145` calls `add_section` +unconditionally — but the section renders whatever was loaded, and nothing is +loaded at 0. The gate is real and it is a user-settable key. + +### realtime is not conditional on being in a realtime session + +`world_state.rs:132` adds `RealtimeState` unconditionally; `realtime_active` is +an argument to it, not a guard around it. The row should not claim the layer +appears "only in a realtime session". + +## Measured per-key effect + +From a plain directory (16,077 byte baseline), each key alone: + +| Key | Delta | Sections removed | +|---|---|---| +| `skills.include_instructions = false` | −10,393 | `skills_instructions` | +| `include_apps_instructions = false` | −646 | `apps_instructions` | +| `include_environment_context = false` | −347 | `environment_context` and its children | +| `include_permissions_instructions = false` | −362 | none — swapped for `CompactPermissionsState` | +| `include_collaboration_mode_instructions = false` | 0 | none in this context | + +Two of those deserve care in the UI. + +`include_permissions_instructions = false` does not remove the permissions +layer. `world_state.rs:160-180` swaps the full section for +`CompactPermissionsState` — the model still learns the sandbox rules, in fewer +words. Calling that switch "off" overstates it. + +`include_collaboration_mode_instructions` measured zero here because the +default collaboration mode contributes nothing in this context. The key is +real (`world_state.rs:183`); its effect is context-dependent. + +## What this means for the panel + +The five-switch UI understates what a user can control. The corrected picture: + +| Layer | Control | Kind | +|---|---|---| +| base-instructions | `model_instructions_file` | replace, one-line floor | +| agents-md | `project_doc_max_bytes = 0` | real off switch | +| permissions | `include_permissions_instructions` | compact, not off | +| collaboration, environment, apps, skills | their `include_*` keys | real off switches | +| personality, token budget, deferred executor, deferred tools, multi-agent | `[features]` | off, elsewhere | +| model-switch, plugins | none | genuinely runtime-conditional | + +Only `model-switch` and `plugins` survive as layers with no user-reachable +control, and both are conditional on runtime facts rather than being always on. + +## Round 2: three of my own corrections were also wrong + +An audit against `origin/main` `f5420174d` rejected half of the above. The +common mistake, in `001` and in my correction alike: **`add_section` registers +state for diffing, it does not emit text.** A section can be registered every +turn and render nothing. + +### agents-md — overstated + +`project_doc_max_bytes = 0` stops the filesystem walk, but +`load_project_instructions` seeds `LoadedAgentsMd::from_user_instructions` +BEFORE the byte budget is consulted (`agents_md.rs:53-68`). Host-provided user +instructions therefore survive at zero. It is a **project-document** gate, not +a whole-layer off switch. + +The −15,300 measurement stands for what it measured: a repository whose +AGENTS.md is the whole of that layer. It is not a general claim. + +### permissions — my correction was worse than the original + +I wrote that `false` swaps in a compact restatement. It does not. +`CompactPermissionsState::ID` is `approved_command_prefixes`, and its +`render_diff` returns `None` outright when the previous state is `Absent` or +`Unknown` (`compact_permissions.rs:24-53`). It emits only newly approved command +prefixes on a later diff. + +So `false` DOES remove the permissions guidance. "Full versus compact" is +wrong; the honest label is that detailed permissions guidance is omitted, while +later approval updates can still appear. + +### realtime — also wrong, in the other direction + +Registration is unconditional but rendering is not: inactive or unknown state +renders nothing, start text appears when active, end text on an +active-to-inactive transition (`realtime.rs:43-53`, `:77-90`). Dropping the +condition text would imply the layer is always present, which is false. The +accurate wording is that it is emitted when realtime starts or ends. + +### base — a precedence caveat + +`model_instructions_file` outranks configured `instructions`, but an explicit +runtime `base_instructions` argument outranks the file +(`config/mod.rs:3842-3857`). The UI must not promise the file is the final +authority. + +## What the measurement can and cannot prove + +Under Responses Lite the top-level `instructions` field is ALWAYS empty and +base instructions are prepended into `input` as a developer fragment +(`client.rs:890-915`). My capture recorded `instructionsChars: 0` for every +run — that is the transport, not evidence of an absent base. The byte deltas +are sound because they measure the whole body; the empty-`instructions` +observation is an artifact and is withdrawn. + +Two more limits worth stating rather than hiding: + +- The five `include_*` deltas sum to roughly 11.7 KB. The headline −94% figure + combines those with base replacement and project-doc suppression, so calling + it "all switches" was wrong. It is one measured configuration, listed in full. +- Every number is a FIRST-request measurement from a fresh run, against one + model, catalog, repository, and transport. World-state sections are + diff-rendered, so these are not per-turn recurring costs. + +## Corrected control map + +| Layer | Real control | Honest label | +|---|---|---| +| base-instructions | `model_instructions_file` | Replace base instructions — advanced, destructive, one-line floor, runtime override still wins | +| agents-md | `project_doc_max_bytes` | Load project instructions — a byte budget, and host instructions survive at 0 | +| permissions | `include_permissions_instructions` | Detailed permissions guidance — off omits the block; approval updates may still appear | +| realtime | none | Emitted when realtime starts or ends | +| collaboration, environment, apps, skills | their `include_*` keys | real switches, though emission also depends on availability | +| environments-instructions | `include_environment_context` AND `features.deferred_executor` | jointly gated, not the feature alone | +| apps | include flag AND apps enabled AND an accessible connector AND model support | on does not guarantee emission | +| plugins | runtime availability AND model support | genuinely conditional | + +## Safety requirements for base replacement + +`model_instructions_file` replaces the prompt the model was tuned against, and +upstream itself discourages the field (`config_toml.rs:242-246`). The flow must: + +- live behind an advanced, explicitly destructive confirmation naming what can + degrade — behavior, tool use, safety posture, performance; +- write only an opencodex-owned marked file, never an arbitrary path the user + already owns; +- update the file and `config.toml` under the existing revision/journal/backup + machinery, with rollback; +- refuse empty or whitespace-only content before touching disk; +- offer Restore Codex default, which REMOVES the key rather than emptying the + file; +- stay entirely separate from `developer_instructions`, which is how custom + layers append without touching base. + diff --git a/devlog/_plan/260802_codex_set_prompt_composer/023_stack_scope_presets.md b/devlog/_plan/260802_codex_set_prompt_composer/023_stack_scope_presets.md new file mode 100644 index 0000000000..d259fa21b0 --- /dev/null +++ b/devlog/_plan/260802_codex_set_prompt_composer/023_stack_scope_presets.md @@ -0,0 +1,161 @@ +# 023 — WP6c: the stack, the scope split, and switching presets in place + +Three asks, one panel: + +1. "각 위치를 순서대로 쌓는거" — assembly order should be VISIBLE, not inferred + from row order. +2. "live 같은 live에만 주입되는건 별도로" — layers that only appear in a live + turn should not sit in the same list as layers that ship every turn. +3. "팝업에서 추가해서 바로바로 ... 개인 프리셋 갈아끼우면서" — move between + saved presets inside the dialog, editing in place. + +## What "live only" actually means + +Measured, not assumed. Three layers render on a TRANSITION rather than on every +turn, and they are not the same kind of conditional: + +| Layer | When it renders | Source | +|---|---|---| +| realtime | only as a session enters or leaves realtime | `realtime.rs:43-53`, `:77-90` | +| model-switch | only after the model changed mid-conversation | `model_switch_instructions.rs:40` | +| agents-md | when the project doc CHANGES, not on every turn | `agents_md.rs:52-64` | + +The first two are genuinely live-scoped: a user reading a steady-state prompt +will never see them. `agents-md` is different — it is diff-rendered like every +other section, so grouping it with realtime would be wrong. + +So the split is **two groups, not a filter**: layers that ship in a steady-state +turn, and layers that only appear on a transition. Both stay visible; the second +group carries the condition that produces it. + +## Ordering + +`LAYER_INVENTORY` already carries `order`, and the panel already sorts by it. What +it does not do is SHOW it, so "this is a stack" is something the reader has to +infer from vertical position alone. + +The research lane (Figma, Photoshop, Zapier, Atlassian, W3C APG) converges on the +same three things for an ordered list: a visible position, a persistent affordance +when the order is editable, and a keyboard path that is not the drag handle. + +Two important limits here: + +- **Built-in order is not editable.** It comes from `world_state.rs`, so a drag + handle on those rows would promise something the runtime does not honor. They + get a position number and nothing else. +- **Custom layers ARE ordered by the user** and already have up/down buttons from + WP5. They gain the same position number so both halves read as one stack. + +## Preset switching inside the dialog + +The ask names swipe or `<-->`. The search lane found no shipped product doing +arrow-cycling for presets: VS Code Profiles, Arc Boosts, and Cursor Rules all use +an explicit current-item label plus a list, and edit in place. The one consistent +warning is that arrows WITHOUT a position indicator lose the user. + +So: prev/next controls as asked, plus `n / total` so the position is never +ambiguous, plus the name of the current preset. Editing stays in the same dialog. + +This is the WP5 custom-layer editor gaining navigation, not a new surface. A +preset opened this way is still an ordinary custom layer — same endpoint, same +validation, same revision handling. + +## Files + +``` +gui/src/pages/codex-set-prompt.tsx (group the stack, pass positions) +gui/src/components/codex-set/PromptLayerRow.tsx (position number) +gui/src/components/codex-set/CustomLayerRow.tsx (position number) +gui/src/components/codex-set/CustomLayerDialog.tsx (prev/next + position) +gui/src/styles-codex-set.css (stack rail, group heading) +``` + +## Tests + +1. every row shows its position, and the numbers ascend without gaps +2. the live-only group contains exactly realtime and model-switch +3. agents-md stays in the steady-state group — it is diff-rendered, not live-only +4. next/prev move between custom layers and the position indicator follows +5. the indicator is present whenever the controls are (the lost-user guard) +6. editing a layer reached by navigation writes through the ordinary custom path +7. navigation is disabled at the ends rather than wrapping silently +8. a dirty editor does not lose its text when the user navigates away + +Case 8 is the one that matters: navigation inside an editor is a new way to +discard someone's typing, and WP5 already had to fix that once for Cancel. + +## Round 2 corrections + +An audit rejected three things above. All three are retractions, not defences. + +### "Steady-state" and "every turn" are both false + +I wrote that one group "ships every turn". Almost nothing does: world-state +sections are diff-rendered, so an unchanged section sends nothing on an ordinary +turn. Naming the groups that way would have taught the user the same wrong model +that produced the last two rounds of corrections. + +The axis is not steady-state versus live. It is what the layer IS: + +- **State layers** — describe configuration or context, and render when their + snapshot first appears or changes. +- **Transition notices** — exist only to announce a change. `realtime` + (`realtime.rs:43-53`) and `model-switch` (`model.rs:44-60`) have no steady + state to describe; a notice about a change that did not happen is nothing. + +That is the honest version of "live에만 주입되는 것": exactly two layers, and +the reason is their kind rather than a scope flag. + +### One position sequence would misrepresent the runtime + +Two separate errors in the original numbering plan. + +First, renumbering per visual group. If the transition notices are lifted out, +the remaining rows must keep their ORIGINAL assembly indices, gaps included. +Renumbering 1..n inside each group would invent an order the runtime does not +have. "Numbers ascend without gaps" is withdrawn as a test. + +Second, and worse: custom layers do not interleave with built-ins at all. They +concatenate into ONE `developer_instructions` projection +(`prompt-layers.ts:384-386`), which occupies a single slot. Numbering built-ins +and custom layers in one sequence would draw a fifteen-plus-n stack that does +not exist. + +So: built-ins carry their assembly index. Custom layers are an ordered sub-stack +INSIDE their one slot, numbered among themselves, and the panel says so. + +### Navigation changes the editor target and nothing else + +Presets are saved custom layers, and they are not mutually exclusive — every +enabled one composes into the projection together. Prev/next must therefore move +only which layer is being EDITED. It must not toggle enablement, and the copy +must not imply a single active preset. + +### Evidence wording + +"Measured, not assumed" was wrong for this section: the classification comes from +reading `render_diff`, not from captured output. It is source-verified, and the +two-member transition group gets a fixture-backed test before it becomes UI +taxonomy. + +## Tests, restated + +1. every built-in row shows its CANONICAL `order` from `LAYER_INVENTORY`, gaps + included after grouping — not a renumbered 1..n +2. the transition group contains exactly `realtime` and `model-switch` +3. every built-in appears exactly once across the two groups, and none is dropped +4. custom layers are numbered among THEMSELVES, and the panel states they share + one slot rather than interleaving with built-ins +5. next/prev move the editor target; title and body follow the layer, and + navigating writes nothing +6. the position indicator exists whenever the controls do +7. at the ends the controls are disabled, and clicking one leaves the editor + unchanged rather than wrapping +8. **edit A, navigate to B, come back to A: the unsaved title and body are still + there, and no PUT was issued before Save.** Blocking navigation while dirty + would pass a weaker version of this test while making the feature useless +9. saving a layer reached by navigation goes to `/api/codex-prompt/custom` with a + stable id, the siblings intact, and the current revision +10. zero and one custom layers: the controls are absent or consistently disabled +11. a refresh that deletes the layer under an open editor does not strand it + diff --git a/devlog/_plan/260802_codex_set_prompt_composer/080_wp8_stack_publication.md b/devlog/_plan/260802_codex_set_prompt_composer/080_wp8_stack_publication.md new file mode 100644 index 0000000000..8657d22951 --- /dev/null +++ b/devlog/_plan/260802_codex_set_prompt_composer/080_wp8_stack_publication.md @@ -0,0 +1,181 @@ +# 080 — WP8: publish the unit as a stacked pull-request chain + +Plan phase for the final work-phase of the unit. WP7's artifacts exist but are +uncommitted; nothing has ever been pushed. This document is the diff-level map +for turning 24 local commits into a reviewable remote stack. + +## State this plan starts from (measured, not remembered) + +``` +git rev-parse --abbrev-ref HEAD -> dev +git rev-list --count origin/dev..HEAD -> 0 (dev checkout carries no commits) +git rev-list --count HEAD..origin/dev -> 3 (local dev is behind) +codex/codex-set-prompt-composer -> 27b62a37f, 24 commits, NEVER pushed +merge-base(origin/dev, composer) -> 779b6090c (3 behind origin/dev 37e5dd5ec) +git status --short -> M docs-site/astro.config.mjs + 8 untracked guides +``` + +The consequence worth stating plainly: **there is no stack on the remote and +never was.** Earlier work-phases closed against local commits only. WP7's docs +were authored in the `dev` checkout, so they are not on the composer branch +either — `grep -c 'codexSet\.' gui/src/i18n/en.ts` returns 0 at this HEAD +because the GUI work lives on the other branch. + +## Scope boundary + +IN + +- Rebase the 24 composer commits onto `origin/dev` (37e5dd5ec). +- Commit WP7's docs (English guide + 7 locale copies + sidebar entry). +- Slice the chain into 5 dependency-ordered branches, push each, open 5 PRs. +- Add the PR screenshots the repository gate requires for GUI layers. +- Verify each layer's own tests at its own tip. + +OUT + +- Base-instruction replacement flow (deferred, flagged; needs its own unit). +- `docs-site/src/content/docs/reference/configuration/` toggle-key reference: + the five keys have no home there today (only `agents.md`, `providers.md`, + `routing.md`, `server.md` exist) and inventing one is a separate decision. +- Merging anything. Stacks land bottom-up by the maintainer. +- `git reset --hard`, force-push to `dev`, or any promotion to `main`. + +## Layer map (DEV-STACK-01: dependency order, never effort) + +Five layers, not seven. WP5/WP6 collapse into one authoring layer and +WP6b/WP6c into one real-text layer, because neither half is independently +mergeable: WP6's preset picker writes through WP5's custom-layer store, and +WP6c renumbers the stack WP6b introduced. + +| # | Branch | Commits | Thesis | GUI? | +|---|--------|---------|--------|------| +| 1 | `codex/prompt-layers-route` | 4b7bfdfce..ec1ac0a23 (3) | Serve prompt-layer state over `/api/codex-prompt` | no | +| 2 | `codex/prompt-layers-shell` | 04e3e7b15..a600254b1 (6) | Codex Auth becomes Codex Set; read the taxonomy | yes | +| 3 | `codex/prompt-layers-authoring` | 9c4eebb51..38e629401 (8) | Custom layers, drift repair, presets | yes | +| 4 | `codex/prompt-layers-realtext` | 4c3831d6e..27b62a37f (7) | Show the text Codex really sends; ordered stack | yes | +| 5 | `codex/prompt-layers-docs` | 1 new | Guide + 7 locales + sidebar | no | + +Base refs: L1 -> `dev`; L2 -> L1; L3 -> L2; L4 -> L3; L5 -> L4. + +Each layer's independent thesis, stated so a reviewer can check it: + +1. **L1** adds one route module and its tests. The GUI never calls it yet, so + merging L1 alone changes no user-visible behavior — it is a served endpoint + with route tests. Verifiable alone. +2. **L2** renames the page and renders all fifteen layers read-only. Depends on + L1 for its data; nothing above it is needed for it to be correct. +3. **L3** adds writes: custom layers, the linter, drift repair, presets. +4. **L4** replaces "Codex does not expose this" with the real probed text and + turns the flat list into a numbered assembly stack. +5. **L5** is documentation only, and sits on top because it documents L4's final + vocabulary (the numbered stack, the four absent-text reasons). + +## Execution steps + +### Step 1 — rebase onto current dev + +``` +git rebase --update-refs origin/dev codex/codex-set-prompt-composer +``` + +The three incoming commits (`c0533ce31`, `6bce1d1c8`, `37e5dd5ec`) touch the +web-search body-bounding path and cannot conflict with this unit's files. If a +conflict appears anyway, stop and report rather than resolving blind. + +### Step 2 — cut the layer branches + +``` +git branch codex/prompt-layers-route +git branch codex/prompt-layers-shell +git branch codex/prompt-layers-authoring +git branch codex/prompt-layers-realtext +``` + +Branch creation, not reset: no commit is discarded and the composer branch stays +as the record of the original chain. + +### Step 3 — WP7's commit, on top of L4 + +Switching to `codex/prompt-layers-realtext` carries the modified +`astro.config.mjs` and the 8 untracked guides across, because no commit in the +chain touches those paths. Then commit them and cut `codex/prompt-layers-docs`. + +Note the count honestly: `070` specifies English + four locales (ja, ko, ru, +zh-cn) because that is what `astro.config.mjs` declared when it was written. +The site now carries seven (fr, tr, zh-tw added), and all seven are present, so +the delivered set is wider than the plan asked for. That is an amendment to +`070`, recorded here rather than left as a silent discrepancy. + +### Step 4 — screenshots for the GUI gate + +`.github/scripts/pr-quality.cjs` is the real gate, and reading it corrected an +error in this plan's first draft. The gate does **not** arm on a `gui` text cue +in the title or body — `hasGuiCue` exists but is not what fires the failure. +The armed condition is the changed-file list (`pr-quality.cjs:527-534`): + +``` +(guiPathsChanged(changedFilePaths) || filesTruncated) && + !hasScreenshotEvidence(body) && !hasGuiOverride(...) + -> failures.push({ code: "missing_ui_screenshot" }) +``` + +`guiPathsChanged` (`:176-180`) is true when any changed path equals `gui` or +starts with `gui/`. So the gate is decided by the diff, not by wording, and +**avoiding the word "gui" in a PR body cannot dodge it.** + +Two consequences the first draft got wrong: + +1. **The screenshot must be in each GUI PR's own body.** `hasScreenshotEvidence` + reads only `body` (`:276-284`). Committing an image file into L2 gives + L3/L4 the *file*, but their gate still inspects their own description. Each of + L2/L3/L4 needs its own rendered `![...](...)` — inheritance buys nothing here. +2. **Committed asset or upload both work**, since evidence is any inline markdown + image, a reference image with a definition, or an `` outside a fence + (`stripNonRenderedRegions`, `:243-255`, strips fences BEFORE comments). A + plain link to an image is explicitly not evidence. + +Chosen approach: commit three assets under `docs-site/public/pr-screenshots/` +in the **L2** range (repository precedent: `1073-context-window-controls.jpg`), +then reference the raw.githubusercontent URL from each of the three GUI PR +bodies. One upload path, three descriptions. Screenshots are re-taken from the +live dashboard rather than trusted from stale `/tmp` timestamps. + +L1 and L5 change no `gui/` path, so the gate never arms for them. + +### Stacked bases are supported by the gate (verified) + +`enforce-pr-target.yml:535-556` paginates open PRs and sets `stackedBase` when +this PR's base ref equals another **open** PR's head in the same repo. +`pr-quality.cjs:497` then skips `wrong_base`, and `:505-508` skips the ancestry +heuristic too. A closed or missing parent falls back to `wrong_base`, which is +the cascade risk if a lower layer merges before an upper one is retargeted. + +### Step 5 — push and open + +Push with `--no-verify` (user-approved this session), `-u` on each branch. +PRs via `gh pr create --base `, each body filling all three +template sections plus the DEV-STACK-03 stack map with "you are here". + +## Accept criteria + +| # | Criterion | Evidence | +|---|-----------|----------| +| 1 | Every layer contains only its own diff | `git log --oneline ..` shows exactly the mapped commits | +| 2 | Chain rebased onto current dev | `git merge-base --is-ancestor origin/dev codex/prompt-layers-docs` exits 0 | +| 3 | Each layer's tests pass at its own tip | narrow `bun test` on that layer's own files | +| 4 | 5 PRs open with correct base refs | `gh pr list --json baseRefName,headRefName` shows the chain | +| 5 | GUI layers pass the screenshot gate | each GUI PR body renders a markdown image outside a fence | +| 6 | No repository-wide local test run | narrow `bun test ` invocations only | + +## Loop spec + +- Archetype: spec-satisfaction repair. The verifier is `gh pr list` plus + per-layer `bun test`; done is defined by the six criteria above. +- Verifier: git ancestry checks, narrow bun test, `gh pr view` per layer. +- Stop condition: five PRs open, correct bases, per-layer tests green. +- Write scope: this repository's `origin` refs, 5 new remote branches, 5 PRs. + No writes to `dev` or `main`. +- Escalation: a rebase conflict, a rejected push, or a gate failure needing a + maintainer label stops the phase and reports rather than working around it. +- Terminal outcomes: DONE when all six criteria hold; BLOCKED if the remote + refuses a push or CI requires maintainer action. diff --git a/devlog/_plan/260802_codex_set_prompt_composer/091_stack_gate_findings.md b/devlog/_plan/260802_codex_set_prompt_composer/091_stack_gate_findings.md new file mode 100644 index 0000000000..ac95586538 --- /dev/null +++ b/devlog/_plan/260802_codex_set_prompt_composer/091_stack_gate_findings.md @@ -0,0 +1,104 @@ +# 090 — What the stack gate caught, and why local green was not enough + +Closing record for the publication phase. The five-layer stack went up green +on my machine and came back red from CI three rounds running. Every one of +those rounds found something real, and they were all the same shape. + +## The defect class: a layer that only builds on top of its successors + +`CustomLayerDialog` calls three i18n keys: + +``` +codexSet.custom.prevLayer +codexSet.custom.navPosition +codexSet.custom.nextLayer +``` + +The dialog landed in the **authoring** layer. The catalog entries landed one +layer higher, in **realtext**. Nothing locally noticed, because every local +check ran at the top of the chain where both halves exist. + +CI checks out each PR's own head. At the authoring tip `bun x tsc` failed with +three TS2345s — the keys are not in `TKey` yet — and the thirteen downstream +jobs that depend on a build all followed it down. Thirteen red checks, one +cause, and the cause was invisible from the place I was testing. + +The same shape produced three more failures: + +| Symptom | Fix landed in | Was defined in | +|---|---|---| +| TS2345 x3 on the nav keys | authoring | realtext | +| `fr`/`zh-TW` locale gate on `navPosition` | realtext | authoring (after the move) | +| `react-compiler` EffectSetState, `only-export-components` | authoring | shell (files originate there) | +| `input[role="switch"]` queries matching nothing | scattered | the layer that made it a ` diff --git a/gui/src/components/codex-account-pool-main-card.tsx b/gui/src/components/codex-account-pool-main-card.tsx index e9bd0328a1..7f53e122dc 100644 --- a/gui/src/components/codex-account-pool-main-card.tsx +++ b/gui/src/components/codex-account-pool-main-card.tsx @@ -105,7 +105,7 @@ export function CodexAccountPoolMainCard({ )} - {!main?.paused && !isMainActive && !showReauth && !inCooldown && ( + {!main?.paused && (!isMainActive || pinnedId !== "__main__") && !showReauth && !inCooldown && ( @@ -183,6 +183,9 @@ export function CodexAccountPoolPageHead({ actionFeedbackTone, onRefresh, onPauseExhausted, + sparkVisible, + sparkBusy, + onToggleSpark, }: { t: TFn; embedded: boolean; @@ -193,6 +196,10 @@ export function CodexAccountPoolPageHead({ actionFeedbackTone?: NoticeTone | null; onRefresh: () => void; onPauseExhausted: () => void; + /** undefined until the preference has loaded, so the switch never renders a guessed state. */ + sparkVisible?: boolean; + sparkBusy?: boolean; + onToggleSpark?: () => void; }) { return (
{actionFeedback ?? ""} + {sparkVisible !== undefined && onToggleSpark && ( + + {t("codexAuth.sparkQuota")} + + + )} + + {t("codexSet.base.position", { position: index + 1, total: slots.length })} + + + +
+ +

{t("codexSet.base.swipeHint")}

+ + {/* C5: Dot indicator showing ring position — a swipe affordance the + text "1 / 2" alone does not provide. */} + {slots.length > 1 && ( + + )} + + {external && ( + // Never silently retarget a key somebody else set. The panel already reports + // this state; the picker refuses to act while it holds. +
+ {t("codexSet.base.externalBlocked", { path: selection.path })} +
+ )} + + {slot.kind === "default" ? ( +
+ {t("codexSet.base.defaultTitle")} + {/* + Read-only because there is nothing stored to edit, not because a control was + disabled. That distinction is the difference between "you cannot change this" + and "this is not a thing that exists". + */} +

{t("codexSet.base.defaultBody")}

+
+ ) : ( + <> + +