Skip to content

feat(work-items): container-scoped frontier and sub-item enumeration verb#709

Merged
kyle-sexton merged 3 commits into
mainfrom
fix/498-work-items-container-frontier
Jul 20, 2026
Merged

feat(work-items): container-scoped frontier and sub-item enumeration verb#709
kyle-sexton merged 3 commits into
mainfrom
fix/498-work-items-container-frontier

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

The work-item-tracker seam reserves work-map containers as a first-class use case, but its read side was repo-global only — there was no way to operate one container within scope (#498). Concretely: list-frontier had no container scoping, its rows drop parent linkage (GitHub's list surface omits it), an unassigned/unblocked container surfaced as its own frontier item, and no verb enumerated a container's children (needed for a closed-children invariant check). Surfaced by the #416 wayfind-routing planning pass.

Fix

  • list-sub-items <parent-id> [--state open|closed|all] — new seam + adapter verb. Returns a container's direct children as full normalized item objects (same {items:[…]} envelope as list-items), each re-parented to the container. Raw enumeration — closed and nested-container children are kept (the closed-children invariant check and sub-map traversal need them). --state defaults to all. Both adapters implement it: GitHub resolves children via the native subIssues link and intersects with list-items output (its list surface omits parent linkage, so the child set can't be derived from rows); the offline local-markdown adapter matches on the stored parent frontmatter.
  • list-frontier --parent <container-id> — container-scoped frontier. Core reads list-sub-items for that container instead of the repo-global list-items, then applies the identical filter. The scoped verb is chosen before the capability gate, so an adapter with list-items but not list-sub-items degrades explicitly (exit 6) rather than passing the gate then failing mid-call.
  • Container exclusion (obs feat: markdown-formatter plugin (pilot migration) #3). list-frontier now excludes any item carrying the container label (work-map) unconditionally — global and --parent-scoped, and under --autonomous. Justification: the CONTRACT already states containers are never claimable by workers, so this is a correctness fix, not a policy toggle — deliberately not behind a flag.

Design notes for the reviewer:

  • The container label is a named constant (WIT_CONTAINER_LABEL, default work-map) matching the CONTRACT term — not a new binding key. Per-repo remapping is deferred to the existing config.role_labels convention, keyed off the first consumer that needs a non-work-map marker.
  • Obs fix: correct plugin-system inaccuracies and harden references #2 (global rows drop parent linkage) is addressed by the --parent scoping path, not by enriching global list-items rows — GitHub's bulk list surface genuinely omits parent, and get-item remains authoritative for a single item's parentage (unchanged CONTRACT stance).
  • Container exclusion lives only in the core frontier filter; list-sub-items stays a raw enumeration (nested sub-maps and closed children must survive it).

Version 0.15.00.16.0 (additive); CHANGELOG entry added.

Verification

All commands run in the worktree on the rebased branch (tip of origin/main + this commit).

New behavioral test — local-markdown list-sub-items end-to-end through the core CLI (offline, CI-runnable):

$ bash plugins/work-items/tools/work-item-tracker/adapters/local-markdown/list-sub-items.test.sh
PASS: [1] list-sub-items holds open child C1
PASS: [2] list-sub-items holds closed child C2
PASS: [3] list-sub-items excludes the container itself
PASS: [4] list-sub-items excludes an unrelated leaf
PASS: [5] list-sub-items excludes a grandchild (direct children only)
PASS: [6] list-sub-items (default all) child count
PASS: [7] children carry parent_id
PASS: [8] list-sub-items --state open keeps only C1
PASS: [9] list-sub-items --state closed keeps only C2
PASS: [10] list-sub-items on a leaf is empty
PASS: [11] scoped frontier holds the open child C1
PASS: [12] scoped frontier drops the closed child C2
PASS: [13] scoped frontier drops the grandchild (not a direct child)
PASS: [14] scoped frontier never holds the container
PASS: [15] global frontier excludes the unassigned/unblocked container
PASS: [16] global frontier still holds the workable leaf

Core frontier filter unit test (container exclusion added):

$ bash plugins/work-items/tools/work-item-tracker/lib/frontier.test.sh
PASS: [7] default frontier excludes the container (work-map)
PASS: [8] autonomous frontier also excludes the container
PASS: [9] explicit container label excludes the remap, not the stale default
... (9 cases, 0 failed)

Abstract conformance suite through the core CLI (exercises list-sub-items + list-frontier --parent for the adapter — CI runs this offline binding):

$ bash plugins/work-items/tools/work-item-tracker/conformance/run-conformance.sh --binding local-markdown
PASS: [43] list-sub-items A (exit code)
PASS: [46] A's children hold B
PASS: [47] enumerated child B is re-parented to A
PASS: [50] list-frontier --parent A (exit code)
PASS: [53] scoped frontier drops blocked child B
Conformance (local-markdown): 72 cases, 0 failed

Full work-items plugin test suite + shellcheck (0.11.0, repo .shellcheckrc) over all changed/new shell files — both clean:

$ find plugins/work-items -name '*.test.sh' | while read t; do bash "$t"; done   # all pass, 0 failures
$ shellcheck -x <changed .sh + .test.sh>                                          # rc=0, no findings
$ bash scripts/check-changelog-parity.sh --check-bump origin/main                 # Every changed plugin has its CHANGELOG entry.

The GitHub subIssues+intersect path runs only under the on-demand e2e-probe (needs a live sandbox repo), which was not executed here; its subIssues node shape (id/number/title/url/state/repository.nameWithOwner — no assignees/labels/blockedBy, hence the intersect-with-list-items design) was verified against gh's own source (cli/cli api/queries_issue.go, LinkedIssue struct). e2e-probe was extended with container-exclusion and list-sub-items/--parent assertions for when it is next run against a sandbox.

Closes #498

Related

…verb

The work-item-tracker seam reserves work-map containers as a first-class use
case but had no read-side to operate one within scope (#498): the frontier was
repo-global only, an unassigned/unblocked container surfaced as its own frontier
item, and no verb enumerated a container's children.

- Add list-sub-items <parent-id> [--state] — a new seam + adapter verb returning
  a container's direct children (both adapters). GitHub resolves via native
  subIssues intersected with list-items; local-markdown matches stored parent.
- Add list-frontier --parent <container-id> — a container-scoped frontier.
- Exclude container (work-map) items from every frontier unconditionally.

Co-Authored-By: Claude <noreply@anthropic.com>
@kyle-sexton kyle-sexton added the automated Opened by automation. label Jul 20, 2026
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(work-items): container-scoped frontier and sub-item enumeration verb

  • Gather context and understand the request
  • Read changed files and diff
  • Review core logic (frontier.sh, work-item-tracker.sh)
  • Review new adapters (list-sub-items.sh for both GitHub and local-markdown)
  • Review tests and conformance suite
  • Review CONTRACT.md, CHANGELOG, and plugin.json
  • Post findings

Overall this is a high-quality, well-structured change. The design is clearly reasoned, the test coverage is broad (unit → end-to-end → conformance → e2e-probe), and the CONTRACT and CHANGELOG are kept in sync. A few findings below — one worth fixing before merge, the rest minor.


Finding 1 — --parent + --repo silently discards --repo (medium)

In work-item-tracker.sh L163–170, --repo is accepted and appended to list_args, but then ignored in the scoped path:

# L177-184
if [[ -n "$parent" ]]; then
  out="$(bash "$adapter_dir/list-sub-items.sh" "$parent" --state open)"
else
  out="$(bash "$adapter_dir/list-items.sh" --state open "${list_args[@]+"${list_args[@]}"}")"
fi

A caller passing list-frontier --parent X --repo Y gets no error and no indication that --repo was dropped. The PR description explains the reasoning ("the container's own repo is carried by its qualified id"), but --repo appearing in the usage string for list-frontier while being silently discarded for one code path is a silent footgun.

Options (any of these would be fine):

  • Reject the combination with a usage error in the arg parser: [[ -n "$parent" && ${#list_args[@]} -gt 0 ]] && { usage; exit "$EX_USAGE"; }
  • Emit a warning to stderr
  • Remove --repo from the usage string's --parent form and document the mutual exclusion in CONTRACT.md

Fix this →


Finding 2 — index() used for container-label check (low, jq gotcha)

In frontier.sh L39:

and (((.labels // []) | index($container)) | not)

index/1 on an array returns the first index of the value (an integer ≥ 0) or null. Both a found index of 0 (first element) and null are handled correctly by jq's truthiness rules (0 is truthy, null is falsy), so not gives the right answer. This is correct, but index(0) returning 0 being "truthy" is a commonly misread jq pattern — the existing human_gated check uses the exact same idiom, so this is consistent with the codebase. No change needed, just flagged for awareness.


Finding 3 — --state open in scoped frontier is redundant with the jq filter (very low, cosmetic)

work-item-tracker.sh L182 passes --state open to list-sub-items, and then wit_filter_frontier independently checks .state == "open". This is intentional efficiency (pre-filter before the jq pass) and is consistent with how list-items is called in the global path. No issue — noted for completeness.


Correctness of the --parent pre-scan

The two-pass approach (pre-scan $@ to pick the adapter verb before the capability gate, then re-parse in the arg loop) is correct and the stated reason (explicit exit 6 on missing list-sub-items vs. passing the gate then failing mid-call) is sound. The loop at L126–132 is a clean minimal scan.

GitHub adapter intersect logic

github/list-sub-items.sh L60–67: The split("#")[-1] | tonumber extraction is correct for the github:<owner>/<repo>#<number> ID grammar. The || exit "$?" pattern at L59 correctly propagates list-items.sh failures despite $() swallowing the exit code — well-noted in the comment.

readonly WIT_CONTAINER_LABEL in a sourced script

The _WIT_FRONTIER_LOADED guard at frontier.sh L10 prevents double-readonly errors on re-source. Correct.

Test coverage

The end-to-end test at local-markdown/list-sub-items.test.sh is thorough — it covers direct-child enumeration, state filtering, parent stamping, scoped frontier, and global container exclusion in 16 cases through the real CLI dispatcher. The frontier unit test correctly verifies that a remapped container label excludes the remap (not the stale default), even though the caller path currently always passes WIT_CONTAINER_LABEL. The conformance additions are well-structured and gate correctly on capability presence.


Verdict: Approve pending resolution of Finding 1 (silent --repo drop). Findings 2 and 3 are informational — no action required.

The comment-hygiene lane flagged six comments citing the source issue by
number. Reword each to state the container-frontier invariant it explains
(a container never surfaces as its own frontier item) without the ticket
citation — version control owns the history.

Co-Authored-By: Claude <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 50s —— View job


PR Re-review (after d085dca)

  • Load tools and update comment
  • Inspect new commit vs prior review
  • Read changed files
  • Assess whether Finding 1 was addressed
  • Post updated findings

New commit: d085dca style(work-items): drop issue back-references from tracker comments

This commit is purely cosmetic — four files touched, all changes are comment text only (no logic, no test assertions, no schema). Each rewrite replaces a #498 ticket citation with a self-describing invariant statement. Example:

-# container exclusion from the GLOBAL frontier (issue #498 obs #3)
+# container exclusion from the GLOBAL frontier (a container never surfaces itself)

No behavioral change; no new findings introduced.


Finding 1 (from prior review) — still open

--parent + --repo silently discards --repo — medium severity.

This was not addressed by the new commit. At work-item-tracker.sh L177–184:

if [[ -n "$parent" ]]; then
  # Container-scoped frontier: enumerate the container's children (the
  # container's own repo is carried by its qualified id, so --repo does not
  # re-target here) and apply the same filter …
  out="$(bash "$adapter_dir/list-sub-items.sh" "$parent" --state open)"
else
  out="$(bash "$adapter_dir/list-items.sh" --state open "${list_args[@]+"${list_args[@]}"}")"
fi

list_args (which accumulates --repo entries at L168) is never passed to the scoped path. A caller passing list-frontier --parent X --repo Y silently drops --repo Y with no error and no warning.

The reasoning is sound and documented both in the inline comment and in CONTRACT.md L63–64: "The container is addressed by its qualified id, which carries the repo, so there is no --repo flag." But the usage string at CONTRACT.md L55 still shows --parent and --repo side-by-side with no exclusion note, and the arg parser accepts the combination silently.

Any of these closes the gap:

  1. Reject at parse time — simplest, no surprises:
    if [[ -n "$parent" && ${#list_args[@]} -gt 0 ]]; then
      printf "work-item-tracker: --repo is not valid with --parent (container id carries the repo)\n" >&2
      exit "$EX_USAGE"
    fi
  2. Warn to stderr — softer, allows scripted callers to see the drop:
    if [[ -n "$parent" && ${#list_args[@]} -gt 0 ]]; then
      printf "work-item-tracker: --repo ignored with --parent (container id carries the repo)\n" >&2
    fi
  3. Document the mutual exclusion in CONTRACT.md — least invasive; update L55 usage line and add a sentence under list-frontier describing that --repo is incompatible with --parent.

Fix this →


Findings 2 & 3 from prior review — unchanged (informational only)

Both remain accurate; no action required.


Verdict: Everything in d085dca is correct and clean. Approve pending resolution of Finding 1 (silent --repo drop with --parent).

… dropping it

A container-scoped frontier (list-frontier --parent) is addressed by the
container's qualified id, which already carries the repo, so --repo cannot
re-target it. The scoped path never threaded list_args, so a caller passing both
flags had --repo silently discarded with no error — a silently-wrong-output
class defect.

Reject the combination at parse time (exit 2, usage error) with a clear stderr
message, matching the seam's fail-loud convention for invalid arg combinations.
Document the incompatibility under list-frontier in CONTRACT.md and cover the
guard with a test asserting the non-zero exit and message.

Co-Authored-By: Claude <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 19s —— View job


PR Re-review (after 849bc13)

  • Load tools
  • Inspect new commit vs prior review
  • Read changed files
  • Verify Finding 1 was addressed
  • Post updated findings

New commit: 849bc13 fix(work-items): reject --repo with list-frontier --parent instead of dropping it

Four files changed: work-item-tracker.sh, work-item-tracker.test.sh, CONTRACT.md, CHANGELOG.md. This commit directly targets Finding 1 from prior reviews.


Finding 1 — FIXED

--parent + --repo silently discards --repo — now correctly rejected.

The guard at work-item-tracker.sh L177–184 is exactly right:

if [[ -n "$parent" && ${#list_args[@]} -gt 0 ]]; then
  printf 'work-item-tracker: --repo is not valid with --parent (container id carries the repo)\n' >&2
  exit "$EX_USAGE"
fi

Three things to confirm — all check out:

  1. Placement: the guard is at L177, immediately after the done at L176 that closes the arg-parse loop. Both $parent and $list_args are fully populated before the check fires, so --parent X --repo Y and --repo Y --parent X are both caught.
  2. Exit code: $EX_USAGE (exit 2) — consistent with every other usage error in the dispatcher.
  3. Output channel: message goes to stderr (>&2), not stdout, which is correct for a usage error.

Test coverage at work-item-tracker.test.sh L113–117 asserts both the exit code and the message text — and the fake adapter fixture correctly gains list-sub-items so the capability check passes and the guard is actually reached. The test is well-constructed.

CONTRACT.md update documents the mutual exclusion precisely: --repo is incompatible with --parent: a container is addressed by its qualified id, which already carries the repo, so --repo cannot re-target a container-scoped frontier. Passing both is a usage error (exit 2), not a silent drop. Exactly what was needed.

CHANGELOG entry is present and correctly cross-references #498.


Findings 2 & 3 from prior reviews — unchanged (informational only)

Both remain accurate observations with no action required.


Verdict: All findings from this review cycle are resolved. This PR is clean. ✓

@kyle-sexton
kyle-sexton marked this pull request as ready for review July 20, 2026 17:27
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Post-green review pass complete (rule 8). Bot review classification:

Finding Verdict Action
list-frontier --parent X --repo Y silently drops --repo VALID — classified DEFER-FORBIDDEN (silently-wrong output feeding an irreversible action: this verb drives autonomous claim selection, including in this very lane) Fixed — rejects at parse time (exit 2, usage error), matches this dispatcher's existing fail-loud convention
(2 informational-only findings from prior reviews) No action needed Unchanged, non-blocking

Fixed on-branch, no inline threads to resolve, CI fully green (18/18), 20/20 dispatcher tests passing including 2 new guard cases.

Flipping to ready for review; handing off to the babysit lane for merge.

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 21s —— View job


Final review pass — PR #709

  • Gather context and understand the request
  • Read all changed files at HEAD (849bc13)
  • Review core logic (frontier.sh, work-item-tracker.sh)
  • Review adapter implementations (list-sub-items.sh for GitHub and local-markdown)
  • Review tests and conformance suite
  • Review CONTRACT.md, CHANGELOG, and plugin.json
  • Post final findings

This is a complete, clean review pass over all 16 changed files. Three prior review cycles already identified and resolved the one actionable finding (silent --repo drop); this pass finds nothing new.


Core dispatcher (work-item-tracker.sh)

The two-pass design is sound. The pre-scan at L126–132 is a minimal loop that sets adapter_verb="list-sub-items" before the capability gate — exactly the right place, and the contract stated reason (degrade explicitly via exit 6, not pass the gate then fail mid-call) is correct.

The guard at L177–184:

if [[ -n "$parent" && ${#list_args[@]} -gt 0 ]]; then
  printf 'work-item-tracker: --repo is not valid with --parent (container id carries the repo)\n' >&2
  exit "$EX_USAGE"
fi

— placement is correct (after the done that closes the parse loop, so both $parent and $list_args are fully populated); exit code is $EX_USAGE (2), consistent with every other usage error; message goes to stderr. All three axes correct.

Exit-code propagation from the scoped call (rc=$? / if ((rc != 0)); then exit "$rc"; fi) is correct.

The --state open pre-filter in the scoped path (L190) is an intentional efficiency step before the jq pass. Consistent with the global path.

frontier.sh

index($container) | not is correct jq. index/1 on an array returns the index (≥ 0, truthy) or null (falsy) — a match at position 0 is still excluded, which is the right behavior. The same idiom is used for the human_gated check. The _WIT_FRONTIER_LOADED re-source guard correctly prevents double-readonly errors.

WIT_CONTAINER_LABEL is a named constant passed explicitly to wit_filter_frontier at the callsite — the third positional parameter — so a future per-repo remap requires no change to the filter itself.

GitHub list-sub-items.sh

The subIssues + intersect design is correctly justified: GitHub's bulk list surface omits parent_id, so children can't be derived from list rows. Same-repo guard (select(.repository.nameWithOwner == $repo)) correctly scopes out cross-repo sub-issues (documented truncation, not an error).

number-keyed intersect (split("#")[-1] | tonumber) is correct for the github:<owner>/<repo>#<number> ID grammar.

|| exit "$?" on line 59 is correct: var="$(cmd)" || exit "$?" correctly propagates the subshell's exit code — the assignment inherits cmd's exit status, and || exit "$?" fires when it's nonzero.

Early-return on empty child set (L51–54) avoids the list-items call entirely for leaf nodes — a clean efficiency step.

Local-markdown list-sub-items.sh

parent_quoted="$(jq -cn --arg p "$id" '$p')" produces the JSON-quoted form "<id>", and wit_fm_field returns the same format from frontmatter — the comparison is correct. State filtering is validated at parse time with a clearly-unreachable *) ;; branch (the comment is accurate). wit_emit_local_item "$n" true correctly stamps parent_id.

Test coverage

frontier.test.sh — 9 cases; the container-exclusion cases cover default label, autonomous mode, and explicit remap (stale default no longer excludes). The remap case is the hardest to reason about and it's tested correctly.

work-item-tracker.test.sh — the fake adapter at L44–47 correctly adds list-sub-items: true so the capability gate passes and the --parent + --repo guard is actually reached. Both exit code (2) and message text are asserted.

local-markdown/list-sub-items.test.sh — 16 cases through the real CLI: direct children, state filtering, parent stamping, scoped frontier, global container exclusion. Covers the full behavioral surface.

conformance/run-conformance.sh — new cases gated correctly on verb_supported list-sub-items && verb_supported add-sub-item (the test depends on add-sub-item to set up the parent relationship). The unsupported-verb degradation cases (exit 6 for both list-sub-items and list-frontier --parent) are present.

CONTRACT.md

--repo incompatibility is documented precisely: "Passing both is a usage error (exit 2), not a silent drop." The list-sub-items verb, --parent scoping, and container exclusion are all documented. The Containers section is clear on the invariant and its named-constant form.

CHANGELOG / plugin.json

Version bump 0.15.0 → 0.16.0 is correct (additive). Both the Added and Fixed sections are present. The --repo rejection fix is described accurately as the fail-loud resolution of a silent-drop bug.


Verdict: approve. All findings from prior review cycles are resolved. No new findings. This PR is clean and ready to merge.

@kyle-sexton
kyle-sexton merged commit cb3e6c0 into main Jul 20, 2026
20 checks passed
@kyle-sexton
kyle-sexton deleted the fix/498-work-items-container-frontier branch July 20, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Opened by automation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

work-items: tracker seam has no container-scoped frontier or child-enumeration verb — containers surface themselves on the global frontier

1 participant