Add no-preflight bucket registration (v0.16.1)

[drernie]

Summary

• Add --no-preflight to quiltx bucket add and quiltx catalog acl so bucket registration can skip local S3/SNS owner setup and let the catalog stack probe via GraphQL.
• Add a GraphQL-only bucketAdd helper that preserves structured union error messages.
• Document the two registration paths and capture the no-preflight spec decisions.

Review fixes (v0.16.1)

• Document --no-preflight's no-prompt contract in --yes help, --no-preflight help, and the README example (spec/7-no-preflight.md §144). --yes is now explicitly unnecessary for bucket add --no-preflight.
• catalog acl --no-preflight --dry-run now lists the skipped local AWS steps (spec §140) instead of returning before no-preflight is evaluated.
• New CLI-boundary tests:
  • test_add_no_preflight_bucket_add_error_exits_nonzero — asserts BucketAddError from add_bucket_without_preflight is rendered to stderr with a non-zero exit.
  • test_add_no_preflight_does_not_prompt_without_yes — asserts the spec §144 no-prompt contract.
  • test_acl_tool_dry_run_no_preflight_lists_skipped_steps — covers the dry-run output for catalog acl --no-preflight --dry-run.

Validation

• ./poe lint — black + mypy clean.
• ./poe test — 333 passed, 1 skipped.
• push hooks: black, mypy, poe test.

Greptile Summary

This PR adds a --no-preflight flag to quiltx bucket add and quiltx catalog acl, providing a GraphQL-only registration path that mirrors the catalog admin UI — skipping local S3/SNS bucket-owner setup and letting the catalog stack probe with its own IAM. A new add_bucket_without_preflight library helper maps the full GraphQL union-error type to structured, human-readable messages.

• New quiltx/bucket.py helper (add_bucket_without_preflight, BucketAddError, _bucket_add_typename, _bucket_add_error_message) covers all documented union variants including BucketDoesNotExist, InsufficientPermissions, and SNS/validation errors.
• --no-preflight is threaded through _cmd_add → _cmd_add_no_preflight (bucket tool) and _run → apply_acl → _register_bucket_with_retry (ACL tool), with QUILTX_NO_PREFLIGHT=1 as a scripted override; the dry-run path lists the skipped local AWS steps.
• New CLI-boundary tests assert no-boto3 invocation, env-var activation, no-prompt contract, force remove+re-add, all error-union messages, and non-zero exit on BucketAddError.

Confidence Score: 4/5

Safe to merge; the no-preflight path is well-isolated and the full owner-setup path is unchanged.

The core logic — skipping local S3/SNS setup and delegating to the GraphQL union — is correct and well-tested. The three findings are all style/maintainability issues: a duplicated env-var check, a dry-run notice that fires when no bucket registrations are actually planned, and a stale pragma comment. None of them affect the happy path or the error path in production.

quiltx/tools/catalog/acl.py — env-var duplication and the no-changes dry-run notice condition; quiltx/acl.py — stale pragma on the exception block.

Important Files Changed

Filename	Overview
quiltx/bucket.py	Adds add_bucket_without_preflight, BucketAddError, and private helpers _bucket_add_typename, _field_value, _bucket_add_error_message. Logic is sound; union-error mapping covers all documented variants.
quiltx/tools/bucket.py	Adds --no-preflight CLI flag, _env_flag helper, _cmd_add_no_preflight handler, and _print_no_preflight_dry_run rich table; no-preflight path is correctly gated behind the outer try-except in _cmd_add.
quiltx/acl.py	Threads no_preflight through apply_acl and _register_bucket_with_retry; the no-preflight branch passes control_account_id or "" but that value is ignored inside _register_bucket_with_retry when no_preflight=True, which is harmless but slightly misleading.
quiltx/tools/catalog/acl.py	Adds --no-preflight to the ACL tool; env-var check is inlined instead of reusing _env_flag, and the dry-run notice is shown even when no changes exist (buckets_to_add is empty).
tests/test_bucket.py	Good new coverage: no-boto3 path, env-var activation, no-prompt contract, force remove+re-add, all union error variants, BucketAddError exit code, and no-prompt spec assertion.
tests/test_acl.py	New tests cover no-preflight threading, failure non-rollback, dry-run notice output, policy-drift reapply with no_preflight preserved; stale # pragma: no cover in the exception block it now exercises.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["quiltx bucket add BUCKET"] --> B{no-preflight flag\nor env var?}
    B -- No --> C[bucket-owner path\nresolve session\nmerge bucket policy\nensure SNS topic\nGraphQL bucketAdd with SNS ARN]
    B -- Yes --> D[_cmd_add_no_preflight]
    D --> E{dry-run?}
    E -- Yes --> F[print skipped AWS steps\nreturn 0]
    E -- No --> G{already registered?}
    G -- Yes and force --> H[admin.buckets.remove]
    G -- Yes no force --> I[print already-registered\nreturn 0]
    H --> J[add_bucket_without_preflight\nGraphQL bucketAdd\nsns_notification_arn is None]
    G -- No --> J
    J --> K{GraphQL typename}
    K -- BucketAddSuccess --> L[return AddBucketResult\nalready_registered=False]
    K -- BucketAlreadyAdded --> M[return AddBucketResult\nalready_registered=True]
    K -- error typename --> N[raise BucketAddError\nhuman-readable message]
    N --> O[print to stderr\nreturn 1]

