Retrospective: friction points from a review-mode run (midnight-node PR #1900)

[m2ux]

Friction points captured from a full review-mode run of the work-package workflow against midnightntwrk/midnight-node PR #1900 (a review of a third-party PR). Compiled from the transcripts of all 16 dispatched agents plus orchestration-level observations. Items already tracked in #270 are marked [#270].

Biggest signal: the work-package workflow is authored for the build/authoring path — payload sizes, variable seeding, the validate steps, the close-out convention, and the declared outcome array all assume you are creating a PR. Running it in review mode forced repeated reinterpretation and workarounds. That, plus consistently oversized get_activity payloads, accounts for most of the friction below.

A. Workflow-server MCP payload & tool ergonomics

1. get_activity payload routinely exceeds the tool-result token cap. The most pervasive friction — hit in start-work-package (97,287 chars), design-philosophy (81,287), codebase-comprehension, implementation-analysis, plan-prepare, and strategic-review (113,158). Every heavy activity's mandatory first call overflowed, spilled to a file, and forced chunked offset/limit reads before any step could run.
2. Oversized-payload compounded with a path typo into a retry loop. implementation-analysis made three failed reads of the spilled file (transposed session-id path segment) before recovering.
3. get_resource cannot load section-anchored / link-ref resources. lean-coding-audit received Resource not found for review-taxonomy, the-ladder, and ponytail-marker-convention (bare and #section, ~10 failed calls); it worked around this using the already-inlined step_techniques. These ids are inline-link refs, not standalone loadable resources — a documented mechanism that does not work for them.
4. Deferred-tool ToolSearch bootstrap tax. Each worker spends round-trips loading workflow-server tool schemas before real work begins.
5. next_activity never returned _meta.trace_token (orchestrator-observed). Trace accumulation stayed empty for the whole run, so the accumulate/resolve trace protocol (get_trace) was inert end to end.
6. next_activity has no usage field (orchestrator-observed). The dispatch-activity technique says to relay per-subagent token usage via a usage param, but the tool schema rejects it (additionalProperties: false), so usage can never be relayed.

B. Session state & variable consistency

7. Worker variable-changes do not persist to the server bag. State propagates only through orchestrator-authored prompts; e.g. skip_optional_activities=true set in design-philosophy read back as false in codebase-comprehension.
8. worktree_created mismatch. Dispatch prompt said true, the authoritative session bag said false, and the worktree physically existed — so remove-worktree was skipped and a real worktree was left un-cleaned; complete/end-workflow had to detect and escalate.
9. validation_passed disagreement. Client bag persisted false (no local build ran) while the prompt asserted true (analytical pass); end-workflow had to decide which gate governs.
10. Meta inspect_session returned stale values. is_monorepo=false, target_path="." from the bag vs the true true / midnight-node; the "source of truth" tool had to be overridden by prompt state.
11. Stale currentTechnique pointer metadata in the meta session (version-control::select-target-component while the activity was end-workflow).

C. Review-mode vs build-mode workflow fit (dominant cross-cutting theme)

12. [Review-mode: consolidated review links are dead at post time, forcing template abandonment #270] The consolidated review abandoned the mandated Consolidated Review Format (free-form prose) under dead-link pressure plus operator override instructions.
13. [Review-mode: consolidated review links are dead at post time, forcing template abandonment #270] Template links cannot resolve at post time — the base URL hardcodes /blob/main/, artifacts commit to the working branch, and there is no publish-before-post step (commit-after-activity vs commit-before-post).
14. validate structurally demands local cargo. Its bound steps (preflight, run-suite, fix-revalidate loop) intrinsically compile; on a machine where that OOMs and is forbidden, the CI-evidence substitute has to be injected by prompt rather than encoded in the workflow.
15. conduct-retrospective / COMPLETE.md assume an authored, merging PR. "Once the PR merged, update status" and the COMPLETE.md output do not fit reviewing a third-party PR; review mode gates off create-complete-doc, yet the retrospective's declared output is a COMPLETE.md section, so the worker had to manufacture 14-COMPLETE.md.
16. verify-outcomes has no review-mode outcome set. target_workflow_outcomes is the build-path array (ADR recorded, docs updated, worktree cleaned); the worker could not evaluate it for a review and had to reconstruct its own review-mode outcome list.
17. [Review-mode: consolidated review links are dead at post time, forcing template abandonment #270, secondary] Rating cap has no self-refutation carve-in. The review refuted the concern that set the cap, but the rule mechanically holds Overall Rating at Request Changes, and the review-summary-approval checkpoint offers no review-type choice.
18. workflow_completed fired before complete's own steps ran. Terminal status appeared mid-activity; harmless but had to be caveated.

D. Discovery & environment

19. Discovery picked the wrong workflow first. discover-session scored midnight-system-review as the top match (catalog tags review / system-review) before reversing to work-package, costing a re-scan. Tag-based scoring mis-points PR-review requests.
20. Recursive submodule refresh aborted on an inaccessible sibling. start-work-package's whole-monorepo git submodule update --remote failed on shieldedtech/midnight-intents-swaps (no access); it worked around by refreshing only the target submodule and touching the freshness sentinel — a deviation from the "refresh all" instruction forced by an access problem.
21. Base-drift diff trap. With the branch behind main, a naive git diff main...HEAD folds in unrelated BABE / pallet_session / spec_version work; every analysis activity had to scope to the merge-base three-dot diff to isolate the true authored surface.
22. GitNexus multi-repo disambiguation. post-impl-review's first context call failed ("Multiple repositories indexed. Specify … repo"); it retried with an explicit repo and succeeded.
23. WebFetch 404 on GitHub URLs. codebase-comprehension's fetch of polkadot-sdk content returned 404 ("use an authenticated tool e.g. gh"); it re-fetched via gh.
24. Sandbox guardrail blocked a shell loop. discover-session's for d in …; done batch loop hit a variable-expansion confirmation guardrail and fell back to per-file reads.

E. Orchestration protocol

25. Workers misreport checkpoint auto-advance capability. For review-findings, a worker claimed defaultOption + autoAdvanceMs existed; present_checkpoint showed neither, so it required interactive resolution. Checkpoint auto-advanceability had to be verified server-side before every resolution.
26. Background dispatch vs the foreground-always rule. The harness ran every worker in the background despite the technique's CRITICAL foreground requirement; checkpoint delivery survived (via completion), but it is a latent tension between the workflow's assumption and the harness.
27. Conflicting commit rules. commit-after-activity (MUST commit after every activity) vs explicit-commit (never commit unless the user asks) vs shared-branch-push caution; the orchestrator had to navigate these by deferring commits, which then needed a late explicit decision.
28. Operator error (recorded for completeness). The orchestrator misapplied a PR-review-reply prose-style convention to the consolidated review summary and wrongly assumed the planning links were inaccessible, which drove items 12/13. The underlying enabler is the dead-link condition in Review-mode: consolidated review links are dead at post time, forcing template abandonment #270; explicit guidance that the template is mandatory and its links are first-class would prevent recurrence.

---

Filed by an AI agent on behalf of Mike Clay following a live review-mode run (midnightntwrk/midnight-node PR #1900). Companion to #270, which covers the template/links/commit-ordering items in depth.

[m2ux]

Closing — PR #274 (merged 2026-07-28, merge commit b9b1056a on workflows) states it closes the still-open findings here: eleven findings across seven change goals.

Covered by #274: items 3 (cross-workflow ponytail/<id> resource ids), 8/9/10 (distrust-then-reconcile when inspect_session disagrees with the worker envelope), 15/16 (review-mode-native close-out and bag-seeded outcome set), 19 (discovery scoring favours actionable PR/implementation targets), 25 (checkpoint auto-advance verified, not asserted), 27 (commit-rule scoping cross-refs), plus 12/13/17 via the #270 half.

Covered elsewhere since this was filed: item 6 (next_activity had no usage param) — PR #329 / #324 B1; item 7 (worker variable changes not persisted to the bag) — PR #306; items 1/2 (oversized get_activity payloads) — partially, via PR #326, which returned fresh-mode delivery to the pre-regression baseline (#322 / #323 T1/T2/T4).

Not carried by any merged change. Closing this retrospective so it does not read as pending review; these remain visible here and are worth re-raising as scoped issues if they recur:

• 4 — deferred-tool ToolSearch bootstrap tax per worker
• 5 — next_activity never returned _meta.trace_token, so trace accumulation was inert end to end
• 11 — stale currentTechnique pointer metadata in the meta session
• 14 — validate structurally demands a local cargo build with no encoded CI-evidence substitute
• 18 — workflow_completed fired before complete's own steps ran
• 20 — recursive submodule refresh aborts on an inaccessible sibling
• 21 — base-drift diff trap (two-dot vs merge-base three-dot)
• 22/23 — GitNexus multi-repo disambiguation; WebFetch 404 on GitHub URLs
• 24 — sandbox guardrail blocks shell loops
• 26 — background dispatch vs the foreground-always rule

Item 28 is recorded as operator error; its enabler was #270, now closed.

Related open tracking: #317 (soft checkpoints self-resolve; autoAdvanceMs unenforced), #319 (residual items from the #274 continuation pass), #323 (delivery cost, T3 outstanding).