Skip to content

Clarify why the ESLint factory cannot dispatch workers #67217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 12 commits into from
Oct 9, 2026
Merged

Clarify why the ESLint factory cannot dispatch workers #67217

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6f54ce5
Clarify uninitialized ESLint factory queue in activation
Copilot Oct 9, 2026
9e0ba6a
Bootstrap work queue on first producer submission
Copilot Oct 9, 2026
0700dc5
Merge remote-tracking branch 'origin/main' into copilot/investigate-e…
Copilot Oct 9, 2026
60849ef
Use configured branch in queue activation guidance
Copilot Oct 9, 2026
b58cc77
Read queue branch from original options
Copilot Oct 9, 2026
362147f
Merge remote-tracking branch 'origin/main' into copilot/investigate-e…
Copilot Oct 9, 2026
515b0ec
Merge latest main and guard queue replay
Copilot Oct 9, 2026
0023df3
Merge remote-tracking branch 'origin/main' into copilot/investigate-e…
Copilot Oct 9, 2026
a81eacc
Share maintenance test parser with integration tests
Copilot Oct 9, 2026
27163d1
Align AGY conformance tests with work-queue worker
Copilot Oct 9, 2026
708cbbf
Update dispatcher test for queue bootstrap behavior
Copilot Oct 9, 2026
fe5c200
Merge remote-tracking branch 'origin/main' into copilot/investigate-e…
Copilot Oct 9, 2026
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
2 changes: 1 addition & 1 deletion actions/setup/js/write_work_queue_snapshot.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ async function main(options = {}) {
fs.chmodSync(outputPath, 0o444);
configuration.core.setOutput?.("work_queue_origin", canonical(snapshot.origin));
configuration.core.info(
`Work queue activation: ${worker ? `${worker.claims.length} immutable Claims authenticated${staged ? " for staged preview (not bound)" : " and bound"}` : runtime.role === "observer" ? "read-only observer" : "unassigned queue-control context"}; snapshot captured`
`Work queue activation: ${worker ? `${worker.claims.length} immutable Claims authenticated${staged ? " for staged preview (not bound)" : " and bound"}` : latest.sha === null ? "queue uninitialized; an administrator must protect the work-queue branch and install Policy before workers can be dispatched" : runtime.role === "observer" ? "read-only observer" : "unassigned queue-control context"}; snapshot captured`

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The activation diagnostic now uses the configured queue branch, defaulting to WORK_QUEUE_BRANCH; the regression test covers a custom branch. Fixed in b58cc77f.

@github-actions github-actions Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/grill-with-docs] The new log text hardcodes the literal string "work-queue branch", but loadQueue (work_queue_binding.cjs:36) already forwards a configurable options.branch. If a custom branch is ever used, this message will mislead the administrator about which branch to protect.

💡 Suggested fix

Use the actual configured branch name (falling back to the default) when building the message, e.g. thread options.branch || "work-queue" through to the log string instead of the literal text. This keeps the diagnostic accurate for non-default deployments and matches the existing not-yet-addressed review comment on this same line.

@copilot please address this.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The activation diagnostic now names the configured branch (with WORK_QUEUE_BRANCH as the default). Fixed in b58cc77f.

@github-actions github-actions Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This nested ternary is hard to parse at a glance, and the resulting template-literal line is ~417 characters (vs. the repo's printWidth: 240 in .prettierrc.json). The logic itself is correct (verified: when worker is falsy and latest.sha === null, the new branch wins before the observer/default branches), but consider extracting the message selection into a small helper function for readability, e.g.:

function activationMessage({ worker, staged, latest, runtime }) {
  if (worker) return `${worker.claims.length} immutable Claims authenticated${staged ? " for staged preview (not bound)" : " and bound"}`;
  if (latest.sha === null) return "queue uninitialized; an administrator must protect the work-queue branch and install Policy before workers can be dispatched";
  if (runtime.role === "observer") return "read-only observer";
  return "unassigned queue-control context";
}

Non-blocking, but would improve maintainability for future edits to this message.

@copilot please address this.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted activation message selection into activationMessage; it now also includes the configured queue branch for an absent queue. Fixed in b58cc77f.

);
return snapshot;
}
Expand Down
16 changes: 15 additions & 1 deletion actions/setup/js/write_work_queue_snapshot.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fs from "fs";
import path from "path";
import os from "node:os";
import { main, resolveWorkerAssignment } from "./write_work_queue_snapshot.cjs";
import { loadWorkQueueSnapshot } from "./work_queue_mcp_server.cjs";
import { loadWorkQueueSnapshot, readWorkQueueState } from "./work_queue_mcp_server.cjs";
import { normalizeRuntimeMessage, readClaimScopeContext } from "./work_queue_claim_scope.cjs";
import { queueFixture, REF, REPOSITORY, WORKFLOW, DISPATCHER } from "./work_queue_lifecycle.test_helpers.cjs";
import { newRequest, newState, replayTransactions, serializeTransactionLog } from "./work_queue_replay.cjs";
Expand Down Expand Up @@ -177,6 +177,20 @@ describe("authenticated immutable activation snapshots", () => {
expect(loadWorkQueueSnapshot(options.snapshotPath).projection.policy_epoch).toBe("e1");
});

it("reports an absent queue as uninitialized without claiming worker readiness or installing Policy", async () => {
const { options } = setup({ granted: false });
const initialize = vi.fn();
const snapshot = await main({
...options,
readWorkQueueLog: async () => ({ sha: null, transactions: [], state: newState() }),
initializeWorkQueue: initialize,
});
expect(snapshot).toMatchObject({ sha: null, worker: null, role: "dispatcher" });
expect(initialize).not.toHaveBeenCalled();
expect(readWorkQueueState(loadWorkQueueSnapshot(options.snapshotPath)).queue_state).toBe("uninitialized");
expect(options.core.info).toHaveBeenCalledWith(expect.stringContaining("administrator must protect the work-queue branch and install Policy"));

@github-actions github-actions Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion hard-codes work-queue, so the regression test will still pass while the runtime tells operators to protect the wrong branch whenever a workflow uses options.branch.

💡 Assert the configured branch

loadQueue() already accepts options.branch, but this test never sets one and then locks in the literal default branch name. Pass a custom branch (for example "custom-queue") and assert that the info message includes that branch so the test actually catches the runtime bug.

Copilot AI Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The absent-queue regression test now sets branch: "custom-queue" and asserts that branch appears in the activation guidance. Fixed in b58cc77f.

});

it("rejects actual oversized encoded framing before creating a snapshot or publishing origin output", async () => {
const { fixture, options } = setup({ granted: false });
const before = fixture.transactions;
Expand Down
7 changes: 7 additions & 0 deletions docs/src/content/docs/patterns/linter-factory.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ they do not seed tasks. The separate `daily-report-dispatcher` uses the

### Provision the factory policy

The scheduled dispatcher cannot initialize its own queue: without the
`work-queue` branch and an installed Policy, activation cannot launch any
workers. A successful run that only reports `queue_state: "uninitialized"`
also does not mean the factory is operating. Check the authoritative queue
state before expecting any of the three worker workflows to run. Do not
substitute an ordinary workflow dispatch for a queue assignment.

The checked-in policy generator provides three immutable worker profiles,
singleton assignments, three native slots, and a 30-pending-task limit:

Expand Down
Loading