Skip to content

fix(fabricator): do not resubmit reused registry records that are already approved or pending - #400

Merged
gibbsie merged 1 commit into
mainfrom
fix/fabricator-reuse-approved-records
Oct 26, 2026
Merged

gibbsie merged 1 commit into
mainfrom
fix/fabricator-reuse-approved-records

Conversation

@gibbsie

@gibbsie gibbsie commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

fix(fabricator): do not resubmit reused registry records that are already approved or pending

Problem

When _find_existing_record_id reuses a Registry record whose status is already APPROVED (or PENDING_APPROVAL), the code unconditionally calls SubmitRegistryRecordForApproval. The registry rejects this with ValidationException: Cannot submit the record for approval in current status: APPROVED. This is counted as a tool registration failure, causing the entire fabrication job to report FAILED even though every record already exists and is usable.

Root cause

The _approve helper inside store_tool_config_registry always called submit_registry_record_for_approval without first reading the record's current status. On a reuse path (SQS redelivery, retry, or "Check build status" re-trigger), the record is already approved, and the blind re-submission fails.

The agent path (store_agent_config_registry) had no status checking at all for reused records — it returned True immediately without verifying the record's lifecycle state.

Fix

_approve in store_tool_config_registry: reads the record's current status via GetRegistryRecord before attempting submission. State table:

Current status Action
APPROVED Return 'approved' immediately — no submit call
PENDING_APPROVAL Return 'pending_approval' immediately — no submit call
DRAFT Submit for approval, map the result
CREATING Falls through to submit; existing CREATING-recovery path handles it
REJECTED / DEPRECATED Raise RuntimeError — terminal, non-recoverable

store_agent_config_registry: when reusing a record, reads its status and applies the same state table. DRAFT records are submitted for approval. REJECTED/DEPRECATED records fail clearly. A reused record's content is NOT rewritten (content writes demote non-DRAFT records).

Overall job status: COMPLETED when every tool and the agent are approved or pending; registryApprovalStatus = 'approved' iff all approved, else 'pending_approval'.

Stack

  • arbiter/fabricator/index.py — _approve() status pre-read; store_agent_config_registry reuse path status handling

Tests

New test file test_reused_record_status.py (10 tests) using botocore.stub.Stubber with the real agent-registry-control service model — no hand-copied expected params. Covers:

  • Tool path: reused APPROVED (no submit), reused PENDING_APPROVAL (no submit + tracked in run_state), reused DRAFT (submit called), reused REJECTED (raises), new record DRAFT (get→submit)
  • Agent path: reused APPROVED (no submit), reused PENDING_APPROVAL (no submit), reused DRAFT (submit called), reused REJECTED (raises)

Updated 2 existing test files (test_creating_conflict_recovery.py, test_registry_filter_approval_idempotency.py) to account for the new GetRegistryRecord pre-read in _approve.

Full CI: pytest arbiter/ -v --tb=short → 2523 passed, 1 skipped, 0 failed.

Smoke test

"Check build status" on agents that were previously built should show them as built (COMPLETED) without re-triggering approval submissions. Retrying a fabrication where the records already exist should complete without ValidationException errors.

import sys
import os
import json
from unittest.mock import patch, MagicMock
@gibbsie
gibbsie merged commit 6bbfa1a into main Oct 26, 2026
15 checks passed
@gibbsie
gibbsie deleted the fix/fabricator-reuse-approved-records branch October 26, 2026 02:33
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