Skip to content

feat(land): the watcher writes flaky tests' follow-up quests - #551

Merged
moq-bot[bot] merged 2 commits into
mainfrom
quest/m0/queue-watcher/flaky-quest
Nov 7, 2026
Merged

moq-bot[bot] merged 2 commits into
mainfrom
quest/m0/queue-watcher/flaky-quest

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m0/queue-watcher/flaky-quest.md.

Summary

  • quest land watch picks a flaky test's follow-up quest path (quest/m0/flaky/<name>.md) at its first verdict, stores it in flaky.json, and passes it in the fixer's known-flaky input (KnownFlaky.quest) so the skip reason names it.
  • The fixer's disable commit holds only the skip; the watcher refuses one that changes anything under quest/, then commits the quest from a fixed [S] template on top (as moq-bot), plus quest/m0/flaky/README.md when the base has none. The epic README is written with no child list, relying on directory membership from moq-dev/quest#518, so it works after moq-dev/quest#537 deletes the epic.
  • assets/land-fix.md no longer tells the fixer to write a quest or epic link; README and the child-lists quest updated.

Verification

  • just check passes; full vitest run passes.
  • The end-to-end disable test now covers: first verdict ignored, protected-path edit refused, quest/ edit refused, and a valid skip opened with the watcher's quest commit (epic created).
  • Generated quests pass quest check both with the current quest/m0/flaky epic (which still lists its child) and with the epic and its m0 link removed as test: split the reviewer usage-limit test per case and unskip it #537 does.

Choices made without asking

  • Name: the failure's last segment after : or >, kebab-cased, cut at 60 chars on a word; -2, -3… if another test or the base already holds the path.
  • The quest is a second commit on the disable branch rather than amended into the fixer's commit, so the fixer's work and the template stay separately reviewable.
  • The template links only https://github.com/… job URLs and drops others, since the jobs come from the untrusted round.
  • The watcher also refuses a disable when the base has no quest/m0/README.md, matching the fixer's existing "no quest/m0/, no disable" rule.

Follow-ups

None.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1b7c3-dff8-70a1-b2ba-e85387f8ef40, round 1
  • Head: dad0aa0
  • Base: origin/main at 7f17c66
  • Score: 9/10, No actionable defects were found; implementation and coverage appear consistent, though builds and tests were not run as instructed.
  • Findings: none

@pullfrog

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

quest land watch picks each flaky test's follow-up quest path at its first
verdict and lists it in the fixer's known-flaky input for the skip to name.
The fixer's disable commit now holds only the skip: the watcher refuses one
that touches quest/, then commits the quest from a fixed template on top,
with the flaky epic's README when the base has none. No headless agent
writes quest prose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title The watcher writes flaky quests feat(land): the watcher writes flaky tests' follow-up quests Nov 7, 2026
@kixelated
kixelated marked this pull request as ready for review November 7, 2026 20:47
@kixelated
kixelated added this pull request to the merge queue Nov 7, 2026

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed dad0aa0 against base 7f17c66.

No actionable findings in the inspected changes. The watcher assigns and persists a collision-checked quest path, includes it in known-flaky input, rejects disable commits that modify quest/, and adds the fixed-template quest in a separate commit with an isolated Git index. The existing protected-path/base validation remains before that addition. Directory-based child discovery in the current document model supports creating the flaky epic without editing Required lists.

The updated end-to-end test distinguishes the fixer's skip commit from the watcher's quest commit, covers protected-path and quest edits being refused, checks the generated job link and epic, and retains restart/idempotence assertions. Template tests cover name normalization, length limiting, duplicate links and malformed/non-GitHub URLs.

Direction: retain the template-only ownership boundary and run the watcher disable/restart tests plus quest-tree validation on the final integration head. No code change requested from this review.

Verification limits: GitHub-only static inspection of the changed source, surrounding validation/Git helpers, document child discovery and tests. I did not execute tests, validate live GitHub enqueue behavior, or independently reproduce reported passes. COMMENT only, not approval.

