Skip to content

🤖 refactor: pass the WorkspaceService archive gate to workflow admissions instead of a process-global guard - #4562

Merged
ThomasK33 merged 1 commit into
mainfrom
ws4b/workflow-archive-guard-ownership
Sep 25, 2026
Merged

ThomasK33 merged 1 commit into
mainfrom
ws4b/workflow-archive-guard-ownership

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Summary

Removes the process-wide workflow archive guard. Workflow start/resume/retry admissions now ask the owning WorkspaceService directly (getWorkflowArchiveRefusal), passed in through WorkflowServiceContext and a required WorkflowService constructor option. With nothing registered at module scope, a WorkspaceService built in one test file can no longer decide admissions in another. The three per-file guard resets, including #4530's, are deleted.

Background

Every WorkspaceService constructor called setWorkflowArchiveAdmissionGuard(...) with a closure over its own config and never cleared it. After #4475 split the WorkspaceService tests, a split file sharing a unit shard with WorkflowService.context.test.ts left a guard over a partial config double behind, and 9 context tests failed. #4530 reset the guard in that one file; evaluateScreening.test.ts and WorkflowService.test.ts carried the same reset. The leak itself stayed open.

A compare-and-clear release on dispose would not have closed it: about 170 test fixtures build a WorkspaceService without disposing it, so the last guard would still outlive its file.

Implementation

  • workflowArchiveAdmission.ts:
    • admissionGuard and setWorkflowArchiveAdmissionGuard are gone.
    • acquireWorkflowArchiveAdmission(guard, workspaceId) takes a WorkflowArchiveAdmissionGuard.
    • The in-process admission/runner counters stay process-wide: WorkflowService instances are per-request, and the archive sink must see admissions from any of them.
  • WorkspaceService: the constructor closure became the public getWorkflowArchiveRefusal(workspaceId), with the same body. The class declares implements WorkflowArchiveAdmissionGuard.
  • WorkflowService: new required archiveAdmission option, so tsgo finds every construction site. It is wired at all three production sites:
    • resolveWorkflowContext passes context.workspaceService.
    • cli/workflow.ts passes the CLI's own services.workspaceService, which is the instance whose guard was installed before.
    • Tool-started runs in TurnRequestBuilder get a new workflowArchiveAdmission binding. It is set in di/layers/core.ts next to the other WorkspaceService bindings, and the builder asserts it when taskService is bound.
  • The top-level startWorkflowRun/resumeWorkflowRun/retryWorkflowRunFromCheckpoint pass context.workspaceService.

Tests:

  • The refusing-guard test in WorkflowService.test.ts injects its gate instead of setting and resetting a global.
  • New owner test: WorkspaceService refuses archived and archiving workspaces, and admits active, unarchived and unknown ones. The logic previously had no owner-side coverage.
  • New wiring test: startWorkflowRun refuses through the context's WorkspaceService before creating a run. Replacing both the top-level and the class-level gate with an admit-all stub makes it fail.

Validation

  • The 🤖 tests: reset the process-global workflow archive guard in WorkflowService context tests #4530 reproduction order passes with the resets removed: 131 pass. The command was bun test workspaceService.archive workspaceService.bashAndFiles workflows/WorkflowService.context workflows/evaluateScreening workflows/WorkflowService.
  • bun test src/node/services/workflows: 347 pass. bun test src/node/services/{workspaceService,taskService,turnRequestBuilder}* src/node/services/serviceContainer.test.ts: 1701 pass. src/node/services/{agentSession*,tools} and src/node/orpc also pass.
  • src/cli passes after make build-main. One run under host load 66 timed out "CLI run closes the AppFiberScope first…" at 5 s; workflow.test.ts then passed 3 of 3 runs on its own.
  • make static-check passes.

Risks

Low. Admission semantics are unchanged in production, which has one WorkspaceService per process; the refusal is now looked up explicitly instead of through a module variable. A future WorkflowService construction must supply the gate, and the type system enforces that.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T15:20:24.791612Z 3867204 Manual request
🔒 Security Review ✅ Completed 2026-09-25T15:22:05.709917Z 3867204 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 3867204d13

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 3867204d13

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 25, 2026
@ThomasK33
ThomasK33 added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit 518df57 Sep 25, 2026
30 of 31 checks passed
@ThomasK33
ThomasK33 deleted the ws4b/workflow-archive-guard-ownership branch September 25, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant