docs(linear): add pre-flight check, post-run checklist, and production tips#160
docs(linear): add pre-flight check, post-run checklist, and production tips#160pawelkrystkiewicz wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Vanguard Review
This is a documentation-only change to docs/onboarding-linear.md. I verified the diff against the current file and the repo layout (src/ exists, so the §0 grep example is accurate). Markdown is well-formed, section numbering (0–5) has no collisions, and no Modelbox-specific content was introduced.
One actionable finding:
Finding: Single-pass variant contradicts the triage contract (§2 vs §3)
The new Single-pass variant note (§2) tells users to apply the vanguard label to issues "already in the agent state (Todo/unstarted) with a complete spec" and says "the watcher builds them directly."
But §3 (The triage contract) — which the diff leaves intact — states the agent pass still refuses an under-specified ticket and that to pass, a ticket in the agent state needs one of:
- a
## Acceptance Criteriamarkdown heading followed by at least one real bullet, or - a Vanguard
<tech_spec>comment (written by the spec pass).
A reader following the single-pass advice with a free-form "complete spec" in the description that lacks the literal ## Acceptance Criteria heading will have the ticket bounced to Needs Info by the agent pass — the exact wasted-budget outcome this doc is trying to prevent. The phrase "complete spec" understates the hard requirement.
Suggested fix: make the requirement explicit, e.g. "…with a ## Acceptance Criteria heading and real bullets (so it satisfies the triage contract)" rather than the looser "complete spec."
This is the only finding worth the author's attention; everything else in the diff is accurate and consistent. I'm reporting it rather than withholding it because the imprecision can cause the failure mode the PR aims to avoid — but it is a low-severity doc-consistency issue, not a code defect.
…n tips Add six practical additions to docs/onboarding-linear.md from production use: - §0 Pre-flight check: grep codebase before dispatching to avoid empty-diff runs - §1 Disk management: docker builder prune -f before long-running watcher hosts - §2 Single-pass variant: skip spec pass when team writes detailed specs upfront - §3 Quality calibration: keep reference issues as "ready to run" examples - Top Non-GitHub VCS note: GitLab MR flow via glab, watcher is VCS-agnostic - §5 Post-run checklist: diff stat, verify gate, auth/security spot-check, open review, close issue Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Review noted 'complete spec' understated the hard requirement: a ticket in the agent state still needs a literal `## Acceptance Criteria` heading with real bullets, or the agent pass bounces it to Needs Info. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fdeea2f to
efe9197
Compare
There was a problem hiding this comment.
Vanguard Review
I've verified the diff against the code. Confirmed: the agent-pass triage gate is a deterministic, no-LLM check (assessTaskReadiness, src/tasks/triage.ts:96-97), and triageAgentRun in src/runners/watch.ts:309-313 returns before invoking the runner — the code comments explicitly state "no implement budget spent" (lines 294-295, 312). This contradicts two new claims in the PR.
Findings
1. (Blocking) §3 "Quality calibration" — "Vague tickets cost a full run budget to reject" is factually wrong
Quality calibration: … Vague tickets cost a full run budget to reject.
This contradicts both the existing doc and the code:
- The existing §3 states the gate runs "before spending model budget the agent pass refuses an under-specified ticket."
assessTaskReadinessis documented as a "Deterministic (no LLM) gate" (src/tasks/triage.ts:96).triageAgentRunrejects vague tickets and returns{}before calling the runner — the inline comment reads "no PR -> watchOnce categorises as noChange, no implement budget spent" (src/runners/watch.ts:312).
A vague ticket costs essentially zero model budget to reject, not "a full run budget." Suggest rewording to something like "Vague tickets get bounced to Needs Info, costing you a round-trip of human clarification" — the cost is latency/human effort, not model budget.
2. (Blocking) §2 "Single-pass variant" — the "wasted-budget outcome" parenthetical is wrong for the same reason
(A free-form "complete spec" in the description that lacks the literal heading is bounced to Needs Info by the agent pass, the exact wasted-budget outcome this skips.)
The structural fact is correct — hasRealAcceptanceCriteria requires the literal ## Acceptance Criteria heading (or a <tech_spec> comment), so a free-form spec without that heading is bounced (src/tasks/triage.ts:41-82, :118-128). But the bounce is the deterministic, no-budget path, so calling it "the exact wasted-budget outcome this skips" misstates the cost. The thing actually skipped is the spec pass, not a wasted implement budget. Suggest dropping the "wasted-budget" framing and keeping just the structural caution.
Non-blocking note
- §2's claim that the literal
## Acceptance Criteriaheading is required is accurate, but note the gate also accepts a real-criteria heading inside a comment (src/tasks/triage.ts:123), not only the description — minor, optional to clarify.
The anchor link #3-the-triage-contract-same-as-github, the §0 grep example, and the non-GitHub VCS / post-run checklist sections are all consistent and correct.
Review (round 2) flagged two factually-wrong cost claims: the agent-pass
triage gate is a deterministic no-LLM check (assessTaskReadiness) that
returns {} before invoking the runner (no implement budget spent).
- §2 single-pass: drop the 'wasted-budget outcome' framing; a free-form
spec without the literal ## Acceptance Criteria heading is bounced for
free, the skipped cost is the spec pass, not an implement budget.
- §3 quality calibration: 'vague tickets cost a full run budget' was
wrong; the bounce is deterministic (no model budget), the real cost is
a human clarification round-trip.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Vanguard Review
Review: docs(linear) onboarding additions
I verified each new claim against the codebase. One finding is blocking because the diff documents behavior the code does not implement; the rest of the additions check out.
Blocking
1. The "Non-GitHub VCS" note describes a Linear→GitLab MR flow that does not exist in the code.
The added sentence (docs/onboarding-linear.md, end of the intro paragraph) reads:
Non-GitHub VCS: GitLab workspaces open MRs instead (
glab mr create --fill); the Linear issue receives the MR link the same way. Any VCS with a CLI that can open a review request works — the watcher is not GitHub-specific.
This doc covers the Linear source. The Linear run path is runLinearIssue → runSourcedIssue (src/runners/source-adapter.ts:144), which calls:
const pr = await publishForReview(ctx, { title: ..., body, draft: true });No cli option is passed, and publishForReview defaults to gh (src/pipeline/pipeline.ts:731: const tool = opts.cli ?? 'gh'). So a Linear-sourced run always opens a GitHub PR — there is no flag or code path that makes a Linear issue open a GitLab MR. The cli: 'glab' branch is used only by runGitlabIssue (src/runners/gitlab.ts:123), i.e. issues sourced from GitLab, which is a different source entirely (not Linear). The claim "GitLab workspaces open MRs instead; the Linear issue receives the MR link the same way" is therefore false as written.
2. The documented glab mr create --fill command is not what the tool runs.
publishForReview builds the glab invocation explicitly (src/pipeline/pipeline.ts:736-744):
args = ['mr', 'create',
'--source-branch', ctx.branch,
'--target-branch', opts.baseBranch ?? 'main',
'--title', opts.title,
'--description', opts.body ?? '',
];
if (opts.draft === true) args.push('--draft');There is no --fill. In fact --fill (auto-populate title/description from commits/branch) would conflict with the explicit --title/--description the code passes. The documented command is both not the actual invocation and not a usage the framework exposes.
3. "Any VCS with a CLI … works — the watcher is not GitHub-specific" overstates support.
publishForReview hardcodes a two-way branch: glab or else gh. Only GitHub and GitLab are implemented; there is no generic VCS abstraction. Bitbucket/etc. are not supported.
Suggested fix: Either drop this note, or rewrite it to match reality — GitLab is supported as a task source (vanguard watch --source gitlab / runGitlabIssue) where the MR opens via the explicit glab mr create --source-branch … --target-branch … --draft form, not as an alternate review surface for Linear-sourced issues. If a Linear→GitLab combination is genuinely intended, that's a feature gap, not a doc, and should not be presented as existing behavior.
Non-blocking / verified clean
- §0 Pre-flight, §1 disk management (
docker builder prune -f), §3 quality calibration, §5 post-run checklist — generic, accurate, no false claims about framework internals. - §2 single-pass variant — correctly matches the triage contract in §3 (literal
## Acceptance Criteriaheading + real bullets, or a<tech_spec>comment); the "free-form spec lacking the literal heading is bounced" caveat is consistent with the documented contract. - Anchor link
#3-the-triage-contract-same-as-githubcorrectly resolves to the## 3. The triage contract (same as GitHub)heading (GitHub strips parens, lowercases, hyphenates). - No Modelbox-specific content (LiteLLM/bun/Prisma) introduced — passes the PR's own test-plan item.
CI review (round 3) caught a blocking false claim: the intro said 'GitLab workspaces open MRs instead (glab mr create --fill); the Linear issue receives the MR link the same way.' Untrue for Linear-sourced runs. runLinearIssue -> runSourcedIssue -> publishForReview is called with no cli option, which defaults to 'gh' (pipeline.ts:732) -> always a GitHub PR. cli: 'glab' is set only by runGitlabIssue (gitlab.ts:123), i.e. GitLab-*sourced* issues, a different source. There is no Linear->GitLab path, and the actual glab invocation uses explicit --source-branch/ --target-branch/--title/--description, never --fill. Dropped the sentence; the doc is Linear-specific and Linear opens GitHub PRs (already stated in the same paragraph). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Second-opinion review (pi / glm-5.2, 3 perspectives) caught: - §5 'draft PR/MR' -> 'draft PR'. The Linear flow always opens a GitHub PR (publishForReview defaults to gh); '/MR' was the residual of the removed GitLab over-claim. - §5 item 2: stop telling the reader to run the verify gate. Vanguard auto-runs it (resolveVerifyCommand -> runVerification) and writes the '## Proof of work' block into the PR body + a vanguard:verify-failed label. Point at that attestation instead. - §5 item 4: drop 'post the link as a comment' — adapter.linkPr already comments the PR link onto the Linear issue (linkLinearIssue). - §2: drop the '<tech_spec> comment' alternative (the spec pass is its only producer, and single-pass skips the spec pass), and reword '(Todo/unstarted)' to '(Todo, of type unstarted)' to match the doc's own state-name vs state-type distinction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
docker builder prune -fon long-running watcher hosts prevents silent mid-run out-of-space failuresglab mr create --fill; watcher is VCS-agnosticTest plan
🤖 Generated with Claude Code