Loading

Comments Outside Diff (1)

1. quiltx/acl.py, line 656-659 (link)

   Stale # pragma: no cover after new test

   The except Exception block here is now exercised by test_apply_acl_no_preflight_failure_does_not_rollback, which injects a RuntimeError through the mocked add_bucket_without_preflight. The pragma was correct before this PR, but keeping it means the coverage report will under-report actual test coverage and the comment "external API surface" no longer accurately describes why the branch exists.

_{Reviews (1): Last reviewed commit: "CHANGELOG: 0.16.1" | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[Copilot]

Pull request overview

This PR introduces a “GraphQL-only” bucket registration path that skips local AWS S3/SNS preflight/setup, allowing bucket registration to rely on the catalog stack’s IAM probe (matching admin UI behavior). It adds --no-preflight support to both quiltx bucket add and quiltx catalog acl, plus supporting docs/spec and CLI-boundary tests.

Changes:

• Add --no-preflight to bucket registration flows (bucket add, catalog acl) with an env-var override (QUILTX_NO_PREFLIGHT=1).
• Implement a GraphQL-only add_bucket_without_preflight() helper with structured union-result error mapping.
• Add/extend tests and documentation covering no-preflight behavior, dry-run output, and non-zero exits on bucketAdd errors.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
uv.lock	Bumps locked quiltx version to 0.16.1.
pyproject.toml	Bumps project version to 0.16.1.
quiltx/_version.py	Bumps runtime version string to 0.16.1.
quiltx/bucket.py	Adds GraphQL-only add_bucket_without_preflight() and bucketAdd union error mapping helpers.
quiltx/tools/bucket.py	Adds --no-preflight CLI flag, env-var handling, and a no-preflight execution/dry-run path.
quiltx/acl.py	Threads no_preflight through ACL bucket registration to use GraphQL-only path.
quiltx/tools/catalog/acl.py	Adds --no-preflight, env-var fallback, and dry-run “skipped steps” notice.
tests/test_bucket.py	Adds CLI-boundary tests for no-preflight bucket add, env var behavior, and union error messaging.
tests/test_acl.py	Adds tests ensuring no-preflight routes through GraphQL-only registration and dry-run notice output.
README.md	Documents when/why to use --no-preflight and provides examples.
README_DEV.md	Documents the two registration paths for developers.
spec/7-no-preflight.md	Adds a detailed design/spec document for no-preflight behavior and decisions.
CHANGELOG.md	Adds 0.16.1 release notes for no-preflight.
AGENTS.md	Updates tool docs to mention --no-preflight behavior.

Comments suppressed due to low confidence (1)

quiltx/tools/bucket.py:391

• The --no-prompt guard doesn’t currently honor the “(or --dry-run)” part of the error message: quiltx bucket add --dry-run --no-prompt will still fail unless --yes is provided (since args.dry_run isn’t checked here). Update the condition to allow --no-prompt when --dry-run is set (and keep the existing --no-preflight exemption).

        no_preflight = bool(args.no_preflight or _env_flag("QUILTX_NO_PREFLIGHT"))
        if (
            getattr(args, "no_prompt", False)
            and not getattr(args, "yes", False)
            and not no_preflight
        ):
            print(
                "Error: --no-prompt requires --yes (or --dry-run) to avoid interactive prompts.",
                file=sys.stderr,
            )

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.