Skip to content

[agentic] - Fail the harness optimizer's score gate closed on NaN and map malformed prior artifacts to AgenticError - #1647

Merged
cgfixit merged 2 commits into
mainfrom
claude/cyclaw-optimize-harness-fail-closed
Oct 13, 2026
Merged

cgfixit merged 2 commits into
mainfrom
claude/cyclaw-optimize-harness-fail-closed

Conversation

@cgfixit

@cgfixit cgfixit commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Proposed changes

This PR closes two gaps in agentic/harness_optimizer/: one gate fails open, and one error escapes the typed-error contract. A /CyClaw-Optimize scan found both, and I reproduced each on main before fixing it.

  1. NaN passes the "score improved" gate.
    • decide_candidate rejects with score_not_improved when candidate.score <= baseline.score.
    • Any comparison with NaN is False, so a NaN score on either side skips the gate.
    • RunReport.__post_init__ only checked that the score was numeric. CaseResult's 0 <= score <= 1 range check also passes NaN.
    • Reproduced on main: a NaN candidate against a 0.7 baseline returns accepted: True.
    • Fix: RunReport now rejects non-finite scores (math.isfinite), and CaseResult's range check now rejects NaN and inf.
  2. A wrong-shaped prior artifact escapes the typed-error contract. apply_candidate_artifact mapped only JSONDecodeError to AgenticError. Reproduced on main:
    • [] and null raised AttributeError.

    • {"version": "x"} raised ValueError.

    • {"version": -1} and {"version": true} were silently accepted, producing versions 0 and 2.

    • Fix: the function now raises AgenticError("existing harness artifact is malformed") when the prior artifact is any of:

      • not a JSON object;
      • carrying a version that is not a non-negative int;
      • not valid UTF-8.

      The artifact is left untouched and the lock is released.

Honest scope: the shipped GitHubCodingRunner builds scores with float(bool), so shipped code cannot produce NaN today. RunReport and CaseResult are public: agentic.harness_optimizer exports them. This change hardens that API to fail closed when a custom runner or direct caller passes NaN; it is not a fix for a live exploit.

Invariant / Governance Impact: none of the six invariants is touched.

  • No core-six file changes, and no graph edge changes.
  • I6 isolation is unchanged: the change stays inside agentic/.
  • check_invariants.py reports 46 passed, 0 failed.
  • The change strengthens the harness's own accept gate.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Security (auth, privacy, injection, egress, redaction, or secrets)
  • Invariant / Governance refinement (use this for changes that strengthen or evolve the 6 invariants, I6 isolation, or harness phases)
  • Dependencies (added, removed, or bumped)
  • CI / infrastructure (workflows, hooks, templates, or checkers)
  • Refactor or tests (behavior-preserving, no new capability)
  • Documentation update
  • Other (none of the boxes above fit)

Scope: Out-of-band agentic layer (agentic/harness_optimizer/core.py, scoring.py, patching.py, plus two test files).

Benefits / why

  • The candidate-acceptance gate now fails closed on NaN or inf instead of open.
  • Every malformed-artifact case now surfaces as the typed AgenticError that the agentic CLI maps to its exit codes. Before, these cases produced raw tracebacks or silently wrong version counters.

Risks to monitor

  • Stricter version parsing: a prior artifact whose version is a numeric string ("3") or a float (3.0) was previously coerced by int(). It is now refused as malformed. The only writer, apply_candidate_artifact, always writes an int, so this affects only hand-edited files.
  • Infinite scores: a runner that deliberately reports inf now raises at construction instead of silently winning the gate.
  • Detection: new tests in tests/test_agentic_harness_optimizer.py cover non-finite RunReport and CaseResult scores. New tests in tests/test_agentic_harness_phase679.py cover five malformed prior-artifact shapes.

Checklist

  • I have read docs/THREAT_MODEL.md, INVARIANTS.md and SECURITY.md (and any relevant Phase docs)
  • This change preserves all 6 security invariants and I6 module isolation (explicit evidence or invariant matrix included for core changes)
  • Full sandbox validation has been run (GROK_API_KEY=dummy pytest tests/ -q --tb=short, and bash .claude/skills/CyClaw-Sandbox/verify.sh for core RAG/agentic paths) and passes with no regressions. Not run locally, per the CLAUDE.md §5 verification policy; CI runs the suites.
  • No new external network dependencies or mandatory online LLM assumptions were introduced without explicit justification + offline fallback path
  • For any agentic/fsconnect change: two-phase audit, quota enforcement, governed delete/trash, and write guards have been verified. The apply path's human gates and lock are unchanged; the new test asserts the lock is released on the malformed path.
  • Relevant architecture docs or threat model notes have been updated if core behavior or topology changed (n/a: no core change)
  • Commit messages follow the title prefix convention above
  • For large or complex changes: before/after invariant matrix + sandbox evidence is included in "Further comments" or linked (n/a: small change)

