Skip to content

feat(session-flow): workflow probes git for missing files before asking - #12630

Closed
melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
work-resolve-issue/issue-12594-41563008213-1
Closed

melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
work-resolve-issue/issue-12594-41563008213-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #12594

Summary

Opened by the work-resolve-issue lane. The run's own summary:

The workflow skill's explore stage used to ask before investigating a referenced file missing from disk. It now runs read-only git status --porcelain and git log --diff-filter=D on that path first. It skips a staged or committed delete and says so, investigates a working-tree-only delete, and asks only when neither probe names the path or a probe fails. To make the two skills give one rule (record row 28), I copied the rule and commands from the explore skill's slice for the same gate, issue #12590, whose wording sits in open PR #12607. That slice is not on main yet, so plugins/discovery/skills/explore/reference/workflow.md still says ask-first until #12607 merges.

Not verified

  • The ai-slop deterministic detector (detect.sh) did not run, because the lane's Bash allows only the named wrappers. The rubric pass was a manual read and found no tells.
  • Tidy skipped passes 1-3 (code passes): the change is markdown only. Passes 4 and 5 ran and made no edits.
  • check-affected's CI-only checks (line-citations, purged-em-dashes, editorconfig and others) and its skipped validate-plugins steps are left to the PR's CI
  • The explore skill's matching wording lands only with open PR #12607 (issue #12590); until it merges, the explore and workflow skills still disagree on main

Fix

  • plugins/session-flow/skills/workflow/context/steps.md: the explore stage's missing-file bullet now runs the read-only git status and git log probe before any question. It classifies the result as intentional (skip it and cite the evidence), unexplained (investigate it) or not settled (ask).
  • .changes/session-flow/work-resolve-issue-issue-12594-41563008213-1-93e14bf0.md: a new minor changelog fragment describing the behavior change.

Verification

The runner ran each check from the base copy against a4d9623:

  • scripts/check-changelog-fragments.sh --check: passed
  • scripts/check-changed-skills.sh --merge-base: passed

The run reports:

  • read https: //github.com/melodic-software/claude-code-plugins/blob/7766ddefddf1e6d99842c105c27bf20a8a1d6a56/docs/audits/7005-confirm-gates/confirm-gates.md: row 139 (steps.md:34) and row 28 (explore) are both reversible-local, probe-first; probe-first means a read-only probe answers the question the
  • read https: //github.com/melodic-software/claude-code-plugins/issues /7005#issuecomment-6570157077: the owner approved probe-first for rows 21, 28 and 139
  • read https: //github.com/melodic-software/claude-code-plugins/issues /12590: the explore slice of the same gate is open, with open PR #12607
  • read https: //github.com/melodic-software/claude-code-plugins/pull /12607/files: the explore wording (the same two git commands, with intentional, unexplained and not-settled outcomes) that this change mirrors
  • check-changelog-fragments: all 5 fragments valid (run before and after tidy and review)
  • check-changed-skills: workflow PASS, 0 errors, 2 warnings (both about SKILL.md, which this change does not touch)
  • check-affected: 11 passed, 7 skipped, the rest CI-only
  • fresh-context review subagent: no defects; I applied two nits (limit the probe to investigation the task did not ask for; 'Skip investigating it')
  • Grep: nothing else in plugins/session-flow states the old ask-first rule, and nothing cites steps.md by line number
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41563008213

Left out: the items under Not verified.

Related

#12594

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed a4d96230ad99c65a5d998b140deeed9ea4448f05: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: no security issues found

I read every hunk of both changed files. Neither matches a trusted-path glob.

  • plugins/session-flow/skills/workflow/context/steps.md: the explore stage used to ask before looking into a referenced file that is missing from disk. Now it first runs two read-only probes, git --no-optional-locks status --porcelain -- <path> and git log --oneline -1 --diff-filter=D -- <path>, and acts on the result: it skips an intentional delete, looks into a delete made only in the working tree, and asks when the probes don't settle it.
  • .changes/session-flow/...93e14bf0.md: a changelog fragment, prose only.

Instruction-surface lens. This change narrows a standing instruction (an ask-first rule). I checked the removed rule against the six Gate 0 classes that docs/contracts/instruction-exception-register/README.md adopts: irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance and agent-authority. Without the old question, the agent now reads git history and investigates a file that was deleted only in the working tree. Nothing is mutated, published or deleted, and no new authority is granted, so no protected class matches. Both probes are read-only. --no-optional-locks keeps status from refreshing the index, and the -- separator stops a hostile filename from being read as a git option. Nothing in this change widens a trust control, a token's scope, what untrusted text reaches a lane, or anything that can bill.

PR context. It contains no embedded directives aimed at this lane.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on a4d96230ad99c65a5d998b140deeed9ea4448f05; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment on lines +34 to +44
- When code, `git status` or history references a file that is not on disk, probe before any
investigation the task did not ask for. Run
`git --no-optional-locks status --porcelain -- <path>` and
`git log --oneline -1 --diff-filter=D -- <path>`, each its own read-only call. A status line
decides first; the deleting commit counts only when status prints nothing:
- **Intentional**: a staged removal (`D` in the first status column), or no status line and a
commit that deleted the path. Skip investigating it and say so, citing the status line or
commit
- **Unexplained**: deleted only in the working tree (`D` in the second status column).
Investigate it like any other file in scope
- **Not settled**: neither probe names the path, or a probe fails. Ask before investigating

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test covers the new rule. Closing issue #12594 sets two criteria:

  • "The step runs the git probe before any question."
  • "A question remains only for the case the probe cannot settle."

This bullet meets both. But no file under plugins/session-flow/evals/ covers the explore stage's missing-file gate. Searching for deleted, not on disk or diff-filter finds only the unrelated cs-env-worktree-not-pruned and re-first-split-hop cases.

That means a later edit could bring back ask-first wording, or drop one of the three branches, and nothing would fail. One example: an edit that makes a worktree-only delete ( D) count as intentional.

Suggested fix: add an eval case. A scaffold could build a repo with three files and reference all of them from code:

  • one staged-deleted (git rm);
  • one deleted only from the working tree (rm);
  • one deleted by an earlier commit.

The case would then assert that:

  • both probes run before any question;
  • the staged and committed deletes are skipped, with the status line or commit cited;
  • the worktree-only delete is investigated;
  • no question is asked.

claude-lane: claude-review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pr-unblock / address-feedback: UNCERTAIN, left open

Evidence: plugins/session-flow/skills/workflow/context/steps.md:34, plugins/session-flow/evals/workflow-stage-5-settings-screen/case.yaml:8

The finding holds as far as the code shows. The new explore-stage rule (steps.md lines 34-44) sets three outcomes: intentional, unexplained and not settled. No case under plugins/session-flow/evals/ exercises it. No fix was made in this round, though. The suggested eval would need a git repo holding a staged delete, a worktree-only delete and a committed delete. Every eval case in this repository builds its fixture through context.scaffold_script, which points to a scaffold.sh file next to case.yaml, and the review-thread tool's check-paths refuses plugins/session-flow/evals/<case>/scaffold.sh because script files are outside this lane's scope. A case.yaml with no scaffold could not reach the gate. A person should add the case, with case.yaml, scaffold.sh and samples, or file it as a follow-up. The graders should check that both probes run before any question, that the staged and committed deletes are skipped with the evidence cited, that the worktree-only delete is investigated, and that no question is asked.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed a4d96230ad99c65a5d998b140deeed9ea4448f05: read 2 of 2 changed file(s); 2 finding(s), 1 as inline comments.

Code review: PR #12630, feat(session-flow): workflow probes git for missing files before asking

I read both changed files in full: plugins/session-flow/skills/workflow/context/steps.md and the new changelog fragment .changes/session-flow/work-resolve-issue-issue-12594-…md. The new rule is consistent. Status is checked first, and the delete commit counts only when status prints nothing. A staged D means skip, a worktree-only D means investigate, and anything else means ask. Both commands are read-only, and --no-optional-locks keeps git status from writing the index. The fragment's bump: minor fits a behaviour change in a skill, and its wording matches the new rule.

Closing issue #12594: acceptance criteria

Other notes

  • Path-scoped rules: .claude/rules/skill-bodies-state-current-rules.md and .claude/rules/cost-claims.md match steps.md. Neither is broken: the change restates no volatile upstream specific and makes no cost claim.
  • No plugin manifest changed, so no manifest lines.
  • No embedded instructions found in the PR context.
  • Verification in the PR body (check-changelog-fragments, check-changed-skills) is author-claimed. I did not run it.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head a4d96230ad99c65a5d998b140deeed9ea4448f05.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on a4d9623.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 3, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the lane left a review finding open and no adjudicator verdict settled it: none was recorded, it found the finding does not hold where the lane had not disputed it (an UNCERTAIN or VALID-defer reply), which only an INCORRECT reply lets a verdict resolve, it found the finding asks only for tests whose covering check runs had not all passed when the round began, or it raised a security question on a finding the security-review lane rated below CRITICAL; so a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=12630 -f activity=address-feedback

@kyle-sexton

Copy link
Copy Markdown
Member

Hold: replaced by #12680 (owner-login replacement after pr-unblock escalated). Releases when: #12680 merges, then this PR closes.

@kyle-sexton

Copy link
Copy Markdown
Member

Superseded by #12680, which carries this branch's head plus the eval scaffold the lane's path scope refused. Closed so the open-PR count reflects live work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Hard merge gate: do not merge while applied. needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(session-flow): workflow probes git for missing files instead of asking before investigating

1 participant