Skip to content

development orchestrator: optional-phase defaults silently skipped, plus 5 bookkeeping/propagation findings from a long run #15

Description

@mkaluzny

Summary

Findings from one long /maister:development run (a large frontend feature: ~14 commits, 12 task groups, 131 plan steps, full verifier suite). The workflow produced good work and the Phase 11 verifier caught two real data-loss defects that no gate, lint rule or test suite would have. These notes are about the bookkeeping and propagation layers, where several things failed silently.

Ordered by how fixable-in-the-skill they are.


1. Phase 2 optional-phase defaults are never applied, and nothing notices — skills/development

task_characteristics.ui_heavy was true. Phase 2 says:

If task_characteristics.ui_heavy: true → set options.e2e_enabled: true, options.user_docs_enabled: true

Both stayed false. Phase 12 (E2E) and Phase 13 (user docs) were therefore never run and never asked about — on a feature shipping two new full-page interactive players, which is precisely the shape E2E exists for.

The interesting part is why. Two steps earlier in the same phase, writing task_characteristics to state carries an explicit self-check:

SELF-CHECK: "Did I read the 5 task_characteristics from the gap-analyzer output and write them to state? Let me re-read orchestrator-state.yml to verify."

That one worked — ui_heavy: true is correctly in state. The very next instruction, "set optional phase defaults," is a bare imperative with no self-check and no gate. It is the only state write in Phase 2 that nothing verifies, and it is the one that silently didn't happen.

Suggested fix: give it the same self-check, and phrase it as a derivation rather than an imperative (e2e_enabled: <ui_heavy>, then re-read to confirm). Better still: these two flags gate real work, so surface them in the Phase 2 exit gate the operator already answers, instead of inferring them silently.


2. A phase can end in pending and finalization accepts it — skills/development

Phase 14 has no precondition that every phase is either completed or skipped with a recorded reason. Phase 3 got a proper skip reason because it was explicitly ruled on; 12 and 13 got nothing, because nothing ever looked at them. The run "finished" with two phases still pending.

Suggested fix: add a Phase 14 precondition — no phase may be pending; each must be completed, or skipped with a reason — and make an unresolved phase fire AskUserQuestion rather than pass silently. This also backstops finding #1: even if the defaults are missed, finalization would force the question.


3. State is maintained by hand in two places and drifted three ways

orchestrator-state.yml and dashboard-data.js are written independently at every phase boundary. By the end they disagreed:

  • completed_phases stopped at phase-10 while the dashboard showed 11 and 14 complete.
  • skip_test_suite: false in state, but the Phase 11 verifier was invoked with skip_test_suite: true.
  • A duplicate phase-10 row sat in the dashboard for most of the run.
  • phase-14 was inserted ahead of 12/13, so the ledger rendered out of order.

None of these is individually serious; collectively they mean neither artifact can be trusted as the record of what happened.

Suggested fix: make dashboard-data.js a generated projection of orchestrator-state.yml — one writer, one source of truth. Failing that, have Phase 14 diff the two and fail loudly on disagreement.


4. The timestamp rule isn't enforceable as written — skills/orchestrator-framework

Several phase entries carried timestamps one day in the future relative to the actual date. They were reconstructed retroactively when back-filling phase records — exactly the drift the Timestamp Rule warns about.

The rule says to re-run date. That's a reminder, and reminders lose to the convenience of inventing a plausible value when back-filling.

Suggested fix: state it as a prohibition instead — never write a timestamp that was not read from date in the same turn; if you are back-filling a phase whose real time was not captured, write null, not a guess. A null is honest and visibly incomplete; a plausible wrong timestamp is neither.


5. implementation-planner emits a wave table that contradicts its own file data

The generated plan's "Execution Order" table declared three task groups file-disjoint and safe to run in parallel. Two of them both modified the same shared translation files (en.json / pl.json), which were correctly listed in each group's own Files to Modify. Dispatching that wave as written would have had two implementer subagents clobbering each other's translation keys — a conflict with no compile error and no test failure, discoverable only by noticing missing keys later.

implementation-plan-executor already contains the correct greedy file-disjointness computation, and applying it produced a different (correct) wave plan. So the planner is emitting a contradictory artifact that the executor must know to override.

Suggested fix: the planner should not emit a wave table at all. It should emit Dependencies and Files to Modify only, and let the executor derive waves — one implementation of the rule, in the component that already has it.


6. Repo-specific commit-message rules don't propagate into subagent prompts — skills/implementation-plan-executor

The target repo's instructions prohibit a specific commit trailer. Two task-group-implementer subagents added it anyway, and one of them explicitly reasoned that the harness default mandated it — i.e. it weighed a generic session-level default above the repo's own explicit instruction. This required rewriting history across the whole branch to correct.

Subagents receive task-group content, standards paths and spec excerpts, but the repo's commit/attribution constraints are not in the prompt template, so each subagent re-derives the policy from whatever it happens to see.

Suggested fix: add the repo's commit-message constraints to the task-group-implementer prompt template explicitly, and state that repo instructions outrank generic defaults. More generally: any constraint the orchestrator is relying on subagents to honour should be in the prompt, not assumed to be inherited.


What worked well, and is worth not regressing

  • The Phase 11 multi-agent verifier earned its cost. Four of five reviewers returned GO; the fifth found two genuine data-loss defects in a dimension none of the others examined (persistence of partially-completed work). Neither was reachable by TypeScript, lint, or a 4,500-test suite.
  • "A critical code-review finding forces a Failed verdict" is the rule that prevented a push. Without that hard mapping, "4 of 5 said GO" reads like a pass.
  • The instruction to verify subagent claims rather than accept them paid off twice — both criticals were independently reproduced before being reported, and separately, a claim made to another agent was disproved by it and had to be retracted. Worth keeping that framing prominent; the reports are persuasive and easy to take at face value.

Environment

Full run with html_output: true, sequential: false, and all four optional Phase 11 reviews enabled. Internal project identifiers omitted — this repo is public. Happy to supply the specific state files or phase ledger privately if useful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions