-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
dispatch-e2e validation + agent time discipline rules #10024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
c1b52cd
96181f6
8778eac
03e3a26
b7caca1
8d93900
b91b7ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -60,11 +60,37 @@ If the iPhone is unreachable at build time, the reload still completes: the sign | |||||
|
|
||||||
| Two commits, so CI proves the test catches the bug: commit 1 adds the failing test only (CI red), commit 2 adds the fix (CI green). This is visible in the PR Commits tab. | ||||||
|
|
||||||
| ## Hosted E2E dispatch | ||||||
|
|
||||||
| Dispatch hosted UI/E2E runs through the validated wrapper. It checks every filter item against the local test sources and refuses to dispatch a filter that matches zero tests, which otherwise wastes a full dispatch+watch cycle before the run fails: | ||||||
|
|
||||||
| ```bash | ||||||
| ./scripts/dispatch-e2e.sh --ref <branch-or-sha> --filter "<Class or Class/method>[,more]" --watch | ||||||
| ``` | ||||||
|
|
||||||
| `--dry-run` validates and prints the `gh` command without dispatching; `--runner`, `--record-video`, `--timeout` (per-test seconds), and `--job-timeout` (minutes) pass through. Raw fallback: | ||||||
|
|
||||||
| ```bash | ||||||
| gh workflow run test-e2e.yml --repo manaflow-ai/cmux -f ref=<branch-or-sha> -f test_filter="<Class or Class/method>" | ||||||
| ``` | ||||||
|
|
||||||
| At most one dispatch per dogfood round. A red run means root-cause locally or on a fleet simulator first; never loop dispatch-fix-redispatch. A new XCUITest needs one green hosted run before the task is done; nothing else does. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Define the dispatch cap in terms of hosted runs.
Proposed wording-At most one dispatch per dogfood round.
+Use at most one filter item per dogfood round. The wrapper dispatches one hosted run per comma-separated filter item.📝 Committable suggestion
Suggested change
🧰 Tools🪛 LanguageTool[grammar] ~77-~77: Ensure spelling is correct (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| ## Agent time discipline | ||||||
|
|
||||||
| Every poll slice pays a full model reasoning pass, so waiting is where agent hours disappear. | ||||||
|
|
||||||
| - **Park on one blocking command per wait**: `gh run watch <id> --exit-status`, `./scripts/dispatch-e2e.sh --watch`, or `gh pr checks --watch`. Never 30-60s poll slices, sleep-and-recheck chains, or PTY heartbeat polls. | ||||||
| - **One build dispatch per need.** The cloud reload waits internally for a builder slot; never cycle `RELOAD_CLOUD_BUILDER` or re-dispatch a build that is already queued. | ||||||
| - **Batch fixes per rebuild.** Accumulate a dogfood round's fixes, preflight with focused tests, then do one tagged rebuild for the round. Never rebuild per one-line fix. | ||||||
| - **Locked or offline iPhone: probe once, then queue.** One reachability probe, enqueue in the install queue, notify, and end the turn with "queued; unlock to receive". Never write unlock/ready watcher scripts or repeat devicectl probes. | ||||||
| - **CI is advisory.** Never block a handoff on bot checks, and never re-dispatch a failing hosted run without a local root cause. | ||||||
|
|
||||||
| ## First pass, then dogfood | ||||||
|
|
||||||
| A first pass ends when the change is implemented, the tagged build succeeded on the pushed HEAD, focused tests ran, and the PR is open (for `web/` PRs, also the live Vercel preview URL). Then hand off to the user. Do not sit in the main conversation watching CI or running speculative review passes after that point. | ||||||
|
|
||||||
| Do not launch a background review agent (`$autoreview`, `codex review`, `claude review`, or a judge loop) by default. Second-model review is explicit user opt-in in the current conversation; an implementation request, open PR, CI failure, closeout, or handoff is not that opt-in. Let required GitHub checks and the automatic review bots run asynchronously, then return to address only concrete check failures and actionable findings before merge. | ||||||
| Do not launch a background review agent (`$autoreview`, `codex review`, `claude review`, or a judge loop) by default. Second-model review is explicit user opt-in in the current conversation; an implementation request, open PR, CI failure, closeout, or handoff is not that opt-in. PR checks are advisory review bots only; there are no required status checks. Let them run asynchronously and return only for actionable findings. Merge validation happens at merge time via the merge gate, not by watching PR checks. | ||||||
|
|
||||||
| The main agent owns dogfood, approval, mergeability, and every pushed fix. Merging app/runtime/UI changes requires the user's explicit approval after dogfood; if a fix changes runtime behavior mid-dogfood, rebuild the tag and re-notify, since the earlier verdict covers only the build the user tested. | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,345 @@ | ||
| #!/usr/bin/env bash | ||
| # Validated dispatcher for the hosted E2E workflow (test-e2e.yml). | ||
| # | ||
| # A test_filter that matches zero tests still costs a full CI dispatch+watch | ||
| # cycle before the run fails with "executed 0 tests". This wrapper validates | ||
| # every filter item against the local checkout first and refuses to dispatch | ||
| # on a miss, printing the nearest candidate classes instead. | ||
| # | ||
| # Usage: | ||
| # scripts/dispatch-e2e.sh --ref <branch-or-sha> --filter "<Class or Class/method>[,more]" \ | ||
| # [--runner <runner>] [--record-video true|false] [--timeout <seconds>] \ | ||
| # [--job-timeout <minutes>] [--watch] [--dry-run] | ||
| # | ||
| # Filter items are comma-separated; each item dispatches its own workflow run. | ||
| # A bare "Class" or "Class/method" targets cmuxUITests (the workflow's | ||
| # back-compat default). Target-qualified "cmuxTests/Class[/method]" or | ||
| # "cmuxUITests/Class[/method]" is validated against that target's directory. | ||
| # | ||
| # Validation reads THIS checkout's test sources, so run it from the worktree | ||
| # that matches the ref you dispatch. | ||
| # | ||
| # Exit codes: | ||
| # 0 dispatched (and, with --watch, all runs passed) | ||
| # 1 usage error, dispatch failure, or watched run failed | ||
| # 2 validation failed (class or method not found) | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| REPO="manaflow-ai/cmux" | ||
| WORKFLOW="test-e2e.yml" | ||
| ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" | ||
|
|
||
| usage() { | ||
| sed -n '2,26p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' | ||
| } | ||
|
|
||
| die() { | ||
| echo "error: $*" >&2 | ||
| exit 1 | ||
| } | ||
|
|
||
| REF="" | ||
| FILTERS_RAW="" | ||
| RUNNER="" | ||
| RECORD_VIDEO="" | ||
| TEST_TIMEOUT="" | ||
| JOB_TIMEOUT="" | ||
| WATCH=0 | ||
| DRY_RUN=0 | ||
|
|
||
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| --ref) | ||
| [ $# -ge 2 ] || die "--ref needs a value" | ||
| REF="$2" | ||
| shift 2 | ||
| ;; | ||
| --filter) | ||
| [ $# -ge 2 ] || die "--filter needs a value" | ||
| FILTERS_RAW="$2" | ||
| shift 2 | ||
| ;; | ||
| --runner) | ||
| [ $# -ge 2 ] || die "--runner needs a value" | ||
| RUNNER="$2" | ||
| shift 2 | ||
| ;; | ||
| --record-video) | ||
| [ $# -ge 2 ] || die "--record-video needs true or false" | ||
| RECORD_VIDEO="$2" | ||
| shift 2 | ||
| ;; | ||
| --timeout) | ||
| [ $# -ge 2 ] || die "--timeout needs a value (per-test seconds)" | ||
| TEST_TIMEOUT="$2" | ||
| shift 2 | ||
| ;; | ||
| --job-timeout) | ||
| [ $# -ge 2 ] || die "--job-timeout needs a value (job minutes)" | ||
| JOB_TIMEOUT="$2" | ||
| shift 2 | ||
| ;; | ||
| --watch) | ||
| WATCH=1 | ||
| shift | ||
| ;; | ||
| --dry-run) | ||
| DRY_RUN=1 | ||
| shift | ||
| ;; | ||
| -h | --help) | ||
| usage | ||
| exit 0 | ||
| ;; | ||
| *) | ||
| die "unknown argument: $1 (see --help)" | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| [ -n "$REF" ] || die "--ref is required" | ||
| [ -n "$FILTERS_RAW" ] || die "--filter is required" | ||
| if [ -n "$RECORD_VIDEO" ] && [ "$RECORD_VIDEO" != "true" ] && [ "$RECORD_VIDEO" != "false" ]; then | ||
| die "--record-video must be true or false" | ||
| fi | ||
| if [ -n "$TEST_TIMEOUT" ] && ! [[ "$TEST_TIMEOUT" =~ ^[0-9]+$ ]]; then | ||
| die "--timeout must be an integer (seconds)" | ||
| fi | ||
| if [ -n "$JOB_TIMEOUT" ] && ! [[ "$JOB_TIMEOUT" =~ ^[0-9]+$ ]]; then | ||
| die "--job-timeout must be an integer (minutes)" | ||
| fi | ||
|
|
||
| # --- validation ------------------------------------------------------------- | ||
|
|
||
| # All class names declared under a test target directory. | ||
| list_classes() { | ||
| local dir="$1" | ||
| grep -rhoE '\bclass[[:space:]]+[A-Za-z_][A-Za-z0-9_]*' --include='*.swift' "$dir" 2>/dev/null | | ||
| awk '{print $2}' | sort -u | ||
| } | ||
|
|
||
| # Files declaring the given class. | ||
| class_files() { | ||
| local dir="$1" cls="$2" | ||
| grep -rlE "class[[:space:]]+${cls}\b" --include='*.swift' "$dir" 2>/dev/null || true | ||
| } | ||
|
|
||
| # Nearest candidates for a missed name, best first. | ||
| suggest_names() { | ||
| local query="$1" | ||
| shift | ||
| [ $# -gt 0 ] || return 0 | ||
| if command -v python3 >/dev/null 2>&1; then | ||
| python3 - "$query" "$@" <<'PY' | ||
| import difflib | ||
| import sys | ||
|
|
||
| query = sys.argv[1] | ||
| names = sys.argv[2:] | ||
| ranked = difflib.get_close_matches(query, names, n=5, cutoff=0.3) | ||
| q = query.lower() | ||
| for name in names: | ||
| if name in ranked: | ||
| continue | ||
| n = name.lower() | ||
| if q in n or n in q: | ||
| ranked.append(name) | ||
| print("\n".join(ranked[:5])) | ||
| PY | ||
| else | ||
| printf '%s\n' "$@" | grep -i -- "$query" | head -5 || true | ||
| fi | ||
| } | ||
|
|
||
| validate_item() { | ||
| local item="$1" | ||
| local target_dir="$ROOT/cmuxUITests" | ||
| local target_name="cmuxUITests" | ||
| local rest="$item" | ||
|
|
||
| case "$item" in | ||
| cmuxTests/*) | ||
| target_dir="$ROOT/cmuxTests" | ||
| target_name="cmuxTests" | ||
| rest="${item#cmuxTests/}" | ||
| ;; | ||
| cmuxUITests/*) | ||
| rest="${item#cmuxUITests/}" | ||
| ;; | ||
| esac | ||
|
|
||
| local cls="${rest%%/*}" | ||
| local method="" | ||
| if [ "$rest" != "$cls" ]; then | ||
| method="${rest#*/}" | ||
| fi | ||
|
|
||
| if ! [[ "$cls" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then | ||
| die "invalid class name in filter item '$item'" | ||
| fi | ||
| if [ -n "$method" ] && ! [[ "$method" =~ ^[A-Za-z_][A-Za-z0-9_]*(\(\))?$ ]]; then | ||
| die "invalid method name in filter item '$item'" | ||
| fi | ||
| [ -d "$target_dir" ] || die "test directory not found: $target_dir" | ||
|
|
||
| local files | ||
| files="$(class_files "$target_dir" "$cls")" | ||
| if [ -z "$files" ]; then | ||
| echo "error: no 'class $cls' found under $target_name/ for filter item '$item'" >&2 | ||
| local -a all_classes=() | ||
| while IFS= read -r name; do | ||
| if [ -n "$name" ]; then | ||
| all_classes+=("$name") | ||
| fi | ||
| done < <(list_classes "$target_dir") | ||
| local suggestions="" | ||
| if [ "${#all_classes[@]}" -gt 0 ]; then | ||
| suggestions="$(suggest_names "$cls" "${all_classes[@]}")" | ||
| fi | ||
| if [ -n "$suggestions" ]; then | ||
| echo "nearest candidates:" >&2 | ||
| printf '%s\n' "$suggestions" | sed 's/^/ /' >&2 | ||
| fi | ||
| exit 2 | ||
| fi | ||
|
|
||
| if [ -n "$method" ]; then | ||
| local method_bare="${method%()}" | ||
| local found=0 | ||
| local file | ||
| while IFS= read -r file; do | ||
| if grep -qE "func[[:space:]]+${method_bare}\b" "$file"; then | ||
| found=1 | ||
| break | ||
| fi | ||
| done <<<"$files" | ||
| if [ "$found" -eq 0 ]; then | ||
| echo "error: no 'func $method_bare' found in class $cls for filter item '$item'" >&2 | ||
| local -a all_methods=() | ||
| while IFS= read -r name; do | ||
| if [ -n "$name" ]; then | ||
| all_methods+=("$name") | ||
| fi | ||
| done < <( | ||
| while IFS= read -r f; do | ||
| [ -n "$f" ] || continue | ||
| grep -hoE 'func[[:space:]]+test[A-Za-z0-9_]*' "$f" 2>/dev/null || true | ||
| done <<<"$files" | awk '{print $2}' | sort -u | ||
| ) | ||
| local suggestions="" | ||
| if [ "${#all_methods[@]}" -gt 0 ]; then | ||
| suggestions="$(suggest_names "$method_bare" "${all_methods[@]}")" | ||
| fi | ||
| if [ -n "$suggestions" ]; then | ||
| echo "nearest candidates:" >&2 | ||
| printf '%s\n' "$suggestions" | sed 's/^/ /' >&2 | ||
| fi | ||
| exit 2 | ||
| fi | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| fi | ||
|
|
||
| echo "ok: $item (class $cls${method:+, method ${method%()}} in $target_name/)" | ||
| } | ||
|
|
||
| # --- dispatch --------------------------------------------------------------- | ||
|
|
||
| dispatch_args_for() { | ||
| local filter="$1" | ||
| DISPATCH_ARGS=(workflow run "$WORKFLOW" --repo "$REPO" -f "ref=$REF" -f "test_filter=$filter") | ||
| if [ -n "$RUNNER" ]; then | ||
| DISPATCH_ARGS+=(-f "runner=$RUNNER") | ||
| fi | ||
| if [ -n "$RECORD_VIDEO" ]; then | ||
| DISPATCH_ARGS+=(-f "record_video=$RECORD_VIDEO") | ||
| fi | ||
| if [ -n "$TEST_TIMEOUT" ]; then | ||
| DISPATCH_ARGS+=(-f "test_timeout=$TEST_TIMEOUT") | ||
| fi | ||
| if [ -n "$JOB_TIMEOUT" ]; then | ||
| DISPATCH_ARGS+=(-f "job_timeout=$JOB_TIMEOUT") | ||
| fi | ||
| } | ||
|
|
||
| # Run ids that exist before dispatch, so the new run can be told apart. | ||
| existing_run_ids() { | ||
| gh run list --repo "$REPO" --workflow "$WORKFLOW" --limit 30 \ | ||
| --json databaseId --jq '.[].databaseId' 2>/dev/null || true | ||
| } | ||
|
|
||
| # The workflow's run-name starts with the test_filter, so match on that | ||
| # among runs that did not exist before dispatch. | ||
| resolve_run_id() { | ||
| local filter="$1" pre_ids="$2" | ||
| local attempt id title | ||
| for attempt in $(seq 1 15); do | ||
| while IFS=$'\t' read -r id title; do | ||
| [ -n "$id" ] || continue | ||
| if printf '%s\n' "$pre_ids" | grep -qx "$id"; then | ||
| continue | ||
| fi | ||
| case "$title" in | ||
| "$filter on "*) | ||
| echo "$id" | ||
| return 0 | ||
| ;; | ||
| esac | ||
| done < <(gh run list --repo "$REPO" --workflow "$WORKFLOW" --limit 30 \ | ||
| --json databaseId,displayTitle --jq '.[] | [.databaseId, .displayTitle] | @tsv' 2>/dev/null || true) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| if [ "$attempt" -lt 15 ]; then | ||
| sleep 2 | ||
| fi | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| done | ||
| return 1 | ||
| } | ||
|
|
||
| IFS=',' read -r -a FILTER_ITEMS <<<"$FILTERS_RAW" | ||
| CLEAN_ITEMS=() | ||
| for raw_item in "${FILTER_ITEMS[@]}"; do | ||
| item="$(echo "$raw_item" | sed 's/^[[:space:]]*//; s/[[:space:]]*$//')" | ||
| [ -n "$item" ] || continue | ||
| CLEAN_ITEMS+=("$item") | ||
| done | ||
| [ "${#CLEAN_ITEMS[@]}" -gt 0 ] || die "--filter contained no filter items" | ||
|
|
||
| for item in "${CLEAN_ITEMS[@]}"; do | ||
| validate_item "$item" | ||
| done | ||
|
Comment on lines
+372
to
+374
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift Build the test-source index once for the filter batch. Each Build one per-target class and method index before this loop. Query that index for each filter. As per coding guidelines, avoid repeated batch rescans over scalable collections. As per path instructions, apply 🤖 Prompt for AI AgentsSources: Coding guidelines, Path instructions |
||
|
|
||
| if [ "$DRY_RUN" -eq 1 ]; then | ||
| echo "dry run: validation passed; would dispatch:" | ||
| for item in "${CLEAN_ITEMS[@]}"; do | ||
| dispatch_args_for "$item" | ||
| printf 'gh' | ||
| printf ' %q' "${DISPATCH_ARGS[@]}" | ||
| printf '\n' | ||
| done | ||
| exit 0 | ||
| fi | ||
|
|
||
| RUN_IDS=() | ||
| for item in "${CLEAN_ITEMS[@]}"; do | ||
| dispatch_args_for "$item" | ||
| pre_ids="$(existing_run_ids)" | ||
| echo "dispatching: $item @ $REF" | ||
| gh "${DISPATCH_ARGS[@]}" | ||
| if run_id="$(resolve_run_id "$item" "$pre_ids")"; then | ||
| RUN_IDS+=("$run_id") | ||
| echo "run: https://github.com/$REPO/actions/runs/$run_id" | ||
| else | ||
| echo "warning: dispatched but could not resolve the new run id for '$item'." >&2 | ||
| echo " gh run list --repo $REPO --workflow $WORKFLOW --limit 5" >&2 | ||
| fi | ||
| done | ||
|
|
||
| if [ "$WATCH" -eq 1 ]; then | ||
| [ "${#RUN_IDS[@]}" -gt 0 ] || die "--watch requested but no run ids were resolved" | ||
| FAILED=0 | ||
| for run_id in "${RUN_IDS[@]}"; do | ||
| echo "watching run $run_id..." | ||
| if ! gh run watch "$run_id" --repo "$REPO" --exit-status; then | ||
| FAILED=1 | ||
| fi | ||
| done | ||
| exit "$FAILED" | ||
| fi | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
State that the checkout must match the dispatched ref.
The wrapper validates sources from the current checkout, not from
--ref. If these differ, validation can reject a selector that exists in the dispatched ref or approve one that does not.CLAUDE.md#L65-L74: State that the command must run from a worktree checked out at<branch-or-sha>.skills/cmux-testing/references/local-vs-ci-validation.md#L20-L20: Add the same checkout/ref precondition beside the wrapper command.📍 Affects 2 files
CLAUDE.md#L65-L74(this comment)skills/cmux-testing/references/local-vs-ci-validation.md#L20-L20🤖 Prompt for AI Agents