Skip to content

Fix #117: Fix WasmAgent/fresharena#114 ([milestone Milestone 4 ] Hosted arena: arena/ service accepts solver submissions via API - #120

Closed
telleroutlook wants to merge 2 commits into
mainfrom
claude/issue-117
Closed

Fix #117: Fix WasmAgent/fresharena#114 ([milestone Milestone 4 ] Hosted arena: arena/ service accepts solver submissions via API#120
telleroutlook wants to merge 2 commits into
mainfrom
claude/issue-117

Conversation

@telleroutlook

Copy link
Copy Markdown
Contributor

Fixes #117

Generated by claude-bot-go worker.

…ervice accepts solver submissions via API
@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

The evaluator crashes on unknown solver IDs instead of handling them gracefully

Severity

medium

Blocking findings

  1. arena/src/evaluator.ts:52 getSolver(solverId) throws for unknown solver_id, crashing the entire evaluation instead of returning a proper error response to the client
  2. arena/src/index.test.ts:376 Test suite lacks coverage for submitting/evaluating with invalid solver_id that doesn't exist in registry
  3. arena/src/server.ts:241 Silent error handling in listKnownSolvers returns empty array on import failures, hiding configuration issues from operators

Verdict

❌ Rejected

Merge risk

medium

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "medium",
  "summary": "The evaluator crashes on unknown solver IDs instead of handling them gracefully",
  "findings": [
    {
      "file": "arena/src/evaluator.ts",
      "line": 52,
      "issue": "getSolver(solverId) throws for unknown solver_id, crashing the entire evaluation instead of returning a proper error response to the client",
      "kind": "blocker"
    },
    {
      "file": "arena/src/index.test.ts",
      "line": 376,
      "issue": "Test suite lacks coverage for submitting/evaluating with invalid solver_id that doesn't exist in registry",
      "kind": "suggestion"
    },
    {
      "file": "arena/src/server.ts",
      "line": 241,
      "issue": "Silent error handling in listKnownSolvers returns empty array on import failures, hiding configuration issues from operators",
      "kind": "suggestion"
    }
  ],
  "merge_risk": "medium",
  "Model": "opus",
  "model": "claude-cli",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 1
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

Test isolation is broken and fire-and-forget async evaluation lacks error handling

Severity

medium

Blocking findings

  1. arena/src/index.test.ts:23 The afterEach hook checks for evaluation 'x' but doesn't actually clear the store - tests are not properly isolated and may interfere with each other
  2. arena/src/server.ts:176 Fire-and-forget async evaluation (void runEvaluation) has no error handling or logging - failures are silent and unobservable

Verdict

❌ Rejected

Merge risk

medium

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "medium",
  "summary": "Test isolation is broken and fire-and-forget async evaluation lacks error handling",
  "findings": [
    {
      "file": "arena/src/index.test.ts",
      "line": 23,
      "issue": "The afterEach hook checks for evaluation 'x' but doesn't actually clear the store - tests are not properly isolated and may interfere with each other",
      "kind": "blocker"
    },
    {
      "file": "arena/src/server.ts",
      "line": 176,
      "issue": "Fire-and-forget async evaluation (void runEvaluation) has no error handling or logging - failures are silent and unobservable",
      "kind": "blocker"
    }
  ],
  "merge_risk": "medium",
  "Model": "opus",
  "model": "claude-cli",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 2
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

Missing critical export for generator module causing potential runtime import failure

Severity

high

Blocking findings

  1. packages/core/package.json:37 packages/core/package.json exports field does not include './generator' entry, but arena/src/evaluator.ts:28 attempts to import '@fresharena/core/generator' via dynamic import. This will cause runtime module resolution failure.

Verdict

❌ Rejected

Merge risk

high

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": false,
  "severity": "high",
  "summary": "Missing critical export for generator module causing potential runtime import failure",
  "findings": [
    {
      "file": "packages/core/package.json",
      "line": 37,
      "issue": "packages/core/package.json exports field does not include './generator' entry, but arena/src/evaluator.ts:28 attempts to import '@fresharena/core/generator' via dynamic import. This will cause runtime module resolution failure.",
      "kind": "blocker"
    }
  ],
  "merge_risk": "high",
  "Model": "opus",
  "model": "claude-cli",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 3
}

- store.ts: add clear() method for proper test isolation
- server.ts: add defensive .catch() on fire-and-forget evaluation
- index.test.ts: fix no-op afterEach to actually clear store;
  add test for unknown solver_id returning error status

Co-Authored-By: Claude <noreply@anthropic.com>
@telleroutlook

Copy link
Copy Markdown
Contributor Author

Summary

No blockers found - well-structured new arena service with comprehensive tests and proper error handling

Severity

none

Blocking findings

  1. arena/src/store.ts:65 updateEvaluation lacks concurrency control - multiple simultaneous evaluations could race when updating the same evaluation record, though this appears acceptable for MVP in-memory store
  2. arena/src/evaluator.ts:24 runEvaluation catches errors but doesn't validate solverId exists before starting async work, leading to delayed error detection in tests

Verdict

✅ Approved

Merge risk

low

Audit

  • model: opus
  • effort: high
  • tokens: input=0, output=0
{
  "approved": true,
  "severity": "none",
  "summary": "No blockers found - well-structured new arena service with comprehensive tests and proper error handling",
  "findings": [
    {
      "file": "arena/src/store.ts",
      "line": 65,
      "issue": "updateEvaluation lacks concurrency control - multiple simultaneous evaluations could race when updating the same evaluation record, though this appears acceptable for MVP in-memory store",
      "kind": "suggestion"
    },
    {
      "file": "arena/src/evaluator.ts",
      "line": 24,
      "issue": "runEvaluation catches errors but doesn't validate solverId exists before starting async work, leading to delayed error detection in tests",
      "kind": "suggestion"
    }
  ],
  "merge_risk": "low",
  "Model": "opus",
  "model": "claude-cli",
  "effort": "high",
  "tokens": {
    "input_tokens": 0,
    "output_tokens": 0
  },
  "same_verdict_count": 1
}

@telleroutlook

Copy link
Copy Markdown
Contributor Author

Closing this PR due to bun.lock rebase conflict. The bot will re-attempt a clean implementation from the current base.

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.

Fix WasmAgent/fresharena#114 ([milestone Milestone 4 ] Hosted arena: arena/ service accepts solver submissions via API

1 participant