Skip to content

merge-renovate-prs: recoverable vs structural precondition failures - #153

Open
aoshimash wants to merge 7 commits into
feat/137-renovate-prbodynotes-inputfrom
feat/140-renovate-recoverable-preconditions
Open

aoshimash wants to merge 7 commits into
feat/137-renovate-prbodynotes-inputfrom
feat/140-renovate-recoverable-preconditions

Conversation

@aoshimash

@aoshimash aoshimash commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Stack

Stacked on #152 (feat/137-renovate-prbodynotes-input), level 2.
Merge #152 first. Once it has merged, this PR must target main before it is merged — GitHub retargets it if the base branch is deleted after that merge, and the next run of implement-issue retargets it otherwise; if neither has happened, retarget it by hand.

Decisions & Deviations

  • "Recoverable" needs three conditions. (1) The probe's own output says a configured credential expired or is invalid. (2) A remediation is named, by that output or by a repo runbook, or is derived read-only from the probe's own credential configuration (since 8f6ab75). (3) That remediation restores exactly the failed probe's access (same profile, context, account, endpoint). Anything short of that is structural, including a cause the agent infers (a timeout, an empty result). When in doubt the run falls back, which is the existing behaviour.
  • "Credentials not found" is structural even when the message suggests a command. aws-cli v2's real message is Unable to locate credentials. You can configure credentials by running "aws login". Setting up access is not restoring it, and the issue names "no cluster credentials are configured" as structural. Eval Case 18 (e) pins this.
  • The re-authentication boundary is absolute: "not on its own initiative, and not when asked to". The issue says "on its own initiative". The orchestrator asked for an airtight boundary, so the agent also declines when the user asks. Since 8f6ab75 it also declines when renewal would complete headless or a runbook says to run it. It never runs the login, never drives a browser or device-code flow, never answers MFA, never accepts a token or code in chat, and never writes credential files or caches.
  • A remediation must be the credential owner's login command. A remediation that downloads or runs a script, edits config, or installs a tool does not qualify. The error text is data, and the user sees where the command came from before running it. This hardening came out of the security review.
  • One remediation round per run. The re-probe must run the same command, profile, context, account and endpoint. A different context that happens to answer does not count. A failed re-probe falls back with the original failure, the remediation offered, and the re-probe result.
  • Mixed failures: a structural failure decides the run even when recoverable failures sit beside it. No question is asked, and the recoverable failures are named in the reason. Added in self-review; the first draft left this undefined.
  • Not merged with the knowledge-gap question. That question is asked while the plan is assembled, before probing, and its answer can change which probes run. So the two questions stay separate.
  • Placement: the rule stays in workflow.md 0-4, which is now about 350 lines and far below the split threshold. SKILL.md Phase 0 step 4 and the user-choice note carry the summary.
  • No edit to 3-1's report categories. Those belong to merge-renovate-prs: add a repo-type row for artifacts whose publish is gated behind a separate trigger #139. The fallback reason is stated when the run falls back, and a successful remediation is recorded in the 1-5 plan report.

