feat(work-items): add work-loop invocation argument surface

[kyle-sexton]

Fixes #1291

Summary

Gives /work-items:work-loop a minimal invocation argument surface mirroring babysit-loop where appropriate:

[<owner/repo>] [--drain] [--shard <i>/<n>] [--ordering oldest-first|newest-first] [--instance <id>] [--scope <label>]

Rejects --merge / babysit tier / cap knobs fail-closed. Progressive disclosure moves stop-mode exit semantics into reference/mode-standing.md and reference/mode-drain.md.

Bumps work-items to 0.35.17 (0.35.16 reserved for #1290 attend-queue claim).

Test plan

• Evals 6–8 cover shard/ordering/rejected-merge, drain vs standing, scope partitioning
• Eval 3 updated to pass --drain where drain-terminal stop is expected

Related

• work-items(work-loop): no argument surface — scope, ordering, and mode live in prose the skill never reads #1291
• Unblocked by work-items(work-loop): fixed telemetry title and sentinel make two lanes on one repo clobber durable state, including first_drain_complete #1295 / fix(lanes): key telemetry state per lane instance, not per lane type #1841

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 3m 11s —— View job

---

Security Review — /review:security-review

• Gather context (diff, changed files, CLAUDE.md)
• Run /review:security-review skill criteria against the diff
• Post findings

Reviewed the diff between origin/main and HEAD (066a008). All changed files are Markdown/JSON skill instructions and metadata — no application code, no GitHub Actions workflow changes. Per the skill's scope (logic/trust-boundary flaws the diff introduces, not general quality/docs issues), one finding:

---

IMPORTANT — the fail-closed argument-validation/rejection behavior the PR describes is not actually encoded in the shipped instructions

The PR body claims two safety controls for the new invocation surface:

• optional <owner/repo> — "fail closed on mismatch" against the checkout
• --merge, babysit-tier keywords, and cap knobs are "rejected... never silently ignored"

Neither is present anywhere in SKILL.md. The file:

• still has argument-hint: "(no arguments — cycle behavior comes from the launch prompt's standing rules and persisted config)" at SKILL.md#L3, contradicting the feature this PR adds
• never defines a grammar/parsing section for $ARGUMENTS (SKILL.md#L14) — no instruction tells the executing agent how to derive stop_mode, ordering, shard, scope, or instance from the invocation tokens
• never states the owner/repo checkout-match validation or its fail-closed refusal
• never states the rejection rule for --merge / babysit tier keywords / cap knobs, or the required error message

The only places these controls are described are the PR body and evals/evals.json eval 6 (test fixtures used for eval grading, not instructions an invoking agent reads at runtime). reference/mode-standing.md even references the missing control in passing — "an unrecoverable configuration error (missing binding, rejected argument)" — without anything upstream ever defining what triggers a rejection.

This matters specifically because the missing controls are safety/authorization boundaries, not cosmetic ones:

• The worker lane is contractually barred from merging ("the loop never merges", SKILL.md gotchas). Without an explicit, mandatory reject-on---merge instruction, an operator or upstream automation passing --merge gets undefined behavior instead of a guaranteed refusal — the one place this PR should be tightening that boundary for the new argv surface.
• The owner/repo check is the only guard against the loop mutating the wrong repository when launched with a mismatched target; with no instruction to perform the comparison, a mismatch silently proceeds rather than failing closed.

By contrast, --instance is comparatively safe: it flows into reference/telemetry-upsert.md's existing (pre-existing, unchanged) ^[a-z0-9][a-z0-9-]{0,31}$ validation before shell/jq interpolation, so no injection path was found there — but note even this "override lane_instance userConfig" wiring is asserted only in the PR body, not in SKILL.md.

Suggested fix: add an explicit "Invocation arguments" section to SKILL.md (and update argument-hint) that defines the grammar, the owner/repo fail-closed check, and the exact rejection behavior/message for disallowed tokens — mirroring what babysit-loop does — so the behavior the evals test is actually reachable from the instructions an agent follows.

Fix this →

No other security-relevant issues found — the rest of the diff (telemetry JSON field additions, exit-condition text moved into reference/mode-standing.md / reference/mode-drain.md, CHANGELOG.md, version bump) is a faithful refactor of existing, already-reviewed logic with no new trust-boundary or injection surface.
· Branch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 066a008091

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Last security-reviewed head: a74c310a2334c6df6d9d0a23480cf2eb0f370c45. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @cursor[bot]'s task in 2m 56s —— View job

---

Security review complete

• Gather context (PR diff, files changed)
• Skip-gate check (open PR, security-relevant surface present)
• Review changed files for security-relevant issues
• Report findings

Scope: this PR's changed files only (plugin.json, CHANGELOG.md, work-loop/SKILL.md, evals/evals.json, reference/mode-drain.md, reference/mode-standing.md), per gh pr diff.

---

IMPORTANT — the advertised fail-closed argument rejection isn't actually specified in the skill, only in its evals

The PR body claims: "Rejects --merge / babysit tier / cap knobs fail-closed", and the CHANGELOG entry claims the new surface is "enforced by the skill (not prose-only)". Neither is true of the shipped SKILL.md.

• argument-hint at SKILL.md#L3 is unchanged by this PR and still reads "(no arguments — cycle behavior comes from the launch prompt's standing rules and persisted config)".

• The body of SKILL.md only ever refers to the "resolved" --scope/--ordering/--shard/stop_mode as if parsing already happened elsewhere (L203, L217, L93-L95). There is no section anywhere in SKILL.md that:

  • defines the actual grammar ([<owner/repo>] [--drain] [--shard i/n] [--ordering …] [--instance <id>] [--scope <label>]),
  • validates <owner/repo> against the checkout's origin remote,
  • validates --instance against ^[a-z0-9][a-z0-9-]{0,31}$, or
  • rejects --merge, tier keywords (e.g. autopilot), or cap-override tokens.

  All of that behavior exists only as expected_output/expectations prose in evals.json (new eval 6, evals.json#L88-L104) — which grades a transcript, it is not itself loaded as an instruction at invocation time.

  For comparison, the sibling babysit-loop skill this PR says it mirrors specifies this kind of thing explicitly and verbosely (plugins/source-control/skills/babysit-loop/SKILL.md, "Required argument and config resolution" — required-arg enforcement, the single named autopilot + --merge c3-this-run exception, fail-closed defaults, etc.). work-loop/SKILL.md has no equivalent section.

• Why this matters here specifically: this skill's own "Gotchas" reiterate that the worker lane "never merges — and never asks another lane to" (SKILL.md#L444-L445) — this repo's whole design philosophy for these LLM-executed skills is that authority boundaries (merge authority, capability-tier escalation) must be spelled out explicitly and fail-closed, never left to model inference, precisely because prose is the enforcement mechanism. Shipping an invocation surface whose merge/tier-rejection behavior is untested-by-instruction (only tested-by-eval) means a live invocation carrying --merge c3-this-run or an autopilot-style token has no textual directive telling the executing agent to refuse it — the fail-closed guarantee is aspirational, not instructed.

Fix: add the missing "argument surface" section to SKILL.md itself (grammar, owner/repo checkout validation, --instance regex validation, and an explicit fail-closed rejection clause for --merge/tier/cap tokens with the rejection message), and update argument-hint to match. The eval prompts already encode what that section needs to say — it just needs to actually be said in the skill body, not only in the eval fixture.

Fix this →

---

No other security-relevant issues found in this diff. The plugin.json and CHANGELOG.md changes are version/description bumps with no permission or schema widening, and the new reference/mode-drain.md / reference/mode-standing.md files are faithful extractions of existing exit-condition prose (no behavior change beyond the split).
· branch cursor/fix-1291-work-loop-argv-2ae1

[claude]

Claude finished @cursor[bot]'s task in 1m 23s —— View job

---

Security Review — /review:security-review

• Gather context (diff, changed files, CLAUDE.md)
• Run /review:security-review skill criteria against the diff
• Post findings

---

Skip gate hit: this head already has a successful security review that still applies.

The only commit added since the prior review (comment above, run against 066a008) is 5eae4e7 — a pure version bump (plugins/work-items/.claude-plugin/plugin.json 0.35.16 → 0.35.17, CHANGELOG.md), with no change to SKILL.md, reference/mode-standing.md, reference/mode-drain.md, or evals/evals.json. There is no new security-relevant surface for this push, so I re-verified rather than re-litigated the prior finding.

The prior IMPORTANT finding is unresolved at current HEAD (5eae4e7): the fail-closed argument-validation/rejection behavior the PR describes is still not encoded in the shipped instructions.

• argument-hint still reads "(no arguments — cycle behavior comes from the launch prompt's standing rules and persisted config)", contradicting the new argv surface.
• SKILL.md#L14 still just exposes $ARGUMENTS with no grammar/parsing section defining how <owner/repo>, --drain, --shard, --ordering, --instance, and --scope are derived, defaulted, or validated.
• No instruction states the owner/repo checkout-match fail-closed check, or the required rejection of --merge / babysit-tier keywords / cap knobs (--wave-cap, babysit_loop_*) — the one place this PR should be tightening the worker lane's "never merges" boundary for the new surface.
• reference/mode-standing.md still references the missing control in passing ("an unrecoverable configuration error (missing binding, rejected argument)") without anything upstream defining what triggers a rejection.

These controls are described only in the PR body and in evals/evals.json fixtures (test data, not runtime instructions), so the behavior the evals grade is still unreachable from what an invoking agent actually reads. Codex's inline review on this PR (SKILL.md:95) independently flagged the same gap.

Suggested fix (unchanged from prior review): add an explicit "Invocation arguments" section to SKILL.md — grammar, precedence/defaults, the owner/repo fail-closed check, and the exact rejection message for disallowed tokens — mirroring babysit-loop, and update argument-hint to reflect the new surface.

No other security-relevant issues found in this push — the diff since the last review is a non-functional version bump only.

Fix this →
· Branch

[claude]

Claude finished @cursor[bot]'s task in 1m 28s —— View job

---

Security Review — /review:security-review

• Gather context (diff, changed files, CLAUDE.md)
• Run /review:security-review skill criteria against the diff
• Post findings

---

Skip gate hit: this head already has a successful security review that still applies.

Current HEAD (ea3116f) and the previously-reviewed SHA (5eae4e7) are the same commit content under a different hash — git diff 5eae4e7..ea3116f -- plugins/work-items is empty (both are "chore(work-items): bump to 0.35.17 to clear parallel 0.35.16 collision with #1290", same author/timestamp/diffstat), consistent with a rebase rather than a substantive change. There is no new security-relevant surface at this push, so I re-verified rather than re-litigated.

The prior IMPORTANT finding is still unresolved at current HEAD (ea3116f): the fail-closed argument-validation/rejection behavior the PR describes is not encoded in the shipped instructions.

• argument-hint still reads "(no arguments — cycle behavior comes from the launch prompt's standing rules and persisted config)", contradicting the new argv surface this PR adds.
• SKILL.md#L14 still just exposes $ARGUMENTS — no grammar/parsing section defining how <owner/repo>, --drain, --shard, --ordering, --instance, and --scope are derived, defaulted, or validated. Confirmed by grep: SKILL.md contains no occurrence of --merge, owner/repo, fail-closed rejection language, or an "Invocation arguments" / "argument surface" section anywhere in the file.
• No instruction states the owner/repo checkout-match fail-closed check, or the required rejection of --merge / babysit-tier keywords / cap knobs (--wave-cap, babysit_loop_*) — the one place this PR should be tightening the worker lane's "never merges" boundary for the new surface.
• reference/mode-standing.md#L21 still references the missing control in passing — "an unrecoverable configuration error (missing binding, rejected argument)" — without anything upstream defining what triggers a rejection.

These controls are described only in the PR body and in evals/evals.json fixtures (test data graded against a transcript, not runtime instructions an invoking agent reads), so the behavior the evals grade remains unreachable from the instructions themselves. Codex's inline review on this PR (SKILL.md:95) independently flagged the same gap, as did the two prior automated security-review passes on this PR.

Suggested fix (unchanged from prior reviews): add an explicit "Invocation arguments" section to SKILL.md — grammar, precedence/defaults, the owner/repo fail-closed checkout-match check, --instance regex validation, and the exact fail-closed rejection message for --merge / babysit-tier / cap tokens — mirroring plugins/source-control/skills/babysit-loop/SKILL.md's "Required argument and config resolution" section, and update argument-hint to reflect the new surface.

No other security-relevant issues found — the rest of the diff (telemetry JSON field additions, exit-condition text moved into reference/mode-standing.md / reference/mode-drain.md, CHANGELOG.md, version bump) remains a faithful refactor of existing, already-reviewed logic with no new trust-boundary or injection surface.

Fix this →
· Branch

[claude]

Claude finished @cursor[bot]'s task in 6m 47s —— View job

---

Code Review — /review:code-review

• Gather context (diff, changed files, CLAUDE.md, REVIEW.md lane split)
• Run /review:code-review skill criteria against the diff
• Post findings

Reviewed the diff between dd95c78 (verified merge base — aggregate +153/-50 and every changed-file blob SHA match the PR's authoritative file list exactly) and HEAD (ea3116f). Per REVIEW.md's lane split, this repo carries a claude-security-review.yml workflow, so the missing argument-hint update / undefined invocation grammar / unenforced --merge rejection already flagged repeatedly by the security-review lane and by Codex is out of scope here — that's an authorization-boundary gap and belongs exclusively to the security lane, not restated in this comment.

Two non-security correctness findings posted inline:

1. reference/mode-drain.md:26-27 — the ordinary "Satisfied" drain-exit branch lost the post-snapshot intake report that the "Drain-terminal state" section right below it still has. Pre-refactor SKILL.md said "Either stop's report names the intake that arrived after the snapshot" — this split kept that only on the terminal path, so an item landing after the snapshot before an ordinary drain-satisfied exit is now silently dropped from the final report, regressing the "reported, never chased" guarantee for that path.
2. SKILL.md:217 — "filling slots in resolved --ordering order" has no instruction to actually obtain createdAt for admitted candidates. The frontier snapshot (list-frontier) is the normalized frontier model, which the sibling /work-items:work skill documents as omitting createdAt — that skill compensates with an explicit extra adapter "List items" projection call before sorting oldest-first (plugins/work-items/skills/work/SKILL.md:155). work-loop has no equivalent step, so oldest-first/newest-first has no defined data source to sort admitted items by.

No other correctness/maintainability issues found in evals.json, plugin.json, or CHANGELOG.md beyond the already-flagged spec/implementation gap (out of scope for this lane).
· Branch

[claude]

Claude finished @cursor[bot]'s task in 1m 16s —— View job

---

Security Review — /review:security-review

• Gather context (diff, changed files, CLAUDE.md)
• Run /review:security-review skill criteria against the diff at HEAD 448aa47
• Post findings

---

Prior IMPORTANT finding is now resolved. Commit 448aa47 ("address work-loop argv review threads") adds the missing instruction surface that earlier review passes on this PR flagged as absent from SKILL.md:

• argument-hint (SKILL.md#L3) now reflects the real grammar instead of "no arguments".
• A new "Invocation argument surface" section (SKILL.md#L64-L122) defines the grammar, resolution order, and per-flag validation, including:
  • owner/repo fail-closed validation against the checkout's origin remote, hard stop on mismatch (L87-L89) — no shell interpolation of the untrusted token, just a prose comparison instruction.
  • --instance regex validation (^[a-z0-9][a-z0-9-]{0,31}$) before it reaches the telemetry marker (L102-L104), matching the existing reference/telemetry-upsert.md gate.
  • Explicit fail-closed rejection of babysit-tier keywords, --merge …, and adaptive-cap override tokens, naming the owning surface (/source-control:babysit-loop) rather than silently ignoring them (L109-L116) — closing the gap where the worker lane's "never merges" boundary previously had no textual enforcement for this new argv surface.
• Codex's P1 comment (SKILL.md:155, "define the invocation grammar before resolving options") is addressed by the same section plus explicit "parse, validate, default, and reject before telemetry lookup ... or any cycle work" ordering (L70-L72).
• Codex's P2 comment on ordering (missing createdAt fetch) is addressed: the execute step now sorts admitted items on the adapter "List items" createdAt projection with an explicit fallback for missing timestamps (SKILL.md#L276-L283).
• Codex's P2 comment on mode-drain.md (post-snapshot intake reporting only specified for the drain-terminal branch) is addressed: a new "Post-snapshot intake report (every drain exit)" section now applies to both the ordinary drain-completion and drain-terminal exit paths (reference/mode-drain.md#L27-L45).

No new security-relevant issues found in this push. The remaining diff since the last-reviewed SHA is limited to CHANGELOG.md documenting these fixes — consistent with the code change.

No other security-relevant issues found in the PR as a whole — the rest of the diff (telemetry JSON field additions, reference/mode-standing.md extraction, plugin.json version bump) remains a faithful refactor of existing, already-reviewed logic with no new trust-boundary or injection surface.
· Branch

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 5m 44s —— View job

---

Code Review — /review:code-review

• Gather context (diff, changed files, CLAUDE.md, REVIEW.md)
• Run skip-gate check
• Review changed files against criteria
• Post inline findings / summary

Scope: code-review lane (correctness/maintainability only — security scope is owned by the separate claude-security-review lane already running on this PR, per REVIEW.md's split).

Skip gate: not hit. PR is open, non-draft, and this is the first pass from this lane (prior comments on the PR are all from the security-review lane).

Context: the security lane's repeated finding — that the argv surface was documented only in the PR body/evals and never actually specified in SKILL.md — is resolved as of this head (448aa47). The fix(work-items): address work-loop argv review threads commit added the "Invocation argument surface" section, the createdAt-based ordering in step 4, and the "every drain exit" post-snapshot intake report in reference/mode-drain.md, which also independently closes both Codex P1/P2 findings from the earlier review pass.

Two findings from this pass, posted inline:

1. SKILL.md:70-85 — the new section states argument resolution happens "before telemetry lookup, durable-state adoption, or any cycle work," then two sentences later says relaunch defaulting reads persisted values from the durable-state block — a direct contradiction. The numbered "Resolution order" list also never places persisted durable state among its three ranked layers even though it's used as a de facto fourth source.
2. SKILL.md:102-104 — --instance is documented as using "the same gate as reference/telemetry-upsert.md," but that file (untouched by this PR) still describes lane-instance resolution purely as userConfig → hostname fallback, with no mention of --instance at all. An agent following telemetry-upsert.md's literal shell steps has no textual basis for honoring a supplied --instance value — which evals 6 and 8 both depend on.

No other correctness/maintainability issues found — evals.json is well-formed and its new evals (6-8) match the behavior now specified in SKILL.md/the reference files; the --merge/tier-keyword rejection list matches babysit-loop's actual tier vocabulary; the shard formula and mode-standing/mode-drain split are internally consistent; plugin.json/CHANGELOG.md version bump is consistent (0.35.17).

[claude]

Claude finished @cursor[bot]'s task in 3m 11s —— View job

---

Security Review — /review:security-review

• Gather context (diff, changed files, CLAUDE.md)
• Run /review:security-review skill criteria against the diff at HEAD a74c310
• Post findings

---

Reviewed the full PR diff (gh pr diff 2537, base origin/main) against HEAD a74c310a2334c6df6d9d0a23480cf2eb0f370c45. The changed surface is entirely Markdown skill instructions, JSON eval fixtures, plugin.json metadata, and CHANGELOG.md — no application code, no GitHub Actions workflow changes.

Prior IMPORTANT finding (fail-closed argument rejection undocumented) is resolved and remains resolved at this head. The new reference/invocation-argv.md still carries everything earlier passes required:

• argument-hint reflects the real grammar.
• SKILL.md#L64-L69 points to the grammar/validation/resolution/rejection owner and requires parsing before cycle work.
• owner/repo checkout-match validated with a hard stop on mismatch (invocation-argv.md#L316-318) — a prose string comparison, not shell-interpolated, so no injection path.
• --instance is regex-validated (^[a-z0-9][a-z0-9-]{0,31}$) before it reaches the shell/jq-interpolated telemetry marker, and reference/telemetry-upsert.md — updated in this commit — now correctly names --instance as the first-checked source ahead of userConfig, closing the drift the code-review lane flagged against the pre-a74c310 head. The case "$INSTANCE" in "" | -* | *[!a-z0-9-]*) guard rejects anything that wouldn't already satisfy the regex, so a rejected/malformed id can't reach the double-quoted $SENT/jq --jq string.
• Explicit fail-closed rejection of babysit-tier keywords (safe, worker, autopilot) and --merge … (invocation-argv.md#L338-343), naming /source-control:babysit-loop as the owning surface — matches that skill's actual tier vocabulary and --merge <value> pattern, so the worker lane's "never merges" boundary is textually enforced for the new argv surface. Because the grammar is a closed allowlist and unknown flags are rejected fail-closed, dimension flags outside the explicitly-named set (--strip-do-not-merge, --grace-window-minutes, etc.) are also caught by the generic unknown-flag rejection, not just the named ones.
• --shard/--scope are applied as in-memory id filters only (confirmed via grep — no shell/gh command in SKILL.md or the reference files interpolates either value), so there's no injection surface through those tokens either.

No new security-relevant issues found in the delta this commit adds (invocation-argv.md extraction, telemetry-upsert.md instance-resolution wording, CHANGELOG.md, plugin.json version bump + cosmetic em-dash re-encoding). The two non-security findings the code-review lane raised on the prior head (resolution-order contradiction; telemetry-upsert.md silent on --instance) are both addressed here, and neither was a trust-boundary issue to begin with.

No security-relevant issues found in this PR at current HEAD.

Branch ·

[claude]

Claude finished @cursor[bot]'s task in 2m 19s —— View job

---

Code Review — /review:code-review

• Gather context (PR diff, files changed, prior review threads)
• Invoke /review:code-review skill against HEAD a74c310
• Post inline findings (none — see below)
• Post summary

Scope: code-review lane (correctness/maintainability only — security is owned by the separate claude-security-review lane already running repeatedly on this PR).

Skip gate: not fully hit, but re-verification only — the delta since the last code-review pass (448aa47/83d6a6b → a74c310) is limited to SKILL.md, the new reference/invocation-argv.md, reference/telemetry-upsert.md, and CHANGELOG.md, and it exists specifically to resolve the two P2 findings this lane raised in its previous pass.

Both prior code-review findings are now resolved:

1. Resolution-order contradiction (SKILL.md:66-70). The argument surface was extracted into reference/invocation-argv.md, whose "Resolution order" list now explicitly ranks four layers — invocation arguments, persisted durable state, userConfig, defaults (L12-L23) — and the "before telemetry lookup" clause now correctly scopes to explicit-token parsing only, with durable-state read explicitly sequenced after (L7-L10). No contradiction remains.
2. --instance not honored by reference/telemetry-upsert.md (telemetry-upsert.md:11-23). The file's lane-instance resolution paragraph and the INSTANCE="<lane-instance>" script comment now both name --instance as the first-checked source, ahead of durable state and userConfig — matching invocation-argv.md's resolution order and what evals 6/8 depend on.

Also confirmed still holding from earlier passes, with no regression in this final commit:

• reference/mode-drain.md's "Post-snapshot intake report" now applies to both the ordinary "Satisfied" and "Drain-terminal state" exit branches (mode-drain.md:35-45).
• SKILL.md's execute step sorts admitted items on createdAt from the adapter "List items" projection, with an explicit missing-timestamp fallback, before filling cap slots by --ordering (SKILL.md:224-231).
• The rejected babysit-tier keyword list (safe, worker, autopilot) in invocation-argv.md matches plugins/source-control/skills/babysit-loop/SKILL.md's actual tier vocabulary exactly.
• mode-standing.md's "rejected argument" configuration-error phrase is now a defined term, backed by invocation-argv.md's "Fail-closed rejections" section.
• evals.json evals 6-8 match the behavior now specified in SKILL.md/the reference files; plugin.json/CHANGELOG.md version bump is consistent (0.35.17).

No new correctness/maintainability issues found in this pass. No inline comments needed.
· Branch