feat(orch): reconcile-work-items — the tracker sweep for state written once and never re-read (VST-318) - #1430
Conversation
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
ApprovabilityVerdict: Needs human review Introduces a new You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a read-only “tracker reconciliation” sweep to catch mismatches between tracked issue state and actual work, and wires it into audit/close-out guidance.
Changes:
- Introduces
reconcile-work-itemsscript to flag parked containers, stale started items, and Done items with unchecked acceptance boxes. - Adds an offline bash test pinning expected findings/exit codes for the new script.
- Updates audit/oversee workflows, skill docs, and changelog to incorporate the new sweep.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/project-management/workflows/audit-issues.md | Adds reconcile-work-items to the audit preflight checklist and documents intended behavior. |
| skills/orch/workflows/oversee.md | Updates close-out guidance to run reconcile-work-items before final fleet close. |
| skills/orch/tests/reconcile-work-items.test.sh | Adds offline fixture-based tests validating findings vs healthy cases and exit codes. |
| skills/orch/scripts/reconcile-work-items | New read-only reconciliation script implementing the three checks and exit codes. |
| skills/orch/SKILL.md | Documents the new command and its checks/exit behavior. |
| CHANGELOG.md | Records the new reconciliation sweep and where it’s invoked. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2638677ea1
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Suppressed comments (3)
skills/orch/scripts/reconcile-work-items:43
- The script uses
jqto validate the cache before checking thatjqexists. Ifjqis missing, the user is likely to see the misleading “does not parse as an issue array” error instead of “jq is required”. Move thecommand -v jqcheck before the firstjqinvocation so dependency failures surface the correct message.
CACHE="$REPO_ROOT/.cache/linear/issues.json"
[ -f "$CACHE" ] || config_error "no linear cache at $CACHE — run linear.sh sync first"
jq -e 'type == "array"' "$CACHE" >/dev/null 2>&1 || config_error "linear cache at $CACHE does not parse as an issue array"
command -v jq >/dev/null 2>&1 || config_error "jq is required"
skills/orch/scripts/reconcile-work-items:78
- Invoking
$GH_CLIunquoted allows word-splitting/globbing and makes behavior dependent on shell parsing of an env var; if the variable is ever set unexpectedly, this becomes fragile and can be abused as command injection. Prefer treatingRECONCILE_GH_CLIas an executable path (single token) and invoke it quoted, or parse it into an array once and invoke via an array expansion to safely support optional fixed arguments.
pr_state="unverified"
if command -v "${GH_CLI%% *}" >/dev/null 2>&1; then
merged="$($GH_CLI pr list --state merged --head "$branch" --json number --jq 'length' 2>/dev/null)" || merged=""
open="$($GH_CLI pr list --state open --head "$branch" --json number --jq 'length' 2>/dev/null)" || open=""
skills/orch/scripts/reconcile-work-items:71
- Using
date -dis GNU-date specific and will fail on macOS/BSD environments, which would cause thestarted-stalecheck to silently skip items (sinceupdated_epochbecomes empty). If this skill is expected to run on developer machines, consider using a portable timestamp conversion approach (e.g., via Python) or clearly documenting the GNU date requirement (or usinggdatewhen available).
now_epoch="$(date -u +%s)" || config_error "could not read the clock"
while IFS=$'\t' read -r iid ititle istate iupdated; do
[ -n "$iid" ] || continue
updated_epoch="$(date -u -d "$iupdated" +%s 2>/dev/null)" || updated_epoch=""
[ -n "$updated_epoch" ] || continue
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
skills/orch/scripts/reconcile-work-items:35
RECONCILE_GH_CLIis documented as a 'command', and the code partially supports embedded args via${GH_CLI%% *}, but then executes$GH_CLI ...relying on word-splitting. This is brittle (paths with spaces, quoted args) and hard to reason about. Recommendation: either (a) explicitly constrainRECONCILE_GH_CLIto a single executable path/name and document that, or (b) switch to an argv-safe representation (e.g., array-based) so optional args are handled predictably.
GH_CLI="${RECONCILE_GH_CLI:-gh}"
skills/orch/scripts/reconcile-work-items:88
RECONCILE_GH_CLIis documented as a 'command', and the code partially supports embedded args via${GH_CLI%% *}, but then executes$GH_CLI ...relying on word-splitting. This is brittle (paths with spaces, quoted args) and hard to reason about. Recommendation: either (a) explicitly constrainRECONCILE_GH_CLIto a single executable path/name and document that, or (b) switch to an argv-safe representation (e.g., array-based) so optional args are handled predictably.
if command -v "${GH_CLI%% *}" >/dev/null 2>&1; then
merged="$($GH_CLI pr list --state merged --head "$branch" --json number --jq 'length' 2>/dev/null)" || merged=""
open="$($GH_CLI pr list --state open --head "$branch" --json number --jq 'length' 2>/dev/null)" || open=""
if [ -n "$merged" ] && [ -n "$open" ]; then
skills/orch/tests/reconcile-work-items.test.sh:23
- The test uses
date -d, which is not available on BSD/macOSdateand will fail on those runners. Since the main script already includes a BSD-friendly parsing fallback, the test should also be portable (e.g., use fixed timestamps, or computeoldvia a tool likely present in CI such aspython, or use a POSIX-compatible approach).
now="$(date -u +%Y-%m-%dT%H:%M:%S.000Z)"
old="$(date -u -d '3 days ago' +%Y-%m-%dT%H:%M:%S.000Z)"
skills/project-management/workflows/audit-issues.md:55
- This guard is quite subtle and has a couple of sharp edges: it silently skips the check if the file exists but is not executable (
-x), and$?-based chaining is harder to audit/modify correctly. Consider rewriting to a more explicit pattern that (1) checks for file existence (or runs viabash), and (2) captures and inspects the exit code with a smallrcvariable so the intent (tolerate 1, fail on 2+) is obvious.
[ ! -x .agents/skills/orch/scripts/reconcile-work-items ] || .agents/skills/orch/scripts/reconcile-work-items || [ $? -eq 1 ]
skills/orch/workflows/oversee.md:45
- This bullet is a single very long line, which makes future diffs/reviews harder and tends to wrap poorly in some renderers. Please wrap it across multiple lines (keeping the same bullet) while preserving Markdown formatting.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges, run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 036f51c5d5
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
skills/orch/scripts/reconcile-work-items:33
- The validation pattern rejects
RECONCILE_STALE_HOURS=0even though the error message says “non-negative integer”. Adjust the case patterns to allow0(and optionally still reject leading-zero forms like01if that’s intended).
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/project-management/workflows/audit-issues.md:60
- The parenthetical is a bit ambiguous/grammatically off (“exit 1 is findings”). Consider rewording to “exit 1 means findings (tolerated by the guard above); exit 2 means a broken sweep and fails the preflight” to make the contract clearer for operators.
`reconcile-work-items` (read-only; runs only where the orch skill is
installed — this workflow does not require it; exit 1 is findings and is
tolerated by the guard above, exit 2 is a broken sweep and fails the
preflight)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ac63c2305
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
skills/orch/scripts/reconcile-work-items:50
mktemp -dwithout a template is not portable across BSD/macOS variants (often requires-tor an explicitXXXXXXtemplate). To avoid the script failing on those systems, use an explicit template (optionally respectingTMPDIR) when creating the scratch directory.
TMPD="$(mktemp -d)" || config_error "could not create a scratch directory"
trap 'rm -rf -- "$TMPD"' EXIT
skills/orch/tests/reconcile-work-items.test.sh:10
- Same portability issue as the production script:
mktemp -dwithout a template commonly fails on macOS/BSD. Using an explicit template (and/ormktemp -d -t ...) will make the test suite runnable on those platforms.
TMP="$(mktemp -d)"
trap 'rm -rf "$TMP"' EXIT
skills/project-management/workflows/audit-issues.md:67
- This paragraph has duplicated guidance (the ‘audit decision … failure … prevent’ sentence appears twice) and line 64 starts mid-sentence (‘names tracker rows…’), which makes the workflow instructions hard to follow. Please rewrite this block into a single, non-redundant explanation (what it checks + exit codes + what to do with findings).
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/workflows/oversee.md:45
- This line changes the list formatting from the previous indented form (
|- ...) to a top-level bullet (- ...). If this section is intended to be a nested bullet under a parent list item, the indentation/marker change will break the rendered structure. Restore consistent indentation/marker style with surrounding list items (and consider wrapping the long sentence to keep the markdown maintainable).
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18ddf4728b
ℹ️ 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 (@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 (@codex) address that feedback".
|
Merge queue ejected this PR ( Ejecting merge-group run: not identified Failing job(s): No same-named check comparison available for the PR head. Automated by merge-queue-ejection-alert (VST-196). This alert never re-arms auto-merge. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
skills/project-management/workflows/audit-issues.md:67
- This block has duplicated content and a grammatical break at line 64 ("names tracker rows..." has no subject). Consolidate into a single, non-repetitive paragraph that clearly enumerates the exit codes and checks once; that will make the workflow instructions easier to follow and less error-prone to maintain.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/workflows/oversee.md:45
- This line appears to have changed list indentation compared to the surrounding bullets (previously it looked like a nested list item). If the intent is for these event rules to remain nested under the preceding section, restore consistent indentation so Markdown renders the hierarchy correctly and doesn't flatten or reflow the list unexpectedly.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
skills/orch/scripts/reconcile-work-items:105
- For each started item, this runs two separate
gh pr listcalls. On repos with many stale started items, this can become noticeably slow and can also increase the chance of transient gh/rate-limit failures. Consider reducing it to a single call (e.g., query both states in one request or query once and compute merged/open counts from the same JSON) and reuse the result for the decision logic.
merged="$($GH_CLI pr list --state merged --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || merged=""
open="$($GH_CLI pr list --state open --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || open=""
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
skills/project-management/workflows/audit-issues.md:67
- This paragraph appears to contain duplicated guidance (the “carry findings into the audit … failure this line exists to prevent” sentence repeats) and a sentence fragment at line 64 (“names tracker rows…” is missing a subject, e.g., “It names…”). Consider rewriting this as a single, non-redundant paragraph (or short bullet list) that clearly states: where to run it, exit-code meaning, and the three finding categories.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/workflows/oversee.md:45
- This list item is very long and mixes the primary action with several parenthetical conditions and rationale, which makes the “merged” event handling harder to scan. Consider splitting into (a) a short first sentence for the core action, and (b) an indented sub-bullet (or a following paragraph) describing the Linear-only condition and the rationale for running
reconcile-work-items.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
skills/orch/scripts/reconcile-work-items:108
- The script treats
RECONCILE_GH_CLIas a free-form command string and interpolates it directly into command substitution ($($GH_CLI pr list ...)). This allows whitespace-splitting and can execute unexpected tokens if the env var contains spaces or shell metacharacters. Prefer constrainingRECONCILE_GH_CLIto a single executable path (reject values containing whitespace) and invoke it with proper quoting, or parse into an argv array and execute via that array.
if command -v "${GH_CLI%% *}" >/dev/null 2>&1; then
# This repo's branch only — a fork's same-named branch is someone
# else's PR and must not vouch for this item.
merged="$($GH_CLI pr list --state merged --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || merged=""
open="$($GH_CLI pr list --state open --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || open=""
if [ -n "$merged" ] && [ -n "$open" ]; then
if [ "$open" -gt 0 ]; then
continue # a live PR is a live item, whatever the clock says
elif [ "$merged" -gt 0 ]; then
pr_state="PR merged"
else
pr_state="no PR"
fi
fi
fi
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d37906e15c
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (6)
skills/project-management/workflows/audit-issues.md:67
- These lines contain duplicated guidance (the ‘audit decision…failure’ sentence appears twice) and line 64 starts mid-sentence (‘names tracker rows…’) which reads like a copy/paste artifact. Please collapse this into a single coherent paragraph (e.g., “It names tracker rows…”) and remove the repeated sentence to avoid confusion for workflow users.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/scripts/reconcile-work-items:40
- The validation rejects
0because it matches0*[0-9], but the error message says “non-negative integer,” which includes zero. Either allow0(and optionally still reject leading-zero values like01), or update the validation + error message to accurately describe what is accepted. A robust alternative is to accept any digits and force base-10 when doing numeric comparisons/arithmetic.
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/orch/scripts/reconcile-work-items:56
mktemp -dwithout a template is not portable on macOS/BSD (it typically requires a template or-t). Since the script already includes BSDdatefallbacks, this looks intended to run on macOS too; please switch to a portable mktemp invocation (e.g., try GNU style, then fall back tomktemp -d -t <name>).
TMPD="$(mktemp -d)" || config_error "could not create a scratch directory"
trap 'rm -rf -- "$TMPD"' EXIT
skills/orch/tests/reconcile-work-items.test.sh:10
- Same portability issue as the script:
mktemp -dwithout a template commonly fails on macOS/BSD. Updating the test to use the same portable mktemp pattern will avoid platform-specific CI/local failures.
TMP="$(mktemp -d)"
trap 'rm -rf "$TMP"' EXIT
skills/orch/scripts/reconcile-work-items:13
- The implementation flags stale started items even when the PR probe is verifiable and finds no PR (it reports
pr_state=\"no PR\"). The comment currently says “PR is merged (or unverifiable)” and doesn’t mention the “no PR” case; please update this header comment to include the “no PR/absent PR” behavior so it matches what the script actually reports.
# started-stale an In Progress / In Review item untouched for longer
# than the threshold whose branch-named PR is merged (or
# unverifiable) — the item outlived its work
skills/orch/scripts/reconcile-work-items:116
- This assumes
$mergedand$openare always numeric when non-empty. If the PR probe outputs unexpected text (or partial output),[ \"$open\" -gt 0 ]/[ \"$merged\" -gt 0 ]will error and can lead to inconsistent behavior. Consider explicitly validating both values as^[0-9]+$before doing-gtcomparisons; if validation fails, treat the PR state asunverified(as the script intends for probe failures).
merged="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state merged --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || merged=""
open="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state open --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || open=""
if [ -n "$merged" ] && [ -n "$open" ]; then
if [ "$open" -gt 0 ]; then
continue # a live PR is a live item, whatever the clock says
elif [ "$merged" -gt 0 ]; then
pr_state="PR merged"
else
pr_state="no PR"
fi
fi
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
skills/orch/scripts/reconcile-work-items:41
- The validation pattern rejects
0(and any value with leading zeros), but the error message says “non-negative integer”, which includes0. Either (a) allow0explicitly (and optionally allow/normalize leading zeros), or (b) change the message/semantics to “positive integer” if0should be invalid.
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/project-management/workflows/audit-issues.md:67
- This section appears to have an accidental duplication and a broken sentence start at line 64 (“names tracker rows…”). Please deduplicate/rewrite into a single coherent paragraph (e.g., explain what it checks once, list exit codes once) so the workflow instructions are unambiguous.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/workflows/oversee.md:45
- This list entry is doing a lot in one line, which makes it hard to scan in a runbook context. Consider splitting after the first sentence and moving the “When…run reconcile…” guidance into an indented sub-bullet (or a short follow-up paragraph) to improve readability and reduce the chance operators miss the action.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27f483e8aa
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
skills/project-management/workflows/audit-issues.md:67
- This paragraph appears to have an accidental duplication and a sentence fragment: line 64 starts with “names…” without a subject, and the “audit decision taken against…” sentence is repeated (lines 61–63 and 66–67). Consolidate into a single, grammatically complete explanation to avoid confusion in the workflow instructions.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 100c3e5aa9
ℹ️ 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 (@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 (@codex) address that feedback".
…row — incomplete rows can no longer read as a clean tracker Claude-Session: https://claude.ai/code/session_012epxJEzGqT7q3qcFhdZUt5
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
skills/project-management/workflows/audit-issues.md:68
- This block appears to have duplicated/overlapping sentences (the ‘audit decision taken…’ line is repeated) and a fragment at line 64 (‘names tracker rows…’) that reads like it lost its subject. Recommend rewriting into a single, non-redundant paragraph (or bullet list) that: (1) states when to run it, (2) lists exit codes, and (3) briefly enumerates what it reports.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/workflows/oversee.md:45
- This line changes the list formatting from an indented nested bullet (
|- ...) to a top-level bullet (- ...), which may break the intended structure/visual hierarchy of the rules list in this section. Consider restoring the prior indentation style and wrapping the long sentence across multiple lines to keep the Markdown readable and consistent with surrounding bullets.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20056258f0
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (4)
skills/orch/scripts/reconcile-work-items:40
- The validation pattern rejects
RECONCILE_STALE_HOURS=0even though the error message says “non-negative integer” (0 should be valid). Adjust the check to allow exactly0while still rejecting leading-zero values like01(or update the message/semantics if 0 is intentionally disallowed).
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/project-management/workflows/audit-issues.md:67
- This paragraph is duplicated/fragmented: line 64 starts mid-sentence (“names tracker rows…”) and the rationale sentence repeats. Recommend rewriting as a single, non-redundant paragraph with a clear subject (e.g., “It names tracker rows…”), and keeping the exit-code explanation exactly once.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/scripts/reconcile-work-items:56
- The script later hard-fails if
.updatedAtis missing/unparseable, but the up-front structural validation doesn’t include.updatedAt. Consider validating.updatedAtis a string here as well so the failure mode is earlier and the config error message can point to the missing/invalid field more directly.
jq -e 'type == "array" and all(.[];
type == "object"
and (.identifier | type == "string")
and (.state | type == "object")
and (.state.name | type == "string")
and (.state.type | type == "string"))' "$CACHE" >/dev/null 2>&1 \
|| config_error "linear cache at $CACHE is not an array of issue rows with identifier and state.name/state.type"
skills/orch/workflows/oversee.md:45
- This list item is doing a lot in a single line, which makes it easy to miss the actionable step. Recommend splitting into a primary bullet for the
mergedevent and an indented sub-bullet (or separate sentence/paragraph) that calls out the close-out requirement to runreconcile-work-itemsand what to do with findings.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24933daaa1
ℹ️ 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 (@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 (@codex) address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (5)
skills/project-management/workflows/audit-issues.md:67
- This new paragraph is internally inconsistent: it contains duplicated rationale (lines 61–63 repeated again in 65–67) and line 64 starts mid-sentence (“names tracker rows…”) without a subject. Recommend deduplicating and rewriting as a single, well-formed paragraph (e.g., start with “It names tracker rows…” or “The sweep names…”), keeping the exit-code explanation once.
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/workflows/oversee.md:45
- This bullet is a single very long line with multiple parentheticals, which hurts readability and makes future edits error-prone. Suggest wrapping into multiple indented lines / sub-bullets (keeping the same meaning) so the
mergedrule and the reconcile guidance are easier to scan.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
skills/orch/scripts/reconcile-work-items:40
- The validation rejects values with leading zeros (e.g. "01"), but the error message only says “non-negative integer”, which is misleading ("01" is non-negative). Recommend updating the message to explicitly mention the no-leading-zeros rule (and/or documenting the accepted format) so users can correct the value quickly.
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/orch/scripts/reconcile-work-items:127
- For each stale candidate, this always runs two
gh pr listcalls (mergedandopen), even thoughopen > 0causes an earlycontinue. You can reduce API calls and runtime by queryingopenfirst and only queryingmergedwhenopen == 0(or by using a single--state allquery and filtering).
merged="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state merged --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || merged=""
open="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state open --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || open=""
if [ -n "$merged" ] && [ -n "$open" ]; then
if [ "$open" -gt 0 ]; then
continue # a live PR is a live item, whatever the clock says
elif [ "$merged" -gt 0 ]; then
skills/orch/scripts/reconcile-work-items:119
$GH_CLIis expanded unquoted as part of a command line. IfRECONCILE_GH_CLIever contains unexpected whitespace/shell metacharacters, this can lead to unintended argument splitting or command injection. Safer approach: treat the CLI as an executable path (no args), or parse into a bash array and execute via array expansion (and considercommand --/ explicit validation of allowed values).
merged="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state merged --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || merged=""
open="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state open --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || open=""
…e linear skill's container guards Claude-Session: https://claude.ai/code/session_012epxJEzGqT7q3qcFhdZUt5
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (5)
skills/project-management/workflows/audit-issues.md:67
- This new paragraph duplicates itself (the 'audit decision taken…' sentence appears twice) and line 64 starts mid-sentence ('names tracker rows…'), which reads like a grammatical fragment. Consolidate into a single, non-redundant paragraph (e.g., one explanation of: where to run it, exit codes, what it reports, and why it matters).
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/scripts/reconcile-work-items:40
- The validation rejects integer strings with leading zeros (e.g., "08" / "01") but the error message says only 'non-negative integer'. Either (a) allow leading zeros safely (e.g., base-10 coercion before numeric comparisons), or (b) update the error message to explicitly state that leading zeros are not allowed (and why, if it's to avoid bash/octal pitfalls).
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/orch/SKILL.md:85
- The script’s
started-stalecheck can also report items as 'PR state unverified' whenghis absent/failing (it explicitly degrades to unverified rather than staying silent). This table description currently says 'PR merged or absent' only; please include the 'unverified' case (or adjust the script behavior to match the documented contract).
| `reconcile-work-items` | Read-only tracker sweep: parked containers (children Done, parent open), stale started items (untouched past `RECONCILE_STALE_HOURS`, PR merged or absent), Done items with unchecked `- [ ]` boxes. Exit 1 on findings, mutates nothing. |
skills/orch/README.md:48
- Related to the
started-stalebehavior: the script will still emit a finding when the PR state is unverifiable (e.g.,ghmissing/failing), and it keys onstate.type == \"started\"rather than specific state names. Consider clarifying in this row that the sweep can flag stale started items even when PR state can’t be verified, and/or that it relies on Linearstate.typerather than exact state names.
| `RECONCILE_STALE_HOURS` | Hours before an In Progress / In Review item counts as started-stale in `reconcile-work-items` sweeps | `24` |
skills/orch/workflows/oversee.md:45
- This bullet is very long and packs multiple conditions, rationale, and instructions into a single line, making it hard to scan during an incident/ops workflow. Consider splitting into 2–4 wrapped lines / sub-bullets (condition → action → why → what to report) to keep the event handling section readable.
- `merged` → mark the lane done, launch the next unblocked item. When the fleet's LAST item merges and the items live in Linear (the sweep reads the linear skill's cache; a GitHub-item fleet skips it with a note), run `.agents/skills/orch/scripts/reconcile-work-items` before closing out — a parked container or a Done item with open acceptance boxes is exactly what this pass ends with when a close step was skipped; report its findings with the close-out.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (3)
skills/project-management/workflows/audit-issues.md:65
- This paragraph is duplicated/overlapping and has a grammatical break at line 64 ("names tracker rows..." lacks a subject). Please consolidate into a single, non-repetitive explanation of the exit codes and what findings represent, and fix the sentence structure (e.g., start line 64 with "It names...").
Run `reconcile-work-items` only where the orch skill is installed (skip the
line otherwise — this workflow does not require orch). It is read-only:
exit 0 is a clean tracker, exit 1 is findings — carry them into the audit
as facts — and exit 2 is a broken sweep to fix before auditing. An audit
decision taken against a parked container's stale state is the failure
this line exists to prevent.
names tracker rows whose state no longer matches the work: parked containers,
stale started items, Done items with unchecked acceptance boxes. Carry its
findings into the audit as facts — an audit decision taken against a parked
container's stale state is the failure this line exists to prevent.
skills/orch/scripts/reconcile-work-items:40
- The validation rejects values like
0(matches0*[0-9]), but the error text says "non-negative" (which includes 0). Either adjust validation to allow 0, or tighten the message/spec to "positive integer" (and ideally explicitly document whether leading zeros like "01" should be accepted or rejected).
STALE_HOURS="${RECONCILE_STALE_HOURS:-24}"
case "$STALE_HOURS" in
"" | *[!0-9]* | 0*[0-9]) config_error "RECONCILE_STALE_HOURS must be a non-negative integer, got '$STALE_HOURS'" ;;
esac
skills/orch/scripts/reconcile-work-items:127
- For each stale started item, this always executes two
gh pr listcalls (merged + open). On large trackers this can become slow (and rate-limit prone). Consider queryingopenfirst and only queryingmergedwhenopen == 0(or otherwise restructure to avoid the guaranteed 2x CLI calls per item).
merged="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state merged --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || merged=""
open="$(env -u GH_REPO -u GITHUB_REPOSITORY $GH_CLI pr list --state open --head "$branch" --json number,isCrossRepository --jq '[.[] | select(.isCrossRepository | not)] | length' 2>/dev/null)" || open=""
if [ -n "$merged" ] && [ -n "$open" ]; then
if [ "$open" -gt 0 ]; then
continue # a live PR is a live item, whatever the clock says
elif [ "$merged" -gt 0 ]; then
pr_state="PR merged"
else
pr_state="no PR"
fi
fi
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37fca4c503
ℹ️ 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 (@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 (@codex) address that feedback".
Both observed defects share one shape — a write with no read-back — so one read-only sweep covers them:
reconcile-work-itemsreports parked containers (every non-canceled child Done, parent still open), stale started items (untouched past RECONCILE_STALE_HOURS with the branch-named PR merged or absent; a live PR stays quiet; an unavailableghdegrades to 'unverified', never silence), and Done items whose descriptions still carry unchecked- [ ]acceptance boxes — the check that would have caught DRO-15 the same day. It reads the linear skill's cache (offline, deterministic) and mutates nothing. oversee's close-out runs it when the fleet's last item merges; audit-issues' Linear preflight runs it so audit decisions never build on a parked container's stale state. Twelve pins, every finding paired with its healthy twin, missing cache a loud config error.Closes VST-318
Closes #1388
https://claude.ai/code/session_012epxJEzGqT7q3qcFhdZUt5