Generalize the mobile agent workflow into a repo-wide kilo workflow - #4812
Conversation
Four repo-wide role agents (plan-reviewer, implementer, impl-reviewer, e2e-verifier) generalized from apps/mobile/.kilo/agent/, discoverable from any cwd via the root .kilo/ config dir. The e2e-verifier moves to kimi-k3 with no step ceiling and bakes in output discipline and the learnings contract.
Generalizes apps/mobile/.kilo/MOBILE_WORKFLOW.md into a surface-agnostic workflow (starter -> planners -> orchestrator -> role-agent loops -> PR), copies the e2e slot semaphore (same machine-global state dir), and seeds learnings/ from WORKFLOW_LEARNINGS.md as one indexable file per entry; learnings/system/ is gitignored for machine-local entries. The mobile .kilo and e2e/ are left untouched pending the follow-up simplification.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers the single commit added since the last review ( Files Reviewed (1 file)
Previous Review Summaries (15 snapshots, latest commit 74d86e5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 74d86e5)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers the commits since the last review ( Files Reviewed (14 files)
Previous review (commit cfea850)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers the commits since the last review ( Files Reviewed (6 files)
Previous review (commit 8c4292b)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the single commit since the last review — wording tightening in Files Reviewed (1 file)
Previous review (commit 09048c2)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the single commit since the last review — removal of the stale Files Reviewed (2 files)
Previous review (commit 2e8da78)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the single commit since the last review — removal of the stale Files Reviewed (2 files)
Previous review (commit 2f782e9)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the commits since the last review — removal of two stale learnings docs ( Files Reviewed (3 files)
Previous review (commit fe8d519)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the single commit since the last review — wording clarifications in Files Reviewed (2 files)
Previous review (commit f9dc9b5)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the single commit since the last review — moving the KILO_/OPENCODE env-strip computation in Files Reviewed (1 file)
Previous review (commit 36d23c5)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the single commit since the last review — a documentation-only correction of the Kilobot retrigger mention handle from Files Reviewed (1 file)
Previous review (commit dc5934d)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the commits since the last review; the previous WARNING on Files Reviewed (3 files)
Previous review (commit 9eb5c5e)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThis incremental review covers only the commits since the last review; the previous WARNING on Overview
Issue Details (click to expand)WARNING
Files Reviewed (14 files)
Fix these issues in Kilo Cloud Previous review (commit 5ef238d)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review covers only the commits since the last review; all changed files are workflow/agent documentation ( Files Reviewed (19 files)
Previous review (commit 9212f6a)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe simulator-claim simplification in Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 4fcc7da)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the latest commits: workflow doc wording tweaks (uncapped agent parallelism, E2E-slot cap clarification, worktree-prepare learnings note), path updates in Files Reviewed (11 files)
Previous review (commit 47d0fb6)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the new repo-wide kilo workflow docs, four role-agent configs, and the Files Reviewed (18 files)
Reviewed by claude-sonnet-5 · Input: 30 · Output: 4.7K · Cached: 686.8K Review guidance: REVIEW.md from base branch |
Removes apps/mobile/.kilo (workflow doc, learnings, role agents, slot script) now superseded by .kilo_workflow and root .kilo/agent; repoints apps/mobile docs at the new paths. Worktree prep now copies the machine-local learnings/system/ folder into fresh worktrees, and the workflow doc states that agent parallelism is uncapped - only E2E device/stack phases contend for slots.
The claim protocol predated the machine-global e2e slot semaphore and duplicated its job with a preparing/ready state machine, PID-identity liveness, recovery resets, and phase-aware relabeling. With device phases capped by e2e-slot.sh, a claim only needs to answer which worktree owns which simulator: an atomic JSON lock file, stale once its worktree is deleted, labeled 'Kilo E2E - <worktree>' on claim and restored on release. Boot verification (bootstatus terminal-failure detection) is kept. The --phase flag is gone along with the prewarm concept the workflow no longer has; the build-cache claim validation now checks ownership only. dev/local/mobile-simulator.ts drops from 1093 to ~430 lines and its test file from 2921 to ~470. The one failing mobile-workflow test (settle-app Open guard) is pre-existing on main.
Desk review, five fresh-context probes (kimi-k3, grok-4.5, opus, two haiku role-plays, kilo-auto/efficient), and kilo CLI's own memory surfaced gaps; the valid ones are fixed: - Slot ownership: device-phase dispatches get their own tmux session (slots are owned and reaped by session name; window-named owners leak or share slots); verifier tears down services before releasing. - Terminal states: the PR is the deliverable and everything else closes. COMPLETE deletes the scratch dir (its absence is the completion sentinel); BLOCKED leaves scratch + final-report.md as evidence. Monitors distinguish crash from completion by scratch state. - Void rounds: every role definition now requires a fixed sentinel line; a log without one is a crashed round, never a pass. - Dispatches: handoffs travel via --file only (shell substitution through a tmux command string executes backticks in the file); env strip covers all KILO_*/OPENCODE* vars; --model dropped for role agents (definitions pin models); concrete launch snippets with shell-expanded scratch paths; starter monitors planners. - Loops: hard round caps with recorded-resolution escapes; the plan-review deadlock-breaker has an else branch; Kilobot wait has retriggers, a waiver, and concrete gh checks (retriggers observed dead on #4776 per kilo memory - green check on head is the review). - Verifier learnings go to scratch (byte-identical baseline restore) or the main checkout's learnings/system/; the orchestrator commits them. .kilo_workflow/ is exempt from section disjointness. - Six learnings added from kilo CLI memory and private harness memory (backtick execution, startup db race, paid-model auth, steering, orchestrator relaunch, simulator E2E limits, nextjs suspend protocol); stale entries updated (no --model, full env strip, sentinel-based void detection, dead retriggers); e2e runbook fixes (persistent remote relay, multi-sim UDID targeting).
…im leak Second hardening pass from five more fresh-context reviews (grok-4.5, kimi-k3, kilo-auto/efficient, gpt-5.6-sol via codex, plus a deep verification agent) run against the updated doc, looping until rounds stopped producing valid findings: - .kilo_workflow/dispatch-role.sh: checked-in dispatcher encoding the whole contract (tmux wrapping, dedicated session for device roles, full env strip, quoting, log naming, EXITCODE marker) so no agent hand-assembles it; the doc's dispatch section shrinks to its usage. - starter/planner/orchestrator agent definitions in .kilo/agent/ pin permissions (and the orchestrator's model), so unattended runs no longer depend on this machine's global kilo config. - Reviewer sentinels are machine-checkable (FINDINGS: n / No findings. / STOPPED EARLY. as the last line); the verifier sentinel now follows a truth table; void rounds count toward caps and three consecutive voids are an infrastructure blocker; caps are ceilings with only takeover/BLOCKED left at the cap. - Slice review diffs include new files (git add -N); cumulative review base is origin/main...HEAD and runs after a defined synchronization point (deferred project-wide checks); commits stage owned paths only. - BLOCKED evidence is retained (monitor no longer cleans it); a scratch-gone state without a PR is a crash, not COMPLETE; planners write BLOCKED reports too; monitor relaunches cap at three. - E2E loop: quiescence rule before dispatch, leftover cleanup after a dead verifier, E2E-Kilobot alternation cap; slot script reap has a grace window for mid-acquire slots and hardcodes its contract. - Kilobot thread state via GraphQL (new learning with exact queries); clean state is zero unresolved threads; PRs use the repo template with what/why/how inside it. - mobile-simulator: a failed boot whose recovery shutdown also fails now keeps the claim (flagged error) so a peer cannot adopt a possibly-running device - found by Kilobot on this PR; test added. - Learnings: GitHub threads API added; env-strip and dispatch entries updated; system-learnings carve-out for the main checkout.
…osition, round definition grok convergence review on the hardened doc found four real defects, all introduced by the prior batch: the env-strip grep aborts dispatch-role.sh under pipefail when no KILO_*/OPENCODE* vars exist (a non-kilo dispatcher - now || true, with the same guard in the launch templates); the sentinel is the line above the EXITCODE marker, so void detection reads tail -2 | head -1 instead of the last line or a whole-log grep; the repro gate's blocker phrase now matches the verifier's exact CANNOT REPRODUCE. sentinel; and one round is defined as one full loop-body iteration so caps are unambiguous.
Kilobot: stat -f %m is BSD/macOS-only; on GNU/Linux it switches to filesystem mode, the fallback made age always 0, and the ownerless-slot grace reap silently never fired. Try stat -f %m then stat -c %Y.
Panel review: STRIP was evaluated from the dispatcher's environment and baked into the command, but tmux panes inherit the tmux SERVER env, so server-carried KILO_*/OPENCODE* strays missing from the dispatcher's env leaked into role agents - the exact poisoning path the strip exists to close (and the header's 'full strip' claim was false). The strip is now an escaped substitution that evaluates in the pane. Smoke-tested via a stub kilo binary with a poisoned tmux global env: zero leaked vars.
…check, grok consistency nits Final convergence panel (kimi-k3, kilo-auto/efficient, gpt-5.6-sol, grok-4.5, plus a deep verification agent) on the finished spec: kimi and efficient returned clean; the rest produced five one-line fixes, all absorbed - the Kilobot waiver is now an explicit loop exit alternative; the completion monitor checks the section branch's PR in every touched repository instead of bare gh pr view; the pre-round reviewer snapshot is ordered after the add -N diff generation; and the verifier definition uses the exact CANNOT REPRODUCE. sentinel and lists the extension runbook.
Drop the now-undocumented deterministic-turns mocking exception, align the planner's gate shorthand and the retrigger waiver message with the approving-summary contract.
jeanduplessis
left a comment
There was a problem hiding this comment.
What is the thinking with having .kilo_workflow as a top-level root directory? Why not put it in the existing .kilo/ directory?
jeanduplessis
left a comment
There was a problem hiding this comment.
Before landing these changes, could we add instructions/context on how they interact, get triggered, etc. to something like DEVELOPMENT.md or another relevant documentation file?
These changes feel like the primitives of something bigger that isn't clear yet as part of this PR.
The flow is meant to be self-improving, with the agent delivering learnings and workflow improvements as part of the PR it delivers. Kilo CLI can't edit anything in the .kilo folder without triggering a permission prompt, even when given explicit access. Moving everything to a different folder circumvents this. |
Resolves the modify/delete conflict on apps/mobile/.kilo/WORKFLOW_LEARNINGS.md in favor of the delete. Learnings main added since the branch point that the generalized layout does not already cover are ported as individual files: uncommitted learning edits reverted by role agents, mobile cloud-agent create needs a GitHub integration, extension analytics posthog key at build time, Firefox geckodriver newSession under load, and the AirPlay port-offset collision. Entries already covered by .kilo_workflow/learnings/ or WORKFLOW.md (TTY, interactive wedge, nested env, --file handoffs, kilobot handle, extension scope) are dropped, along with the safe-mode fixture note (to be fixed in the fixture), the worklets bundle-mode note, and the R2 credential outage (resolved at the infra level).
The uncommitted-learnings trap described the retired mobile roles' revert-anything-dirty behavior; the generalized verifier restores only its own edits and WORKFLOW.md already forbids dispatch over uncommitted changes. Fold the durable rule (commit learnings promptly; planner-authored learnings are named in the handoff for the first commit) into the Learnings section and drop the learning file. Cross-link the port-offset learning and the e2e runbook's no-export rule so the per-command prefix does not read as a contradiction.
mockKiloApi asserted dangerousToolNames by default while fresh conversations default to safe mode, so every safe-mode spec had to pass toolNames explicitly and carried its own copy of the safe list. Export safeToolNames and dangerousToolNames from the fixture, default the assertion to safe mode, drop the seven spec-local copies, and pass dangerousToolNames explicitly in the nine specs that switch the conversation to dangerous mode. Full chrome e2e suite: 95 passed, 0 failed.
Reviewer-requested orientation doc: how to launch a run, the two terminal states, the pipeline and roles at a glance, the wall-time-for-correctness trade the workflow makes, and what each file in the directory is for.
What
Replaces the mobile-only agent workflow with a surface-agnostic, adversarially hardened one:
.kilo_workflow/WORKFLOW.md— the workflow: starter → planner(s) → orchestrator → implementer/impl-reviewer loops → E2E verification → PR. A starter grills requirements (hands-on/hands-off) and splits work into disjoint sections; one planner/worktree/orchestrator/PR per section. Agent parallelism is uncapped; only E2E device/stack phases contend for slots. The PR is the deliverable — on COMPLETE everything closes and the deleted scratch dir is itself the completion sentinel; BLOCKED retains scratch + a final report as evidence..kilo/agent/— seven definitions:starter,planner,orchestrator(permissions pinned so unattended runs don't depend on machine-local global config; orchestrator pins kimi-k3) and the four worker roles (plan-reviewer,implementer,impl-reviewer,e2e-verifier), each with machine-checkable final-line sentinels so a crashed round can never read as a pass..kilo_workflow/dispatch-role.sh— checked-in dispatcher encoding the fragile parts (tmux wrapping, dedicated sessions for device roles, fullKILO_*/OPENCODE*env strip, quoting, log naming,EXITCODEmarker)..kilo_workflow/e2e-slot.sh— slot semaphore, hardcoded contract, mid-acquire grace window in reap..kilo_workflow/learnings/— 23 committed symptom/cause/fix entries: migrated fromWORKFLOW_LEARNINGS.md, private harness memory, and kilo CLI's own memory (deduped, bucketed);learnings/system/is gitignored for machine-local entries and copied into fresh worktrees byworktree-prepare.sh.apps/mobile/.kilo(superseded) and repoints mobile docs; simplifies the simulator claim to a worktree-owned lock file (mobile-simulator.ts1093 → ~450 lines, tests 2921 → ~530) — the preparing/ready protocol, PID-identity liveness, recovery resets, and--phasemachinery predated the global slot semaphore.Why
The mobile workflow proved the planner/orchestrator/role-agent loop but was locked to
apps/mobile, and its device-ownership machinery predated the slot semaphore. The goal for the generalized version: even a weak model can follow it mechanically, mistakes land in learnings or doc fixes instead of repeating, and nothing fails silently.How it was hardened
Ten fresh-context adversarial reviews across two loops — kimi-k3 (×2), grok-4.5 (×3, incl. convergence), kilo-auto/efficient (×2), gpt-5.6-sol via codex, two role-play walkthroughs by a deliberately weak model, plus a deep verification agent — with every finding triaged as untrusted and bloat-adding suggestions rejected. Live probes validated the mechanics end to end: a real
plan-reviewerdispatch (which experimentally disproved a mock plan's assumptions), an implementer boundary probe (fixed, checked, reported, did not commit), and a comprehension exam the actual orchestrator model (kimi-k3) passed 8/8. Notable structural fixes that came out of it: slot ownership moved to per-dispatch tmux sessions (window-named owners silently defeated the cap), terminal-state semantics that distinguish crash from completion, sentinel-gated void-round detection for every role, round caps that interlock with the escalation ladder (ceilings, not budgets), and a claim-leak fix in the simulator wrapper found by Kilobot on this PR (a failed boot whose recovery shutdown also failed left a running device adoptable).Checks: typecheck, lint green; simulator tests 27/27;
test:mobile-workflow124/125 (the settle-appOpen-guard failure is pre-existing on main).