Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/scripts/enforce-pr-target.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,12 @@ describe("enforce-pr-target workflow", () => {
assert.match(workflow, /stackedBase/);
assert.match(workflow, /github\.rest\.pulls\.list/);
assert.match(workflow, /treating as stacked/);
assert.match(workflow, /other\.base\?\.repo\?\.owner/);
assert.match(workflow, /other\.head\?\.repo\?\.owner/);
assert.doesNotMatch(
workflow,
/other\.head\?\.repo\?\.(?:owner\?\.login|name)\s*\?\?/,
"stacked-base detection must fail closed when an open PR head repo is unavailable",
);
const qualityCall = workflow.match(
/collectPrQualityFailures\(\{([\s\S]*?)\}\);/,
);
Expand Down
69 changes: 51 additions & 18 deletions .github/scripts/pr-hygiene.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -87,28 +87,61 @@ function isTestPath(path) {
// writing tests, weakening the gate everywhere it actually matters.
//
// Deliberately narrow: a single non-comment line anywhere in the file's patch
// makes the whole file count as behavior again. Block-comment CONTINUATION
// lines are recognized only in the common leading-asterisk form; anything more
// clever than that reads as code and keeps the requirement.
// makes the whole file count as behavior again. Block-comment state is tracked
// line by line so #private fields and *generator methods are not mistaken
// for comment text; anything ambiguous reads as code and keeps the requirement.
function isCommentOnlyChange(patch) {
if (typeof patch !== "string") return false;
const changed = patch
.split("\n")
.filter(
(line) =>
(line.startsWith("+") && !line.startsWith("+++")) ||
(line.startsWith("-") && !line.startsWith("---")),
)
.map((line) => line.slice(1).trim());
if (changed.length === 0) return false;
return changed.every(
(line) =>
let changed = 0;
let oldInBlockComment = false;
let newInBlockComment = false;

const isComment = (text, inBlockComment) => {
const line = text.trim();
const opensBlock = !inBlockComment && line.startsWith("/*");
const blockComment = inBlockComment || opensBlock;
const blockEnd = blockComment ? line.indexOf("*/", opensBlock ? 2 : 0) : -1;
// Only the conventional leading-asterisk continuation counts as comment text.
// A bare line after an opener may sit inside a string or template literal
// that merely contains "/*", so it stays behavior.
const continuation = inBlockComment && line.startsWith("*");
const comment =
line === "" ||
line.startsWith("//") ||
line.startsWith("/*") ||
line.startsWith("*") ||
line.startsWith("#"),
);
((opensBlock || continuation) && (blockEnd === -1 || line.slice(blockEnd + 2).trim() === ""));
Comment thread
lidge-jun marked this conversation as resolved.
const nextInBlockComment = blockComment && blockEnd === -1;
return { comment, nextInBlockComment };
};

for (const diffLine of patch.split("\n")) {
if (diffLine.startsWith("@@")) {
oldInBlockComment = false;
newInBlockComment = false;
continue;
}
if (diffLine.startsWith("+++") || diffLine.startsWith("---")) continue;

const marker = diffLine[0];
if (marker !== "+" && marker !== "-" && marker !== " ") continue;
const text = diffLine.slice(1);
if (marker !== "-") {
const result = isComment(text, newInBlockComment);
newInBlockComment = result.nextInBlockComment;
if (marker === "+") {
changed += 1;
if (!result.comment) return false;
}
}
if (marker !== "+") {
const result = isComment(text, oldInBlockComment);
oldInBlockComment = result.nextInBlockComment;
if (marker === "-") {
changed += 1;
if (!result.comment) return false;
}
}
}
return changed > 0;
}

function hasEmptyCatch(lines) {
Expand Down
32 changes: 32 additions & 0 deletions .github/scripts/pr-hygiene.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,38 @@ describe("assessHygiene", () => {
}
});

it("does not mistake private or generator members for comments", () => {
for (const patch of [
"@@\n+ #disableAuth() { return true; }",
"@@\n+ *[Symbol.iterator]() { yield secret; }",
]) {
const failures = assessHygiene({ files: [{ filename: "src/router.ts", patch }] });
assert.equal(failures[0].code, "missing_regression_test", patch);
}
});

it("recognizes block-comment continuations only inside a block comment", () => {
assert.deepEqual(assessHygiene({ files: [{
filename: "src/router.ts",
patch: "@@\n /**\n- * old explanation\n+ * clearer explanation\n */",
}] }), []);
assert.deepEqual(assessHygiene({ files: [{
filename: "src/router.ts",
patch: "@@\n+/* one line */\n+/*\n+ * opened here\n+ */",
}] }), []);
});

it("does not treat bare lines after an opener as comment text", () => {
for (const patch of [
// An unchanged template literal holding "/*" must not hide the added SQL.
"@@\n const query = `\n+/* note\n+DELETE FROM sessions;\n+*/\n `;",
"@@\n+/* note */ runUntrusted(payload);",
]) {
const failures = assessHygiene({ files: [{ filename: "src/router.ts", patch }] });
assert.equal(failures[0].code, "missing_regression_test", patch);
}
});

it("classifies renamed behavior files on both sides", () => {
const failures = assessHygiene({ files: [
{ filename: "docs/moved.md", previous_filename: "src/router.ts", patch: "" },
Expand Down
28 changes: 15 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,22 @@ on:
- "desktop/**"
- "gui/**"
- "assets/**"
- ".github/scripts/**"
- ".github/workflows/**"
- ".gitattributes"
- ".npmignore"
- "package.json"
- "bun.lock"
- "tsconfig.json"
- "README.md"
- "LICENSE"
- ".github/workflows/ci.yml"
- ".github/workflows/release.yml"
- ".github/workflows/enforce-pr-target.yml"
- ".github/workflows/stale-needs-info.yml"
workflow_dispatch:
inputs:
lane:
description: "all (default) or macos-control"
description: "all (default), release-gates, or macos-control"
type: choice
default: all
options: [all, macos-control]
options: [all, release-gates, macos-control]

permissions:
contents: read
Expand Down Expand Up @@ -230,17 +228,15 @@ jobs:
- 'desktop/**'
- 'gui/**'
- 'assets/**'
- '.github/scripts/**'
- '.github/workflows/**'
- '.gitattributes'
- '.npmignore'
- 'package.json'
- 'bun.lock'
- 'tsconfig.json'
- 'README.md'
- 'LICENSE'
- '.github/workflows/ci.yml'
- '.github/workflows/release.yml'
- '.github/workflows/enforce-pr-target.yml'
- '.github/workflows/stale-needs-info.yml'
# Native-gated surface: platform-macos, widget and desktop-shell
# only run when the code they build or bundle could have changed.
# The listed src/ entries are what the bundled sidecar executes,
Expand Down Expand Up @@ -686,7 +682,10 @@ jobs:
macos-control:
name: macos control
needs: changes
if: github.event_name == 'workflow_dispatch'
# Diagnostic-only lanes stay off the ordinary release gates: list every lane
# that should run this job, so a new lane defaults to skipping the diagnostic.
if: >-
github.event_name == 'workflow_dispatch' && (github.event.inputs.lane == '' || github.event.inputs.lane == 'all' || github.event.inputs.lane == 'macos-control')
runs-on: macos-latest
# Unsharded full-membership control, one worker in sequential fresh batches.
# Long-lived Bun isolate pools repeatedly wedged while synchronously reaping
Expand Down Expand Up @@ -1314,8 +1313,11 @@ jobs:
dispatch=not-requested
windows=not-requested
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
dispatch=requested
# `lane=macos-control` is the one dispatch that deliberately omits Windows.
# Mirror the diagnostic job allowlist; release-gates requests neither suite.
if [ -z "$LANE" ] || [ "$LANE" = "all" ] || [ "$LANE" = "macos-control" ]; then
dispatch=requested
fi
# Only the default/all lane requests the nine Windows suite shards.
if [ -z "$LANE" ] || [ "$LANE" = "all" ]; then
windows=requested
fi
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/enforce-pr-target.yml
Original file line number Diff line number Diff line change
Expand Up @@ -671,8 +671,8 @@ jobs:
other =>
other.number !== pull_number &&
other.head?.ref === pr.base.ref &&
(other.base?.repo?.owner?.login ?? owner) === baseOwner &&
(other.base?.repo?.name ?? repo) === baseName
other.head?.repo?.owner?.login === baseOwner &&
other.head?.repo?.name === baseName
);
if (stackedBase) {
core.info(
Expand Down
2 changes: 2 additions & 0 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,7 @@
"terminal-guard-server.test.ts": "server",
"terminal-guard.test.ts": "server",
"test-home-guard.test.ts": "ci-workflows",
"test-sandbox-cleanup.test.ts": "ci-workflows",
"test-runner.test.ts": "ci-workflows",
"thought-signature-credential-scope.test.ts": "responses",
"token-estimate.test.ts": "lib",
Expand Down Expand Up @@ -1626,6 +1627,7 @@
"combo-authoritative-reset.test.ts": "codex-integration",
"devin-stated-reset-hardening.test.ts": "providers",
"devin-stated-reset-retry.test.ts": "providers",
"ci-review-lanes.test.ts": "ci-workflows",
"ci-scope-reduction.test.ts": "ci-workflows",
"ci-structure-gate.test.ts": "ci-workflows",
"responses-code-mode-patch-compile.test.ts": "responses",
Expand Down
15 changes: 13 additions & 2 deletions structure/ops/docs-and-release.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ Those controls still have no owner, so there is no image-publish workflow or off

| Workflow | Trigger | Purpose |
| --- | --- | --- |
| `.github/workflows/ci.yml` | Any `pull_request`; runtime/package `push` to `main`/`preview`; manual dispatch | A pull request verifies Linux and TypeScript: Linux runs four suite shards plus `gates` alongside the scoped docs, structure, packaging, keyring, and npm-global jobs. The `platform-macos` macOS suite, the `widget` macOS widget + Tauri app-bundle build, and the `desktop-shell` Rust toolchain build are native-gated: they run on `main`/`preview` pushes and manual dispatch, and on a pull request only when the `changes` job's native path filter selects the change. `dev` pushes start nothing; dev integration is covered by the pull-request run, while `main` and `preview` must stay push triggers because `release.yml` requires a push-event run for the exact release SHA. Windows runs nine shards only on manual dispatch with `lane=all` (or empty), not on push events. Linux runs at-most-12-file processes with a 120-second process bound; Windows uses measured six-file/480-second processes and all-file scope so its full-suite contract is unchanged. The dedicated Windows batch step sets `OCX_TEST_NO_QUEUE=1` because its sequential processes are one logical runner; each process still creates an isolated home and arms the test guards before the lock boundary. No lane retries: a test failure, a process timeout and a Bun runtime crash each fail their job on the first occurrence. Aggregate `ci` is event-aware — it derives which jobs this event requested and requires `success` from each of them and `skipped` from the rest, and on a `lane=all` dispatch it reads the run's own job list and requires nine concrete successful `windows N/9` results. `npm-global-smoke` remains GitHub-hosted because it mutates the global package prefix. |
| `.github/workflows/ci.yml` | Any `pull_request`; runtime/package `push` to `main`/`preview`; manual dispatch | A pull request verifies Linux and TypeScript: Linux runs four suite shards plus `gates` alongside the scoped docs, structure, packaging, keyring, and npm-global jobs. The `platform-macos` macOS suite, the `widget` macOS widget + Tauri app-bundle build, and the `desktop-shell` Rust toolchain build are native-gated: they run on `main`/`preview` pushes and manual dispatch, and on a pull request only when the `changes` job's native path filter selects the change. `dev` pushes start nothing; dev integration is covered by the pull-request run, while `main` and `preview` must stay push triggers because `release.yml` requires a push-event run for the exact release SHA. Windows runs nine shards only on manual dispatch with `lane=all` (or empty), not on push events. Linux runs at-most-12-file processes with a 120-second process bound; Windows uses measured six-file/480-second processes and all-file scope so its full-suite contract is unchanged. The dedicated Windows batch step sets `OCX_TEST_NO_QUEUE=1` because its sequential processes are one logical runner; each process still creates an isolated home and arms the test guards before the lock boundary. No lane retries: a test failure, a process timeout and a Bun runtime crash each fail their job on the first occurrence. Aggregate `ci` is event-aware — it derives which jobs this event requested and requires `success` from each of them and `skipped` from the rest, and on a `lane=all` dispatch it reads the run's own job list and requires nine concrete successful `windows N/9` results. `npm-global-smoke` remains GitHub-hosted because it mutates the global package prefix. Manual `lane=release-gates` keeps the ordinary native-gated jobs and selected dynamic keyring/packaging matrix legs, but skips the Windows suite and unsharded macOS control. Default `all` (or empty) still requests both diagnostics; `macos-control` requests the control without Windows suite shards. Release eligibility requires successful push-event CI on the exact release SHA; a manual lane does not authorize publishing. |
| `.github/workflows/dev-version-bump.yml` | Manual dispatch with an intended version and `pre-move` or `repair` mode | Opens the reviewed pull request that moves `dev` past a release target. The default `pre-move` mode runs before promotion and publication; explicit `repair` mode retains the post-publish catch-up path. It is neither called by `release.yml` nor triggered by publication. |
| `.github/workflows/release.yml` | Manual dispatch only | npm publish/dry-run workflow. It requires a successful push-event Cross-platform CI run for the exact `GITHUB_SHA` (a pull-request run does not qualify), requires `dev` to outrank the target, then checks the target against the freshly fetched global tag set before publish or dry-run. |
| `.github/workflows/deploy-docs.yml` | `push` to `main` touching `docs-site/**` or the workflow, or manual dispatch | Build and publish the Astro/Starlight docs site to GitHub Pages. This is the deploy path; the pull-request build gate is the `docs-site-build` job in `ci.yml`. |
Expand Down Expand Up @@ -445,7 +445,18 @@ storage-policy and api-usage families to its dedicated jobs. The Windows step di
user-scoped test-run queue with `OCX_TEST_NO_QUEUE=1`: the batches already run sequentially in one
dedicated job, and queueing a new batch behind a surviving process from the preceding batch spends
the process timeout without executing tests. The per-process home isolation and live-home/service
manager guards remain active because the preload installs them before the lock boundary. A test
manager guards remain active because the preload installs them before the lock boundary.
`tests/preload.ts` resolves cleanup dependencies after home/lock admission and before test cases,
then teardown awaits native-main startup releases and config hardening, followed by the sandbox's
registered ACL child reaps before removing that root. Its synchronous exit fallback leaves an
undrained root for ownership-checked stale recovery instead of blocking child cleanup with removal
retries. `tests/ci-workflows/test-sandbox-cleanup.test.ts` pins that ordering with a delayed reap.
`tests/helpers/test-sandbox-cleanup.ts` also exposes case-scoped lifecycle ownership: cancellation
starts listener stops while owned asynchronous work settles, and repeated close/stop calls share
one promise. Expected teardown aborts are observed without hiding ordinary assertion failures.
Callers settle that lifecycle before draining producers/reaps and restoring or removing a home;
the helper does not replace fixture-specific cleanup or claim OS ACL coverage for synthetic tests.
A test
failure, a process timeout and a Bun runtime crash each fail their job on the first occurrence; the
batch runner still sweeps a crashed or timed-out batch one file per process, but only to attribute a
failure the shard has already taken. The aggregate `ci` gate derives, from the event and the `changes` outputs, which
Expand Down
Loading
Loading