Further comments

Verification I ran directly, following CLAUDE.md §5:

  • Code probe, main vs this branch: on main, a NaN candidate returned accepted: True. On this branch, NaN and inf are rejected at construction, and a normal 0.7 → 0.8 candidate is still accepted.
  • One targeted test selection: pytest tests/test_agentic_harness_phase679.py -k malformed_prior gives 5 passed. With patching.py reverted to main, the same selection gives 5 failed: one ValueError and four "DID NOT RAISE".
  • Lint:
    • ruff check --select E,F,I,B,C4,UP,S --ignore E501 is clean.
    • The two test files already had ruff format drift before this PR. I did not reformat them, and this diff adds no new drift (the drift count matches main).
  • Invariants: check_invariants.py reports 46 passed, 0 failed.
  • Not run locally: the full suite and the coverage gate. GitHub Actions runs those.

Suggested merge order of open PRs

Trial merges verified (git merge --no-ff, all three branches in sequence onto a throwaway checkout) against main @ 6396152 on 2026-10-13. All three touch disjoint files.

#1647 (this PR) · Blocked ⛔ (draft, CI pending)
Impact: agentic/harness_optimizer/ only; independent.
Action: Merge first (lowest number). Trial merge clean vs main.

#1648 · Blocked ⛔ (draft, CI pending)
Impact: agentic/fsconnect/indexer.py + a sync/filters.py comment; independent.
Action: Merge second; no shared files.

#1649 · Blocked ⛔ (draft, CI pending)
Impact: memory/store.py, memory/policy.py + tests; independent.
Action: Merge third; no shared files.

ELI5

Think of the harness optimizer as a judge who lets a new version through only if it scores higher than the old one. A NaN ("not a number") score is like a blank scorecard. The judge's rule, "reject if new ≤ old," can't evaluate a blank card, so the candidate got waved through. RunReport and CaseResult now refuse blank and infinite scorecards before they reach the judge (decide_candidate).

Separately, recording an accepted candidate means reading the previous record to bump its version number. When that file held valid JSON of the wrong shape, the code either crashed with a raw Python error or quietly computed a nonsense version (-1 became 0, true became 2). apply_candidate_artifact now reports "this artifact is malformed" with the project's standard error type and leaves the file alone.

What it does not do: it changes no gate other than these input checks, and it touches no CyClaw core path. I did not run the full test suite locally; CI runs it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BJXTdWRbDpkynPYbpL2pxd


Generated by Claude Code

Last updated: 2026-10-13 09:43 ET

… map malformed prior artifacts to AgenticError

decide_candidate's score_not_improved gate is `candidate.score <= baseline.score`,
which is False whenever either side is NaN, so a NaN RunReport was accepted.
RunReport now rejects non-finite scores and CaseResult's range check rejects
NaN/inf. apply_candidate_artifact now maps a wrong-shaped prior artifact
([], null, a non-integer/negative/bool version, non-UTF-8 bytes) to the typed
AgenticError instead of a raw AttributeError/ValueError or a silently bogus
version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BJXTdWRbDpkynPYbpL2pxd
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

PR is a complete fill of .github/PULL_REQUEST_TEMPLATE.md (every section, ticked boxes, merge order, ELI5 last, last-updated stamp, title, branch).

@cgfixit

cgfixit commented Oct 13, 2026

Copy link
Copy Markdown
Owner Author

@codex review (for bugs to fix and performance/security/other regressions potentially introduced in this pr)

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T14:38:33.809070Z ff1696b Draft marked ready
ℹ️ 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 review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f775cd0d4

ℹ️ 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 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 address that feedback".

Comment thread agentic/harness_optimizer/core.py Outdated
…cores keep the typed-error contract

math.isfinite(10**1000) raises OverflowError converting the int to float, so
the new check turned a previously valid RunReport into an untyped exception and
replaced CaseResult's AgenticError range rejection with one (Codex review).
Ints are always finite; the check now runs on floats only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BJXTdWRbDpkynPYbpL2pxd
@cgfixit
cgfixit marked this pull request as ready for review October 13, 2026 14:36
@cgfixit
cgfixit merged commit 7a647be into main Oct 13, 2026
71 checks passed
@cgfixit
cgfixit deleted the claude/cyclaw-optimize-harness-fail-closed branch October 13, 2026 14:37
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.

2 participants