Skip to content

Isolate Claude review settings and diagnose failed executions safely - #128

Merged
sbalci merged 3 commits into
masterfrom
codex/claude-review-safe-diagnostics
Oct 20, 2026
Merged

sbalci merged 3 commits into
masterfrom
codex/claude-review-safe-diagnostics

Conversation

@sbalci

@sbalci sbalci commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

The October 12 Mixed Cox and classical survival power reviews initialized but returned subtype: success with is_error: true, empty model usage, zero reported cost and no permission denials. GitHub OIDC/app-token authentication and actor authorization succeeded. Claude OAuth validity, account/model access and service/CLI failures remain unproven because sanitized logs omit the underlying error and neither run retained an execution artifact. The action correctly failed both runs.

Use --setting-sources user,project to exclude tracked workstation-local macOS PATH/permissions from CI while retaining action-created user settings and shared project settings. This fixes a portability risk, not a confirmed cause of the original failures. Add a secret-availability preflight, 30-minute limit and terminal-result verification with safe fixed diagnostic categories. Full output stays disabled; no raw transcripts/error text are published. Require action success plus terminal subtype: success and is_error: false. Missing/malformed results and startup errors cannot pass. Execution success does not certify code correctness or feedback delivery.

Add .github/claude-review-troubleshooting.md with evidence, remaining uncertainties, private credential/model checks and exact claude setup-token / existing Actions-secret replacement instructions when needed. GitHub App authentication, permissions and default model are preserved; no secret values were read or changed.

Validation: YAML parsing, embedded Python compilation, 11 diagnostic fixtures and two secret-preflight cases passed. Sentinel secrets and workflow-command injection text did not reach stdout/summary. Input names, setting-sources pass-through and execution-file fallback verified against upstream at failing action SHA 1d6de8cb0c237e7c15e9e1bdf973826ebae490cc. actionlint is unavailable locally. No statistical source changed.

Live run 38734392859 passed credential availability, then the Claude GitHub App deliberately skipped execution because the workflow differs from the default branch. The action step returned success, but the new guard correctly failed the job for the missing result. This validates the missing-result protection on GitHub, not credential/model access. Actual execution must be tested after merge, in a fresh PR run whose workflow matches the default branch. Do not bypass the App protection; rerunning an old run uses its old workflow. Authentication/account intervention is still conditional on further evidence.

@sbalci sbalci left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Operational validation review (2026-10-20): inspected the two-file change against current master 3e38501. No integration-blocking issue found in the credential-availability check, strict action-plus-terminal-result guard, fixed-label diagnostics, transcript suppression, settings isolation or 30-minute limit. Existing GitHub App protections and permissions are preserved.

Independent local validation passed YAML parsing, embedded Python compilation, 17 execution fixtures and two credential-preflight cases. Fixtures cover action failure despite a success result, missing/malformed output, init-only, missing/wrong is_error flags, last-result selection, five diagnostic categories, and sentinel secret/workflow-command injection suppression. These are local fixtures, not successful model inference.

Latest PR checks: Ubuntu R release passed; Claude run 38734593158 passed credential availability, then the App skipped the differing workflow and the guard correctly failed on the missing terminal result. No newer Claude run was found in the inspected recent runs. Error categories remain heuristic hints, not proven causes.

Next: integrate through this PR, then open a controlled documentation-only PR based on the merged default branch with an identical workflow. Verify a fresh Claude terminal non-error result and separately verify bot feedback delivery. Do not rerun old workflow revisions or bypass App validation. No secret values were read or changed. This COMMENT records technical review; it is not a fabricated independent maintainer approval.

@sbalci
sbalci marked this pull request as ready for review October 20, 2026 16:20
@sbalci
sbalci merged commit 1d54dd1 into master Oct 20, 2026
3 of 6 checks passed

sbalci commented Oct 20, 2026

Copy link
Copy Markdown
Owner Author

Post-merge operational validation is recorded in #129. Fresh run https://github.com/sbalci/ClinicoPathJamoviModule/actions/runs/39456744526 executed Claude without the workflow-mismatch skip, then returned is_error: true with no recorded model usage. Both the action and result guard failed correctly. Safe diagnostics reported an authentication hint; Claude credential validity/account-model access and feedback delivery remain unverified pending a private maintainer credential/account check. No secret was read/changed and no protection was bypassed.

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