(Written by review (OpenAI))

@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The watcher now assigns a follow-up quest path to a flaky test and supplies it to the fixer. The fixer instructions require a skip-only commit and prohibit edits under quest/. After validating the disable commit, the watcher adds the generated quest and creates the flaky epic README if it is absent. Tests cover quest naming, generated content, validation, and the resulting follow-up pull request.

Merge Risk: Low · up to dad0a

A disable PR can contain a skip that points to the wrong follow-up quest. Maintainer review offers a workaround, so this is a bounded risk rather than a merge blocker.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title "feat(land): the watcher writes flaky tests' follow-up quests" matches the main change. The changeset adds watcher-generated follow-up quests for flaky tests (flakyName, flakyQuest, FLAKY_EP…
Description check Passed The description states the purpose and scope of the change. It covers the watcher selecting the quest path at the first verdict, the fixer's disable commit holding only the skip, the watcher's separat…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "feat(land): the watcher writes flaky tests' follow-up quests" matches the main change. The changeset adds watcher-generated follow-up quests for flaky tests (flakyName, flakyQuest, FLAKY_EPIC, and the disable flow in src/cli/land-watch-flaky.ts), and removes the fixer's quest-writing duties from assets/land-fix.md. The title is specific and concise, and it follows a conventional commit style.

Full details: Description check

Explanation

The description states the purpose and scope of the change. It covers the watcher selecting the quest path at the first verdict, the fixer's disable commit holding only the skip, the watcher's separate quest commit from a fixed template, the update to assets/land-fix.md, and the verification steps. These match the file-level changes in the summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 4 files. (2 skipped: 2 unsupported.)

Finishing Touches

Simplify code

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b9ffb9fb-1d72-4e55-a9e2-84ef5409e79a
Commits

Reviewing files that changed from the base of the PR and between 7f17c66 and dad0aa0.

Files selected for processing: 9

  • README.md
  • assets/land-fix.md
  • quest/m0/queue-watcher/README.md
  • quest/m0/queue-watcher/flaky-quest.md
  • quest/m0/token-waste/child-lists.md
  • src/cli/land-fix.ts
  • src/cli/land-watch-flaky.ts
  • test/land-watch.test.ts
  • test/land.test.ts
Files with no reviewable changes: 3

  • quest/m0/queue-watcher/README.md
  • quest/m0/token-waste/child-lists.md
  • quest/m0/queue-watcher/flaky-quest.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 7, 2026
@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at dad0aa0 because its merge_group build failed (failed_checks).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

Queue / Queue build / Build (job) failure at step Test:

…    �[33m�[2m✓�[22m�[39m refuses a resolution whose tree the rebuild does not reach�[33m 541�[2mms�[22m�[39m

�[31m⎯⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Tests 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m

�[41m�[1m FAIL �[22m�[49m test/gate.test.ts�[2m > �[22mconcurrent processes never admit past the headroom, and admit every caller
�[31m�[1mAssertionError�[22m: expected [ +0, +0, +0, +0, +0, 1, +0, +0, …(4) ] to deeply equal [ Array(12) ]�[39m

�[32m- Expected�[39m
�[31m+ Received�[39m

�[33m@@ -2,11 +2,11 @@�[39m
�[2m    0,�[22m
�[2m    0,�[22m
�[2m    0,�[22m
�[2m    0,�[22m
�[2m    0,�[22m
�[32m-   0,�[39m
�[31m+   1,�[39m
�[2m    0,�[22m
�[2m    0,�[22m
�[2m    0,�[22m
�[2m    0,�[22m
�[2m    0,�[22m

�[36m �[2m❯�[22m test/gate.test.ts:�[2m505:42�[22m�[39m
    �[90m503|�[39m  renameSync(`${go}.tmp`, go);
    �[90m504|�[39m  const results = await Promise.all(callers.map(({ exited }) => exited)…
    �[90m505|�[39m  expect(results.map(({ code }) => code)).toEqual(Array(12).fill(0));
    �[90m   |�[39m                                          �[31m^�[39m
    �[90m506|�[39m  const admissions: { id: string; at: number }[] = results
    �[90m507|�[39m   .map(({ stdout }) => JSON.parse(stdout.split("\n")[1]))

�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m


�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m29 passed�[39m�[22m�[90m (30)�[39m
�[2m      Tests �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m1108 passed�[39m�[22m�[2m | �[22m�[33m5 skipped�[39m�[90m (1114)�[39m
�[2m   Start at �[22m 13:25:23
�[2m   Duration �[22m 154.59s�[2m (tests 76%, transform 21%, import 3%)�[22m


##[error]AssertionError: expected [ +0, +0, +0, +0, +0, 1, +0, +0, …(4) ] to deeply equal [ Array(12) ]

- Expected
+ Received

@@ -2,11 +2,11 @@
    0,
    0,
    0,
    0,
    0,
-   0,
+   1,
    0,
    0,
    0,
    0,
    0,

 ❯ test/gate.test.ts:505:42


error: script "test" exited with code 1
error: recipe `_test` failed on line 33 with exit code 1
error: recipe `test` failed on line 30 with exit code 1
##[error]Process completed with exit code 1.
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/49b07181-062d-4cea-9db9-ba4aa51f71f8' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/quest/quest
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/fa444447-bd50-465e-a9bc-64962ad56ff2' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/quest/quest
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 0 objects and 0 action results (14.8 MiB logical); 50.0 GiB logical remain
dropped 7 stale checkout records
dropped 7 session event streams
Cleaning up orphan processes

Queue / Queue (job) failure at step Require the build to pass:

Current runner version: '2.338.2'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20261028.624
Commit: 54898b74c23fde5556c9b3c3a2290865feef0dfe
Build Date: 2026-10-28T21:51:42Z
Worker ID: {e66633ff-ad20-4e58-9c72-ab7c041628d1}
Region: eastus
Cloud: Azure
##[endgroup]
##[group]Operating System
Ubuntu
24.04.5
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20261101.360.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20261101.360/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20261101.360
##[endgroup]
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
PullRequests: read
##[endgroup]
Secret source: Actions
Cache mode: write
Prepare workflow directory
Prepare all required actions
Complete job name: Queue
##[group]Run [[ "$RESULT" == success ]] || { echo "::error::the queue build ended $RESULT" >&2; exit 1; }
�[36;1m[[ "$RESULT" == success ]] || { echo "::error::the queue build ended $RESULT" >&2; exit 1; }�[0m
shell: /usr/bin/bash -e {0}
env:
  RESULT: failure
##[endgroup]
##[error]the queue build ended failure
##[error]Process completed with exit code 1.
Cleaning up orphan processes

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 7, 2026
@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

codex, gpt-6.1-sol, medium effort land fixer note:

The queue failure reproduces on unchanged base 455ebea: the gate concurrency test exits a caller with ESRCH: no such process, read in startTime (src/cli/supervise.ts:36), called by processStart and running. A process can exit between its liveness check and the /proc/<pid>/stat read; processStart currently catches only ENOENT. The same failure occurred on the twelfth isolated run on base. PR #551 changes neither this test nor that code. No blame assigned and no source changes made. The unchanged pinned PR head merged with base on a detached scratch checkout passed just install, just check, just test (1,109 CLI tests and 539 Worker tests), and nix flake check --no-update-lock-file with substitution disabled in an isolated network namespace. The base race needs a separate reviewed behavior fix.

(written by codex, gpt-6.1-sol, medium effort)

@moq-bot

moq-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

quest land watch: re-enqueued this pull request at dad0aa0, at the back of the merge queue, unchanged, since the land fixer found the failure on the base too: test/gate.test.ts > concurrent processes never admit past the headroom, and admit every caller.

(written by quest land watch)

Merged via the queue into main with commit 2108b1d Nov 7, 2026
22 checks passed
@moq-bot
moq-bot Bot deleted the quest/m0/queue-watcher/flaky-quest branch November 7, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant