Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Notable changes to house-rules. Versions follow [semantic versioning](https://se

## Unreleased

- `hr-what-next`'s `check-open-work.mjs` judges each offer a "Approved, not built:" line names by that offer's own clause (asked, sent, relayed or awaiting a reply), not by words anywhere in the line, (clauses split at `;`, `,` and "and"; approved, selected or "not sent" closes it; "awaiting the user's reply" and "waiting for the user reply" open it; a `trigger:` clause is not read, but an offer written after it is), so a `trigger: pending local build` no longer makes an approved offer count as unanswered and three stage lines each awaiting a reply now give three unanswered offers; and an open stage line whose `trigger:` waits on another thread (an upstream, other or spelled-out "pull request 4", a maintainer) gets the same missing-`recheck:` warning as the line deferred (the spelled-out forms now count for deferred lines too). The stage-line reading moves to `scripts/stage-line.mjs`.
- `hr-code-review`'s seat client now passes `MODEL_PROVIDER` to a Codex seat (codex-acp reads it to choose the provider for new sessions, and the narrowed environment dropped it; no other seat, and not `DEFAULT_AUTH_REQUEST` or `INITIAL_AGENT_MODE`, gets it), and `references/reviewer-role.md` says the parent leaves protected paths (secrets, `.env` files, credentials) out of a large diff it saves into the review root and lists what it left out.
- `hr-read-reddit` and the README say that Reddit announced RSS support ends November 13, 2026, so feeds can fail, and tell readers to use the partial search fallback when one does (on the cutoff day search feeds answered HTTP 403 while a thread feed still answered); the README's dark banner and overview image URLs carry `?v=1`, per the `web-image-formats` rule (bump it in the commit that changes either image's bytes).
- Four wording fixes from review points on landed text: `web-image-formats.md` says to use a version parameter the image host does not own (and to leave a URL alone whose host already uses `v`) and that a branch-pointing URL shows the base branch's image until the pull request merges (check the asset in the pull request's files) and can keep the old bytes up to five minutes after it; `hr-diagnosing-bugs`' necessity test compares occasions with the other inputs and state alike; `hr-sqlite`'s rerun rows call their ratios indicative because the observer rejected those phases; `multi-agent.md`'s read-back clamps each start to 0 and stops when it reaches 0.
Expand Down
26 changes: 17 additions & 9 deletions skills/hr-what-next/scripts/check-open-work.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
// "approved, not built" lists the open lines that start "Approved, not built": the stages of approved work that
// were deferred, which stay candidates for every "next?" and the offer after the stage before them lands. A deferred
// (`[~]`) line that starts this way is a finding: it was filed in the wrong bucket.
// A stage line's offer counts as unanswered only when its own clause says it was asked or awaits a reply, not its
// `trigger:`; an open stage whose `trigger:` waits on another thread needs a `recheck:` date, as a deferred one does.
// "close-out due" prints when every open item is deferred and the plan is not completed: the session's work
// is finished, so the user owes a decision on each item and the reply offers All done or Pause. A plan whose
// Status is paused without a done line recording the user's Pause choice is a finding; with one, the pause
Expand Down Expand Up @@ -39,6 +41,7 @@
import { pathToFileURL } from "node:url";
import { evaluateDoneWhen } from "./done-when.mjs";
import { githubStates } from "./github-state.mjs";
import { openStageOffers, splitStage } from "./stage-line.mjs";
import { MARKS, RECHECK, droppedByUserUnasked, parsePlan, planSnapshot } from "./plan-ledger.mjs";
import { spawnSync } from "node:child_process";
import {
Expand Down Expand Up @@ -83,7 +86,6 @@ const usage =
'The output lists deferred items whose trigger is only the user\'s own action ("user selects", "user asks for the backlog") as "awaiting your yes/no"; this is a text match, see the header.';
// An unbuilt stage of work the user approved (a later phase, "PR N"): an open line that starts this way, never a deferred one.
const UNBUILT_STAGE = /^approved,? not built\b/i;
const PENDING_OFFER = /\b(?:asked|pending|unanswered)\b/i;
const TERMINAL_STATUS = /^(completed?|done|archived)\b/i;
const COMPLETED_STATUS = /^completed\b/i;
const PAUSED_STATUS = /^paused\b/i;
Expand Down Expand Up @@ -116,7 +118,7 @@ const OFFER_LIMIT = 3;
// A deferred item that waits on another thread (an upstream issue or pull request, a maintainer, another
// session), or an open question sent to one, needs a `recheck:` date: "trigger: reply" never fires itself.
const EXTERNAL =
/https:\/\/github\.com\/|\b[\w.-]+\/[\w.-]+#\d+|\bPR\s*#?\s*\d+|(?<![\w/#&])#\d+\b|\bupstream\b|\bmaintainers?\b|\b(?:another|other|the [\w-]+) (?:session|thread)\b/i;
/https:\/\/github\.com\/|\b[\w.-]+\/[\w.-]+#\d+|\bPR\s*#?\s*\d+|\b(?:pull request|issue)s?\s*#?\s*\d+|\bother (?:pull request|PR|issue)\b|(?<![\w/#&])#\d+\b|\bupstream\b|\bmaintainers?\b|\b(?:another|other|the [\w-]+) (?:session|thread)\b/i;
const SENT_QUESTION =
/\b(?:asked|relayed|sent)\b.*\b(?:session|thread|owner|maintainer)s?\b|\b(?:awaiting|waiting (?:on|for)) (?:a |the )?(?:reply|answer|response)\b/i;
const GITHUB_URL = /https:\/\/github\.com\/([\w.-]+)\/([\w.-]+)\/(?:pull|issues)\/(\d+)/g;
Expand Down Expand Up @@ -527,7 +529,9 @@ function rechecks(open, today, warnings) {
due.push(`${row(item)} — due ${date}`);
} else if (
!date &&
((item.mark === "~" && EXTERNAL.test(item.text)) || SENT_QUESTION.test(item.text))
((item.mark === "~" && EXTERNAL.test(item.text)) ||
(UNBUILT_STAGE.test(item.text) && EXTERNAL.test(splitStage(item.text).trigger)) ||
SENT_QUESTION.test(item.text))
) {
undated.push(
`L${item.line} waits on another thread but has no \`recheck: YYYY-MM-DD\` date: ${clip(item.text, 60)}`,
Expand Down Expand Up @@ -681,12 +685,16 @@ function unansweredOffers(items) {
/** @type {Map<string, string>} */
const unanswered = new Map();
for (const item of items) {
// An offer a stage line names is the approval it records, unless the line says the offer was asked and is pending.
if (item.mark === " " && (!UNBUILT_STAGE.test(item.text) || PENDING_OFFER.test(item.text))) {
for (const id of offersNamed(item.text)) {
if (!closed.has(id) && !unanswered.has(id)) {
unanswered.set(id, row(item));
}
if (item.mark !== " ") {
continue;
}
// A stage line's offer is the approval it records unless its own clause says it was asked and awaits a reply.
const ids = UNBUILT_STAGE.test(item.text)
? openStageOffers(item.text, offersNamed)
: offersNamed(item.text);
for (const id of ids) {
if (!closed.has(id) && !unanswered.has(id)) {
unanswered.set(id, row(item));
}
}
}
Expand Down
47 changes: 47 additions & 0 deletions skills/hr-what-next/scripts/stage-line.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Reading an "Approved, not built:" ledger line: its `trigger:` clause says what the stage waits on, and each
// offer the rest of the line names is judged by its own clause.

// The real `trigger:` field (the last one not written as `trigger:` in backticks) and its clause, which ends at ";",
// `recheck:` or `done-when:`. Offers written after the clause stay part of the rest of the line.
/** @param {string} text @returns {{ trigger: string, rest: string }} */
export function splitStage(text) {
const starts = [...text.matchAll(/(?<!`)\btrigger:/gi)];
const start = starts.at(-1)?.index;
if (start === undefined) {
return { trigger: "", rest: text };
}
const after = text.slice(start + "trigger:".length);
const end = after.search(/;|\b(?:recheck|done-when):/i);
const cut = end < 0 ? after.length : end;
return { trigger: after.slice(0, cut), rest: text.slice(0, start) + after.slice(cut) };
}

// What the line says about an offer's reply: asked, sent or relayed, or a reply, answer or selection awaited.
const OPEN = new RegExp(
[
String.raw`\b(?:asked|sent|relayed|unanswered)\b`,
String.raw`\b(?:reply|answer|response|selection)\s+(?:is\s+)?pending\b`,
String.raw`\bpending\s+(?:a\s+|the\s+)?(?:user(?:'s)?\s+)?(?:reply|answer|response|selection)\b`,
String.raw`\b(?:await\w*|waiting\s+(?:on|for))\s+(?:a\s+|the\s+)?(?:user(?:'s)?\s+)?(?:reply|answer|response|go|selection)\b`,
].join("|"),
"i",
);
const CLOSED =
/\b(?:approved|selected|chose|chosen|picked|answered)\b|\bnot\s+(?:yet\s+)?(?:sent|asked)\b/i;

// The offer IDs of a stage line whose own clauses say the offer was asked and awaits a reply. Clauses split at ";", ","
// and "and"; a clause naming no offer ("awaiting user reply") belongs to the offer named before it.
/** @param {string} text @param {(clause: string) => string[]} offersNamed */
export function openStageOffers(text, offersNamed) {
/** @type {Map<string, string>} */
const said = new Map();
/** @type {string[]} */
let last = [];
for (const clause of splitStage(text).rest.split(/[;,]|\band\b/i)) {
last = offersNamed(clause).length > 0 ? offersNamed(clause) : last;
for (const id of last) {
said.set(id, `${said.get(id) ?? ""} ${clause}`);
}
}
return [...said].filter(([, all]) => OPEN.test(all) && !CLOSED.test(all)).map(([id]) => id);
}
98 changes: 98 additions & 0 deletions skills/hr-what-next/test/check-open-work.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,104 @@ test("a stage line that says its offer was asked and is pending counts as an una
expect(report.warnings.join("\n")).toContain("3 offers are open and unanswered");
});

test("each offer a stage line names is judged by its own clause, not by the stage's trigger", () => {
const sent = setup({
ledger: [
"- [ ] Approved, not built: export; Offer NEXT-1 sent; awaiting user reply; trigger: user reply",
"- [ ] Approved, not built: import; Offer NEXT-2 sent; awaiting user reply; trigger: user reply",
"- [ ] Approved, not built: sync; Offer NEXT-3 sent; awaiting user reply; trigger: user reply",
],
});
const sentReport = check(sent.plan).report;
expect(sentReport.unansweredOffers).toHaveLength(3);
expect(sentReport.warnings.join("\n")).toContain("3 offers are open and unanswered");

const approved = setup({
ledger: [
"- [ ] Approved, not built: export, approved in Offer SEARCH-1 trigger: pending local build",
"- [ ] Approved, not built: import, approved in Offer SEARCH-2 trigger: pending local build",
"- [ ] Approved, not built: sync, approved in Offer SEARCH-3 trigger: pending local build",
],
});
const approvedReport = check(approved.plan).report;
expect(approvedReport.unansweredOffers).toEqual([]);
expect(approvedReport.warnings.join("\n")).not.toContain("offers are open and unanswered");

const mixed = setup({
ledger: [
"- [ ] Approved, not built: export, approved in Offer MIX-1, asked in Offer MIX-2, reply pending trigger: PR 4 lands",
],
});
const mixedReport = check(mixed.plan).report;
expect(mixedReport.unansweredOffers).toHaveLength(1);
expect(mixedReport.unansweredOffers.join("\n")).toContain("MIX-2");
expect(mixedReport.unansweredOffers.join("\n")).not.toContain("approved in Offer MIX-1 ,");
});

test("a stage line's offer state is read per offer, with approval, negation and ordinary reply wording", () => {
const unanswered = (/** @type {string} */ line) =>
check(setup({ ledger: [`- [ ] Approved, not built: ${line}`] }).plan).report.unansweredOffers;
expect(
unanswered("export, approved in Offer OLD-1 and asked in Offer NEW-1 trigger: PR 4 lands"),
).toHaveLength(1);
expect(unanswered("export; Offer OLD-1 sent and approved by user trigger: PR 4 lands")).toEqual(
[],
);
expect(unanswered("export; Offer NEW-1 not sent trigger: PR 4 lands")).toEqual([]);
expect(unanswered("export; approved in Offer OLD-1, build pending trigger: PR 4 lands")).toEqual(
[],
);
expect(
unanswered("export; Offer NEW-1 awaiting the user's reply trigger: PR 4 lands"),
).toHaveLength(1);
expect(
unanswered("export; Offer NEW-1 waiting for the user reply trigger: PR 4 lands"),
).toHaveLength(1);
expect(
unanswered("export trigger: local build; asked in Offer NEW-1; reply pending"),
).toHaveLength(1);
});

test("an open stage's real trigger is found after quoted field names, and a spelled-out pull request counts as another thread", () => {
const warnings = (/** @type {string} */ line) =>
check(setup({ ledger: [`- [ ] Approved, not built: ${line}`] }).plan).report.warnings.join(
"\n",
);
const missing = "waits on another thread but has no `recheck:";
expect(
warnings(
"a stage that names the `trigger:` and `done-when:` fields trigger: upstream PR 4 lands",
),
).toContain(missing);
expect(warnings("export; trigger: pull request 4 lands")).toContain(missing);
expect(warnings("export; trigger: pull request 4 lands recheck: 2099-01-01")).not.toContain(
missing,
);
const deferred = check(
setup({ ledger: ["- [~] export: later - trigger: pull request 4 lands"] }).plan,
).report;
expect(deferred.warnings.join("\n")).toContain(missing);
});

test("an open approved stage waiting on an upstream pull request needs a recheck date, like the same line deferred", () => {
const open = setup({
ledger: ["- [ ] Approved, not built: export trigger: upstream PR 4 lands"],
});
expect(check(open.plan).report.warnings.join("\n")).toContain(
"waits on another thread but has no `recheck:",
);
const dated = setup({
ledger: ["- [ ] Approved, not built: export trigger: upstream PR 4 lands recheck: 2099-01-01"],
});
expect(check(dated.plan).report.warnings.join("\n")).not.toContain("recheck: YYYY-MM-DD");
const ownTrigger = setup({
ledger: [
"- [ ] Approved, not built: export, built on PR 4's design trigger: the export schema is frozen",
],
});
expect(check(ownTrigger.plan).report.warnings.join("\n")).not.toContain("recheck: YYYY-MM-DD");
});

test("a stage line with trigger, recheck and a final done-when clause is accepted", () => {
const { plan } = setup({
ledger: [
Expand Down
Loading