refactor(conductor): delete the dead dev-job step coupling (epic #470 C5) - #554
Open
Weegy wants to merge 6 commits into
Open
refactor(conductor): delete the dead dev-job step coupling (epic #470 C5)#554Weegy wants to merge 6 commits into
Weegy wants to merge 6 commits into
Conversation
…C5) The Conductor's `dev.job` step was built in W3 and never wired: `conductor/index.ts` constructs `ConductorRunExecutor` without a `devJob` dep, so the dispatch branch was permanently false; the launch half had no implementation at all (`createConductorJob`, `setAwaitId`, `getAwaitId` existed only as interface members); `DevJobOutcomeEmitter.emit()` had no caller in `src/`; nothing scheduled the reconciliation sweep; and no bundled template referenced `dev.job` (which `listActions` rejects anyway). Per `specs/470-dev-platform-plugin/dormant-capabilities.md` §1 the verdict is DELETE, not genericise — nothing needs the generic version. Deleted whole: `conductor/devJobStepEffect.ts`, `devplatform/devJobConductorBridge.ts`, `test/conductorDevJobStep.test.ts`. Stripped from `runExecutor.ts`: both imports, the `devJob` field + ctor dep, `DevJobPortUnavailableError`, the dispatch branch, `resolveDevJobAwait`, `reconcileTerminalDevJobAwaits`, `openDevJobAwait`. `awaitStore.listWaiting` drops its `AND channel_type <> 'dev_job'` predicate outright rather than keeping a generic filter. `openHumanAwait` is now the single caller of `create`, so every await has a human holder by construction and the excluded set is provably empty; a filter whose complement no member can enter asserts an invariant in the wrong place, and a future non-human await kind would have to remember to add itself to survive it. New `test/conductorAwaitStore.test.ts` guards the channel-agnostic contract (mutation-checked: re-adding a channel predicate fails it). `dev_jobs.conductor_await_id` (migration 0024) is KEPT — migrations are forward-only here and a DROP is the one irreversible act. It is marked as an orphaned column so the schema is not misread as evidence of a live feature. Core-decoupling ratchet: 3303 -> 3164 (-139), no zone rose.
First change in this epic that moves the decoupling count DOWN.
3,303 → 3,167 (−136): middleware/src −96, middleware/test −43.
WHAT THIS IS NOT: the Conductor is a live feature — 31 files, ~6,200 LOC
backend, 23 UI files, 7 migrations, its own spec. Runs, steps, human
approval gates, templates, the Designer canvas are all untouched. All
204 conductor tests pass.
WHAT WAS DEAD: one step type inside it. `dev.job` was meant to let a
workflow launch a Dev Platform job, park the run, and resume on its
terminal outcome. It was built and never wired:
- conductor/index.ts constructs the executor with NO devJob dep, so
the dispatch branch was permanently false — and `git log -S` shows
it was never wired in ANY commit
- the launch half had no implementation at all: createConductorJob,
setAwaitId and getAwaitId existed only as interface members, and
dev_jobs.conductor_await_id was never selected or written
- DevJobOutcomeEmitter.emit() had no caller in src/
- nothing scheduled the reconciliation sweep
- no bundled template referenced dev.job, and listActions never
included it, so the step could not pass validation
Removed: conductor/devJobStepEffect.ts (122 LOC), devplatform/
devJobConductorBridge.ts (113), test/conductorDevJobStep.test.ts (358),
and every dev-job reference in runExecutor.ts (31 → 0), awaitStore.ts
(6 → 0) and routes.ts (1 → 0).
KEPT: migration 0024's conductor_await_id column. Migrations are
forward-only here and dropping a column is the one irreversible act in
this change; it is marked orphaned instead.
THE SUBTLE PART: awaitStore's human-inbox query excluded
`channel_type = 'dev_job'`. That predicate is now gone rather than
genericised, because after the delete `openHumanAwait` is the sole
writer of conductor_awaits — a filter whose complement no code path can
populate asserts an invariant in the wrong place, and a channel
allow/denylist would fail open for a future await kind that forgot to
register itself.
The compensating test needed a fix the review caught: its fixtures were
teams/telegram/web, so restoring `<> 'dev_job'` would have left it
green — it could not detect the exact regression it exists for. Added a
dev_job fixture row and mutation-checked it: with the predicate restored
1 fail, without it 3 pass.
Also propagates the decision into the specs, which still described the
step as a capability the extraction must carry and H2 as a registry to
build. H2 needed no mechanism after all — the coupling was dead, so
deleting it was the whole fix.
The channel-api work added 3 dev-platform references (test/packages). Fourth legitimate raise: main ADDED dev-platform code; core did not re-acquire a dependency.
…-devjob # Conflicts: # specs/470-dev-platform-plugin/decoupling-baseline.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First change in epic #470 that moves the decoupling count down: 3,303 → 3,167 (−136). Every prior change either added or held it flat.
What this is not
The Conductor is a live feature — 31 files, ~6,200 LOC backend, 23 UI files, 7 own migrations, its own spec. Workflow runs, steps, human approval gates, templates, the Designer canvas: all untouched. All 204 conductor tests pass.
What was dead
One step type inside it.
dev.jobwas meant to let a workflow launch a Dev Platform job, park the run, and resume on its terminal outcome. It was built and never wired:conductor/index.tsconstructs the executor with nodevJobdep, so the dispatch branch was permanently false — andgit log -S 'devJob'on that file returns nothing, so it was never wired in any commitcreateConductorJob,setAwaitId,getAwaitIdexisted only as interface members, anddev_jobs.conductor_await_idwas never selected or writtenDevJobOutcomeEmitter.emit()had no caller insrc/dev.job, andlistActionsnever included it — so the step could not even pass validationThat last point matters: nobody could have authored this step.
Removed
conductor/devJobStepEffect.tsdevplatform/devJobConductorBridge.tstest/conductorDevJobStep.test.tsconductor/runExecutor.tsconductor/awaitStore.tsconductor/routes.tsKept: migration
0024'sconductor_await_idcolumn. Migrations are forward-only here and dropping a column is the one irreversible act in this change — it is marked orphaned instead.The subtle part
awaitStore's human-inbox query excludedchannel_type = 'dev_job'. That predicate is now gone rather than genericised: after the delete,openHumanAwaitis the sole writer ofconductor_awaits, so a filter whose complement no code path can populate asserts an invariant in the wrong place. A channel allow/denylist would also fail open for a future await kind that forgot to register itself.On a database that somehow held such a row, it becomes visible in the operator inbox and a resolve attempt returns 403 (the holder gate compares against
dev_job:<id>, which no user id can equal). With the resolver deleted, such a run is permanently parked — the await row is the only evidence of it, so surfacing it is the better outcome than hiding it.A review catch worth naming
The compensating test added for the removed predicate could not detect that predicate being restored. Its fixtures were
teams/telegram/web, so re-addingAND channel_type <> 'dev_job'left it green — it did not cover the exact regression it existed for. Fixed with adev_jobfixture row and mutation-checked: with the predicate restored 1 fail, without it 3 pass.Also
Propagates the decision into the specs, which still described the step as a capability the extraction must carry and H2 as a registry to build. H2 needed no mechanism after all — the coupling was dead, so deleting it was the whole fix. That removes one of the three "hard couplings" from the plan.
Test plan
middleware/,web-ui/,scripts/,.github/— the exportedDevJobPortUnavailableErrorincluded — with no consumer outside the delete listNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.