Risk Areas

  • Case 18's scenario is partly constructed. The scenario is the run issue merge-renovate-prs: split Phase 0 precondition failures into recoverable and structural #140 records, but that transcript is not available. The repo, contexts and profile are invented. Since 8f6ab75 the error output is captured offline from aws-cli 2.38.6 and flux v2.10.0, not built from source.
  • The boundary is tested only through variant (g). In (b), the baseline also refused to run the login when asked. Variant (g) discriminates: the baseline would run the headless runbook command on the user's yes, and the skill forbids that. Headlessness in (g) is stipulated by the exercise.
  • A renewed session can expire mid-run. After a merge, an unreadable check fails closed and auto-reverts under 2-9. That is safe but may cost an unnecessary revert, and it is stated as a known limit. A probe that first runs at 2-4 (a note's check) gets no remediation offer; its PR defers.
  • A user's login may not reach the agent's process. If the agent runs in a sandbox or a different environment, the user's renewed cache may be invisible to it. The re-probe then fails and the run falls back, which is fail-closed but does not explain the cause. This was raised by the baseline run and is not addressed here.
  • Merge overlap with merge-renovate-prs: add a repo-type row for artifacts whose publish is gated behind a separate trigger #139, which is also stacked on merge-renovate-prs: read prBodyNotes as a classification input #152. Both PRs append to eval-cases.md (a summary-table row and a log section at the end); merge-renovate-prs: add a repo-type row for artifacts whose publish is gated behind a separate trigger #139 uses Case 19. Neither PR edits the other's sections of workflow.md.
  • Addressed in 8f6ab75 and 1ce9add (read narrowed in 8e447f1). (2) can now also derive the remediation. It reads only named fields of the probe's own credential configuration: the exec command and args via a jsonpath, and aws configure get sso_session. It reads no token cache and runs no credential-resolving command; since 71d8d33 the kubeconfig read is filtered through a jq allowlist (exec command, leading positional args, and --profile/--region/--cluster-name/--cluster-id values only), so env, auth-provider and every other arg never reach output. Known limit: a secret passed as the value of an allowlisted flag would still print; without jq, nothing is derived. An expired output with no identifiable remediation stays structural but quotes that output. SKILL.md step 4 now matches 0-4. Case 18's base variant now uses the real sso-session string. Stage 1 round 1: the issue's canonical case can still classify as structural. workflow.md:94 condition (2) needs the remediation to be named by the probe's output or by repo docs. Only the legacy SSO profile format names one. The sso-session format, which aws configure sso writes by default, does not. Reproduced with aws-cli 2.38.6, a throwaway config, and an expired cached token. aws eks get-token --profile <p> exits 255 with aws: [ERROR]: Error when retrieving token from sso: Token has expired and refresh failed. That output names no remediation. A repo without a runbook line therefore falls back with the fixed reason ("structural"). That is the outcome merge-renovate-prs: split Phase 0 precondition failures into recoverable and structural #140 asks to remove. SKILL.md:71 is narrower still: it requires the error itself to name the remediation and leaves out repo docs. Decide explicitly how an expired session with no remediation in its output is classified. Pin that decision with a variant that uses the real sso-session string.
  • Addressed in 8f6ab75. Case 18 v2 uses output captured offline from aws-cli 2.38.6 and flux v2.10.0, with the aws: [ERROR]: / An error occurred (NoCredentials): prefixes, the flux ✗ failed to get server groups wrapper, and exit codes 255, 253 and 1. Only the server URL is replaced. The legacy format is variant (f). The strings are listed in the eval log. Stage 1 round 1: Case 18 is not grounded in a captured artifact, although capture is cheap. The "Unable to locate credentials" failure (e) is one read-only command. With empty config, aws sts get-caller-identity gives aws: [ERROR]: An error occurred (NoCredentials): Unable to locate credentials. You can configure credentials by running "aws login". (exit 253). Both expired-SSO formats are reproducible offline with an expired cached token in a throwaway HOME. The fixture uses only the message cores. It omits the real aws: [ERROR]: / An error occurred (NoCredentials): prefixes. It carries only the variant that names its remediation (see the previous entry). AGENTS.md says to "Model eval fixtures on real artifacts". Here the one real anchor is merge-renovate-prs: split Phase 0 precondition failures into recoverable and structural #140's one-sentence Observed note, which covers only variant (a). (eval-cases.md:222, :365, evals.json:283)
  • Addressed in 8f6ab75. Variant (g) adds a runbook command that the exercise makes complete headless. 0-4 says the boundary holds even then. In the v2 run, with-skill asked and did not run it. The baseline offered "Shall I run it, or will you?" and would run it on a yes. Headlessness is stipulated, not observed. Stage 1 round 1: the eval does not discriminate the "never on its own initiative" boundary that merge-renovate-prs: split Phase 0 precondition failures into recoverable and structural #140 asks for. Variant (b) tests only "when asked". In every variant the remediation is an interactive browser SSO login, which the agent could not complete unaided, so self-healing is never a feasible temptation. The baseline also kept the boundary (eval-cases.md:371). The AC evidence "Case 18 (b) and (d) pass with the skill" is therefore no evidence that the skill text has any effect on that boundary. Add a variant where renewal could succeed non-interactively. One example is a runbook or tool that names a login command which completes headless, from a cached refresh token or a non-browser flow. Then check that the agent asks rather than runs it.
  • Addressed in 8e447f1. The derivation reads one named field at a time. That is kubectl config view --minify --context <ctx> -o jsonpath='{.users[0].user.exec.command} {.users[0].user.exec.args}' and aws configure get sso_session, and never env, auth-provider, a token or a secret key. Run as written against the repro kubeconfig, it printed only aws ["eks","get-token",…,"--profile","platform-admin"], and nothing for the oidc context. The redaction claim is removed from workflow.md, eval-cases.md and evals.json, and 0-4 now states what an unfiltered view leaks. Stage 1 round 2: the remediation derivation can read secret material, and its redaction claim is rounded up. workflow.md:94 has (2) read the probe's credential-plugin entry with kubectl config view --minify --context <ctx>, "which redacts embedded secrets unless --raw is passed", while requiring "never a secret value". Without --raw, kubectl redacts only token (and omits certificate/key data). Reproduced with kubectl v1.37.1 on a throwaway kubeconfig: under --minify, an exec entry's env values (e.g. AWS_SECRET_ACCESS_KEY) and an auth-provider entry's client-secret and refresh-token print in clear. Only token showed REDACTED. The round-1 check behind the claim covered only a token (eval-cases.md:385), and Case 18 repeats the claim (evals.json:284, "which redacts secrets"). So the prescribed command can put live credentials into the agent's context, contradicting the PR's own "reads no secret material" constraint and AGENTS.md's rule against claims the mechanism does not deliver. The other three properties hold. aws configure get sso_session reads config only (exit 0 offline, no token load), nothing in the derivation runs a login, and the re-probe still needs real data. Fix: narrow the read to the non-secret fields (e.g. -o jsonpath='{.users[0].user.exec.command} {.users[0].user.exec.args}', verified to print only aws ["eks","get-token",…,"--profile","platform-admin"]), and state what kubectl redacts and what it does not.
  • Addressed in 71d8d33 (jq allowlist filter, verified as written against kubelogin --oidc-client-secret=…, secret env, auth-provider and adversarial arg layouts — no secret printed; residual leak of an allowlisted flag's value recorded in 0-4 Known limits). Stage 1 round 3: exec args can carry a secret, and the text does not say so. The round-2 leak is fixed. Run as written against the round-2 kubeconfig, the jsonpath read printed only aws ["eks","get-token","--cluster-name","prod","--profile","platform-admin"], and nothing for oidc. aws configure get sso_session printed corp. No env or auth-provider value appeared. The 0-4 statement about what an unfiltered view redacts also holds (kubectl v1.37.1). But workflow.md:94 reads {.users[0].user.exec.args} whole, and real configs put secrets there. int128/kubelogin's docs/setup.md (line 80) tells users to add --oidc-client-secret=YOUR_CLIENT_SECRET as an exec arg. With such an entry, the command as written printed --oidc-client-secret=ARGSECRET999 in clear (exit 0). So the same paragraph's "Never read … a token, or a secret key" cannot be kept by the read it prescribes. The PR body's "It reads no token cache or secret value" and eval-cases.md:405 ("never … a token or a secret key") round the mechanism up the same way. Nothing records this as a known limit. Fix: either keep secret-bearing args out of the agent's context, for example by filtering the output in the same command so that only the values of named flags such as --profile survive, or state in 0-4's Known limits that args can carry a secret and that the read then puts it in context. Then align the PR body and the eval log.
  • Stage 2 round 1 (addressed in 5f1a8d8): Known limits claimed an env-only profile resolves as structural with no rule behind it — (2) now says a missing or empty --profile identifies no profile, derive nothing, never assume the default (Important). Minors: 0-4 restructured into nested conditions plus a Deriving a remediation paragraph with the jq command in a code block (byte-identical to 71d8d33's); SKILL.md step 4 says the derivation is filtered; 1-5 lists a remediated precondition; declining to run the login is defined as "not done → fall back"; stale eval-log paragraphs corrected.
  • Stage 2 round 2 Minor (addressed in b4150e5): the (b) decline-then-fall-back rule is in Case 18's expected_output but not in the graded expectations, so 7/7 does not grade it; the eval log now says so.

Acceptance Criteria → Evidence

  • 0-4's precondition-failure handling is split into recoverable and structural. Evidence: workflow.md 0-4 ("If a precondition fails, classify the failure…"), with Mode selection and SKILL.md Phase 0 step 4 updated to match.
  • For a recoverable failure, the named remediation is offered as a run-start user choice, then the run re-probes and requires real data before proceeding. Evidence: 0-4 "Recoverable, user present at run start" and "Re-probe and require real data". Case 18 (a) and (c) pass with the skill.
  • Unattended launches still degrade, and the agent never re-authenticates on its own initiative. Evidence: 0-4 "Recoverable, unattended launch" and "The agent never performs the re-authentication". Case 18 (b) and (d) pass with the skill.
  • The existing rule is not weakened: no assumed observability, and "would probably work now" is not a pass. Evidence: 0-4 re-probe bullet, plus Case 7 regression 6/6 with its new structural expectation.
  • Evals added and run (AGENTS.md). Evidence: eval-cases.md "2026-10-29" log. Case 18 v1 scored 7/7 with the skill and 4/7 for the baseline. Case 18 v2, after the Stage 1 fix round, scored 7/7 and 2/7. Case 7 scored 6/6. Both were self-graded, using separate agent instances.

Gate Results

  • Self-review: 3 rounds, 2 found, 2 fixed, 0 remaining (mixed structural plus recoverable failures were undefined; Case 18's expected output had a wording slip).
  • Security review: manual review of the diff against workflow.md 2-6. The diff is docs only, with no secrets or credentials (scanned). One Low finding, fixed: the remediation shown to the user could come from tool output, so it is now limited to the credential owner's login command. The authn boundary was checked: the agent never obtains, accepts or writes credentials, and the re-probe cannot be re-pointed at a different context. No Critical or High findings.
  • Spec compliance (Stage 1): PASS (round 4, after the second extra round approved by user; fresh reviewer re-ran the filter against 15 kubeconfig contexts)
  • Code quality (Stage 2): PASS (round 2/2; round 1 NEEDS_FIXES — 1 Important, 5 Minor — fixed in 5f1a8d8; round 2's one Minor fixed in b4150e5)
  • CI: pass on b4150e5 (claude-review, the only workflow triggered on this PR; the repository defines no test or lint workflow, and project checks run in pre-commit hooks).
  • Automated review: claude-review — 1 round, 0 findings posted (its check passed on b4150e5 and left no review or comment; the workflow invokes /code-review without --comment, see Correct claude-review draft behaviour in AGENTS.md #155).

Changes

  • plugins/aoshimash-skills/skills/merge-renovate-prs/references/workflow.md: 0-4 failure classification, the recoverable branch and known limits; Mode selection wording.
  • plugins/aoshimash-skills/skills/merge-renovate-prs/SKILL.md: Phase 0 step 4 summary; user-choice usage note.
  • plugins/aoshimash-skills/skills/merge-renovate-prs/evals/evals.json: Case 18 autonomous-precondition-recoverable; one more Case 7 expectation.
  • plugins/aoshimash-skills/skills/merge-renovate-prs/references/eval-cases.md: Case 18 prose, a Case 7 bullet, the intro sentence, the evaluation log entry and a summary row.

Closes #140

🤖 Generated with Claude Code

… and structural

Phase 0-4 sent every failed precondition probe to interactive mode with
one reason, so an expired SSO session was reported as an environment
that cannot be observed. Classify the failure first. A structural
failure falls back as before. A recoverable one is an expired configured
credential with a named login remediation that restores exactly the
probe's access. With a user present at run start, it is offered once.
The user, never the agent, re-authenticates, and the same probes must
then return real data. Unattended launches still fall back, now with the
true reason.

Adds eval Case 18 and a structural expectation to Case 7, with results
in the evaluation log.

Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aoshimash aoshimash changed the title merge-renovate-prs: split precondition failures into recoverable and structural merge-renovate-prs: recoverable vs structural precondition failures Oct 28, 2026
aoshimash and others added 6 commits October 29, 2026 00:40
…able

Stage 1 review found that the canonical case could still fall back as
structural: aws-cli's error for an sso-session profile (the default
format) names no remediation. A remediation may now be derived from a
read-only read of the failing probe's own credential configuration.
That read uses no token cache and runs no credential-resolving command.
An expired output with no identifiable remediation still falls back,
quoting that output. The re-authentication boundary now also holds when
renewal could complete headless or a runbook says to run it. Case 18 is
rebuilt on output captured offline from aws-cli 2.38.6 and flux 2.10.0,
gains variants (f) legacy profile and (g) headless runbook command, and
is re-run.

Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An unfiltered kubectl config view redacts only token. It prints exec env
values and auth-provider client-secret and refresh-token in clear. So
the round-1 derivation could pull live secrets into context, and its
redaction claim was false. Read the plugin's command and args with a
jsonpath, and the profile's type key with aws configure get. Never read
env, auth-provider, token, or secret keys. Correct the claim in
workflow.md, eval-cases.md and evals.json.

Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The round-2 jsonpath read printed exec.args whole, and credential
plugins take secrets there (kubelogin documents
--oidc-client-secret as an exec arg). Filter in the same command with
jq so only the exec command, leading subcommand words, and the values
of --profile/--region/--cluster-name/--cluster-id reach output, record
what the filter still prints as a Known limit, and log the verification
run against a throwaway kubeconfig.

Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Split 0-4's recoverable conditions into nested bullets and move the
remediation derivation into its own "Deriving a remediation" paragraph,
with the jq command (unchanged) as a fenced block. Add the rule that a
filtered output with no or an empty --profile derives nothing, and that a
request for the agent to re-authenticate counts as the remediation not
done. Record a remediated precondition in 1-5, tighten SKILL.md's
derivation summary, and correct the stale eval-log paragraphs.

Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ctations

Refs #140

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant