Skip to content

fix(zero-trust): record Unverified agents' results + follow-up test coverage - #148

Merged
guohai merged 1 commit into
mainfrom
fix/zero-trust-region-followups
Sep 2, 2026
Merged

fix(zero-trust): record Unverified agents' results + follow-up test coverage#148
guohai merged 1 commit into
mainfrom
fix/zero-trust-region-followups

Conversation

@guohai

@guohai guohai commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Follow-ups to the zero-trust agent region merge (#143).

What

  • Complete route: gate result insertion on claimed-ness, not siteId (server/routes.ts). The pre-existing job.siteId != null guard meant "never claimed" under the old model — under zero trust a job claimed by an Unverified agent also has siteId: null, so its results were silently dropped and the My Evals "Unverified" bucket never received route-produced data. The gate now checks job.evalAgentId != null; siteId flows through as NULL into eval_results (nullable, already handled by the unverified metrics scope).
  • Round-trip test: register (Unverified) → targeted run → claim → complete over real HTTP produces an eval_results row with siteId NULL (tests/zero-trust-dispatch.test.ts).
  • Listing lifecycle test: upsertListing(region: null) persists an inactive row; a later updateListingRegion activates it with the detected region (tests/shared-agents-listing.test.ts) — covering the null-region path feat: zero-trust agent region — observed, verified, never self-asserted #143 introduced.
  • Cosmetic: dead visibility field in tests/session-dispatch.test.ts replaced with the real dispatchTier.

Verification

  • 8 suites / 347 tests green (api, eval-agent-token-create, session-dispatch, practical-shared-agents-credits, shared-agents-listing, zero-trust-dispatch, zero-trust-region-api, zero-trust-metrics), incl. the previously-red legacy files.
  • npm run check clean; eslint 0 errors on touched files.
  • Independent diff review: approved, no critical/important findings.

Generated with SMT smt@agora.build

…lts recorded; legacy tests migrated

🤖 Built with SMT <smt@agora.build>
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

No findings.

Reviewed the PR diff against origin/main; the production change in server/routes.ts is consistent with the completion route’s existing assignment check and correctly allows claimed unverified-agent results to persist with siteId: null. The added tests cover the main regression path and the null-region listing lifecycle.

I did not run the test suite in this read-only/no-approval environment.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

I reviewed the merge ref (376e4b4...18eaa04), read the surrounding /complete handler, and traced the downstream consequences of letting eval_results.site_id be NULL for the first time.

Verdict

The core fix is correct and well-scoped. Swapping the recording gate from job.siteId != null to job.evalAgentId != null restores results from Unverified agents, and I confirmed the null-site row is handled everywhere it can surface:

  • eval_results.site_id is nullable (shared/schema.ts:441), and regionScopeCondition already has the scope.unverified → isNull(siteId) arm (server/storage.ts:1562); getAvailableRegions reports it as hasUnverified rather than mixing it into baseIds.
  • No public board can be polluted by a self-asserted region: communityConditions requires tokenDispatchTier IN ('public','shared') and locationTrust IN ('trusted','datacenter')/NULL, and effectiveDispatchIdentity stamps "unknown" for an Unverified agent (server/routes.ts:3341). So these rows land only in the owner's My Evals arm. Mainline stays non-null-site because a public-tier token can only be minted with a region (routes.ts:2882) and the PATCH path refuses region-less promotion (routes.ts:2975).
  • regionMetadata(null) and formatMetricsResults handle null; the only client formatSite(entry.siteId) without a guard is the leaderboard (client/src/pages/leaderboard.tsx:304), which is mainline-only and therefore still unreachable with a null site.

The shared-agents-listing test matches the already-shipped upsertListing/updateListingRegion semantics in plugins/shared-agents/server/repo.ts, and the visibilitydispatchTier migration in session-dispatch.test.ts matches the field the route actually reads.

Minor points

1. The new guard can never be false — server/routes.ts:3747. Twenty lines above, agentId is required non-empty and job.evalAgentId !== agentId 403s. So job.evalAgentId != null is always true at this point. Harmless and arguably defensive, but a reader may believe it's load-bearing; either drop it (keeping the comment) or note in the comment that it's redundant with the authorization check.

2. Behavior change on shared-tier settlement worth confirming. A shared job dispatched while its agent's listing was active, whose agent then loses its region before claiming (targeted claims are trust-exempt), now completes with a result → settle({hasResult: true}) pays where it previously refunded. The result is real, so paying is defensible, and the buyer can still see it on the job detail page — but it appears in no metrics tier (community excludes untrusted locationTrust; the My Evals third arm only covers private/team). Narrow, but if the intent is "buyer always gets something they can see in their dashboard," that arm may need shared added.

3. Test residue in the shared dev DB — tests/zero-trust-dispatch.test.ts afterAll. The token and workflow are cleaned up, but the job and its eval_results row are not (workflowId is ON DELETE SET NULL, so they survive). Every run leaves one synthetic 900 ms result permanently in admin's My Evals. Given the known gate hazard around leaked per-user resources (CLAUDE.md / #134), DELETE FROM eval_jobs WHERE id = jobId in afterAll (results cascade) would keep it self-contained.

4. Test robustness nits. providers[0].id and es[0].id are indexed without a guard — an empty list yields a confusing undefined failure rather than a clear one. Also, describeDb gates on DATABASE_URL, but this suite additionally needs a live server at TEST_BASE_URL; that matches the convention in session-dispatch.test.ts, so it's consistent, just not self-evidently skippable.

@guohai
guohai merged commit 49ea41e into main Sep 2, 2026
7 checks passed
@guohai
guohai deleted the fix/zero-trust-region-followups branch September 2, 2026 07:17
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