Skip to content

fix(agent): bound structured close reasons - #3213

Merged
JSONbored merged 7 commits into
mainfrom
codex/fix-unbounded-close-reasons-issue
Jul 4, 2026
Merged

fix(agent): bound structured close reasons#3213
JSONbored merged 7 commits into
mainfrom
codex/fix-unbounded-close-reasons-issue

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent unbounded growth of persisted structured close reasons that can produce very large metadata_json or params_json rows and exhaust storage or D1 limits.

Description

  • Add STRUCTURED_CLOSE_REASONS_MAX_COUNT and boundStructuredCloseReasonsForPersistence() to cap the number of persisted structured close reasons to a safe, small constant.
  • Apply the cap before persisting: buildAgentActionAudit() copies a bounded slice and exposes closeReasonsTruncated when the original count exceeded the cap, and closeReasonsForAudit() and actionParams() also use the same bound before per-reason truncation.
  • Keep the existing per-reason audit truncation (AUDIT_REASON_MAX_LENGTH = 280) while preventing an unbounded array from being serialized.
  • Add unit tests that exercise approval-queue param bounding, audit metadata bounding, and direct buildAgentActionAudit() truncation behavior, and regenerate the self-host env reference updated by the edit.

Testing

  • Ran the targeted unit tests with npm test -- --run test/unit/agent-execution.test.ts test/unit/agent-action-executor.test.ts, which passed.
  • Ran npm run typecheck which succeeded (no type errors).
  • Attempted the full gate via npm run test:ci, but the run could not be completed in this environment due to unrelated long-running queue tests/timeouts and was stopped; the functional unit coverage for the changed files was verified locally.
  • npm audit --audit-level=moderate was attempted but the registry audit endpoint returned 403 Forbidden in this environment so the dependency-audit step could not be completed here.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 4, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 38a1c3b Commit Preview URL

Branch Preview URL
Jul 04 2026, 11:26 PM

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 4, 2026
@loopover-orb

loopover-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-04 23:27:03 UTC

4 files · 1 AI reviewer · no blockers · readiness 93/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

Review summary
The change correctly caps structured close reasons before persistence in both audit metadata and pending-action params, and it preserves the original reason count so truncated audit rows remain distinguishable. The important source-level detail is handled in `closeReasonsForAudit()`: it slices before mapping through `boundAuditReason()`, so the hot path no longer does unbounded per-reason work before the persistence cap. I do not see a reachable correctness defect in the visible diff.

Nits — 4 non-blocking
  • nit: `src/settings/agent-execution.ts:64` accepts `closeReasonCount` without clamping it to at least `closeReasons.length`, so a mistaken caller could under-report truncation; this is not reachable from the new executor path but the helper contract should either document that invariant or normalize it defensively.
  • nit: `test/unit/agent-execution.test.ts:120` covers the over-limit case but not the exact-limit boundary where `closeReasonsTruncated` must stay absent; that branch is worth pinning because the metadata distinction is part of this PR's contract.
  • In `src/settings/agent-execution.ts:64`, consider deriving `const reportedCloseReasonCount = Math.max(closeReasonCount, closeReasons.length)` after bounding so the audit helper cannot silently suppress `closeReasonsTruncated` if a future caller passes a stale count.
  • Add an exact-limit assertion in `test/unit/agent-execution.test.ts` for `STRUCTURED_CLOSE_REASONS_MAX_COUNT` reasons verifying `closeReasonCount` equals the limit and `closeReasonsTruncated` is not present.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 429 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 429 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 56 PR(s), 429 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.11%. Comparing base (15e308c) to head (38a1c3b).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3213   +/-   ##
=======================================
  Coverage   94.11%   94.11%           
=======================================
  Files         275      275           
  Lines       30120    30124    +4     
  Branches    10993    10995    +2     
=======================================
+ Hits        28347    28351    +4     
  Misses       1127     1127           
  Partials      646      646           
Files with missing lines Coverage Δ
src/services/agent-action-executor.ts 96.77% <100.00%> (+0.01%) ⬆️
src/settings/agent-execution.ts 94.44% <100.00%> (+0.50%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 4, 2026
JSONbored added 4 commits July 4, 2026 15:47
…t path

The executor's closeReasonsForAudit() pre-bounded the reason count
before buildAgentActionAudit() ever saw it, so closeReasonsTruncated
could never be set on a real close action -- only on a direct
buildAgentActionAudit() call with an unbounded array. Count-bounding
now happens exactly once, inside buildAgentActionAudit, so the
persisted audit row's closeReasonCount/closeReasonsTruncated reflect
the true original count regardless of caller. Also removes a
resulting dead ?? fallback that TypeScript's own narrowing already
made unreachable.
JSONbored added 2 commits July 4, 2026 16:23
The prior fix restored truncation visibility but reintroduced the
original cost problem: closeReasonsForAudit mapped every reason
through boundAuditReason before any count cap ran, so an unbounded
closeReasons array still cost O(N) string-length work on the hot
executor path. Bound the count first (a cheap slice), map only the
bounded subset, and carry the true original count to
buildAgentActionAudit separately (as closeReasonCount) so the
persisted audit row still correctly flags truncation.
@JSONbored
JSONbored merged commit 69916ca into main Jul 4, 2026
11 checks passed
@JSONbored
JSONbored deleted the codex/fix-unbounded-close-reasons-issue branch July 4, 2026 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant