feat: add Fly sync target and CLI commands by vieiralucas · Pull Request #86 · faiscadev/zopp

vieiralucas · 2026-03-11T03:26:47Z

Summary

Implement FlySyncTarget in zopp-sync crate with FlyApi trait abstraction for testability
Add zopp sync fly --app <name> and zopp diff fly --app <name> CLI commands following the established AWS pattern
Handle Fly's write-only secrets constraint: fetch_current() returns labels with empty-string values since Fly API doesn't expose secret values
Include retry with exponential backoff for 429/5xx errors, URL percent-encoding for path safety, and token zeroization

Key Design Decisions

Write-only secrets: Fly API returns only secret labels, not values. Diff can detect add/remove but not value changes. Users should use --force to ensure all values are current.
Batch operations: Adds/updates batched into single set_secrets API call; removes processed individually via unset_secret
Task 3 deferred: Sync status integration deferred because current sync status is AWS-specific (requires --region)

Test Plan

8 unit tests for FlySyncTarget (fetch, apply add/remove/update, auth error, empty state, mixed errors)
6 unit tests for FlyClient (URL encoding, retryable status codes)
5 CLI parsing tests (sync/diff flag parsing, required --app validation)
cargo fmt --check clean
cargo clippy --workspace --all-targets --all-features clean
All non-Postgres tests pass

Summary by cubic

Adds a Fly sync target and CLI commands to diff and sync zopp secrets with Fly apps for Story 3.1. Sync now always overwrites overlapping keys to handle Fly’s write‑only secrets, with retries, TLS hardening, and a small CI timeout tweak.

New Features
- Added FlySyncTarget in zopp-sync with a FlyApi client; new commands: zopp sync fly --app <name> and zopp diff fly --app <name>.
- Write‑only secrets: fetch returns labels only. Diff shows adds/updates/removes; sync always overwrites existing keys (no --force needed).
- Reliability/security: exponential backoff for 429/5xx, percent‑encoded paths, FLY_API_TOKEN zeroized in memory, reqwest via rustls-tls with webpki-roots, bumped quinn-proto to 0.11.14, and increased Playwright dev server timeout to 5m for CI.
Migration
- Set FLY_API_TOKEN (create at https://fly.io/user/personal_access_tokens).
- Run zopp diff fly --app <name> to preview, then zopp sync fly --app <name> to apply.

^{Written for commit 87bb800. Summary will update on new commits.}

…arned patterns Update architecture doc with FetchResult type, DiffCommonArgs split, error classification rules, and 3 new anti-patterns from Epic 2 retrospective. Add sprint change proposal documenting the analysis.

Implement FlySyncTarget in zopp-sync crate with FlyApi trait abstraction for testability. Wire up `zopp sync fly --app <name>` and `zopp diff fly --app <name>` CLI commands following the established AWS pattern. Fly secrets are write-only (API returns labels but not values), so fetch_current returns empty-string values. The diff engine detects add/remove by key presence; use --force to push all values. Includes unit tests for FlySyncTarget (mock-based) and CLI parsing tests for both sync and diff Fly subcommands.

…Fly sync - Add send_with_retry() with exponential backoff for 429/5xx responses (max 3 retries, 1s/2s/4s delays) to satisfy AC7 - Percent-encode app names and secret labels in URL paths to prevent path injection with special characters - Wrap FLY_API_TOKEN in ZeroizeOnDrop struct per project security standards - Add unit tests for URL encoding and retryable status classification

cubic-dev-ai

2 issues found across 16 files

Confidence score: 3/5

There is concrete regression risk in apps/zopp-cli/src/commands/sync_fly.rs: sync fly currently ignores --force, so update operations run unconditionally and can rewrite secrets on every execution.
apps/zopp-cli/src/commands/diff_fly.rs can under-report changes when zopp values are empty strings, which means diff fly may miss required updates for write-only Fly targets.
Given both issues are medium severity (6/10) with high confidence, this is a meaningful user-facing behavior risk rather than a minor housekeeping concern.
Pay close attention to apps/zopp-cli/src/commands/sync_fly.rs and apps/zopp-cli/src/commands/diff_fly.rs - force-flag handling and empty-value diff logic can cause incorrect update behavior.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/zopp-cli/src/commands/sync_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/sync_fly.rs:121">
P2: `sync fly` ignores `--force` and always includes update operations, causing unconditional secret rewrites on every run.</violation>
</file>

<file name="apps/zopp-cli/src/commands/diff_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/diff_fly.rs:121">
P2: `diff fly` can miss updates for keys whose zopp value is empty, because Fly placeholder values are also empty strings. For write-only targets, overlapping keys should be treated as updates regardless of value equality.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

…inux

cubic-dev-ai

2 issues found across 16 files

Confidence score: 3/5

There is concrete regression risk in apps/zopp-cli/src/commands/sync_fly.rs: sync fly currently ignores --force, so update operations run unconditionally and can rewrite secrets on every execution.
apps/zopp-cli/src/commands/diff_fly.rs can under-report changes when zopp values are empty strings, which means diff fly may miss required updates for write-only Fly targets.
Given both issues are medium severity (6/10) with high confidence, this is a meaningful user-facing behavior risk rather than a minor housekeeping concern.
Pay close attention to apps/zopp-cli/src/commands/sync_fly.rs and apps/zopp-cli/src/commands/diff_fly.rs - force-flag handling and empty-value diff logic can cause incorrect update behavior.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/zopp-cli/src/commands/sync_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/sync_fly.rs:121">
P2: `sync fly` ignores `--force` and always includes update operations, causing unconditional secret rewrites on every run.</violation>
</file>

<file name="apps/zopp-cli/src/commands/diff_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/diff_fly.rs:121">
P2: `diff fly` can miss updates for keys whose zopp value is empty, because Fly placeholder values are also empty strings. For write-only targets, overlapping keys should be treated as updates regardless of value equality.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

vieiralucas · 2026-03-11T03:39:39Z

@cubic-dev-ai Please do a full re-review of the PR.

cubic-dev-ai · 2026-03-11T03:39:46Z

@cubic-dev-ai Please do a full re-review of the PR.

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

3 issues found across 17 files

Confidence score: 2/5

There is a high regression risk in crates/zopp-sync/src/fly/client.rs: targeting /secrets instead of documented /secretkeys is likely to break Fly sync/diff behavior against the live API.
apps/zopp-cli/src/commands/sync_fly.rs has a concrete behavior bug where --force is ignored, so existing Fly keys are always reapplied, which can cause unexpected repeated updates.
apps/zopp-cli/src/commands/diff_fly.rs can miss real changes when desired values are empty strings, because comparison against Fly placeholder "" values hides updates; this is user-facing diff accuracy risk.
Pay close attention to crates/zopp-sync/src/fly/client.rs, apps/zopp-cli/src/commands/sync_fly.rs, apps/zopp-cli/src/commands/diff_fly.rs - endpoint mismatch and sync/diff correctness issues should be resolved before relying on this behavior.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/zopp-cli/src/commands/sync_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/sync_fly.rs:121">
P2: `--force` is ignored: existing Fly keys are always treated as updates and reapplied on every sync.</violation>
</file>

<file name="apps/zopp-cli/src/commands/diff_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/diff_fly.rs:121">
P2: `diff fly` can miss updates for keys whose desired value is empty string because it relies on value equality against Fly’s placeholder `""` values.</violation>
</file>

<file name="crates/zopp-sync/src/fly/client.rs">

<violation number="1" location="crates/zopp-sync/src/fly/client.rs:182">
P1: The client targets `/secrets` endpoints, but current Machines API docs expose app secret management under `/secretkeys`; this likely breaks Fly sync/diff behavior against the live API.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-11T03:45:45Z

+impl FlyApi for FlyClient {
+    async fn list_secrets(&self, app: &str) -> Result<Vec<SecretEntry>, SyncError> {
+        let app_encoded = encode_path_segment(app);
+        let url = format!("{BASE_URL}/v1/apps/{app_encoded}/secrets");


P1: The client targets /secrets endpoints, but current Machines API docs expose app secret management under /secretkeys; this likely breaks Fly sync/diff behavior against the live API.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/zopp-sync/src/fly/client.rs, line 182: <comment>The client targets `/secrets` endpoints, but current Machines API docs expose app secret management under `/secretkeys`; this likely breaks Fly sync/diff behavior against the live API.</comment> <file context> @@ -0,0 +1,306 @@ +impl FlyApi for FlyClient { + async fn list_secrets(&self, app: &str) -> Result<Vec<SecretEntry>, SyncError> { + let app_encoded = encode_path_segment(app); + let url = format!("{BASE_URL}/v1/apps/{app_encoded}/secrets"); + + let resp = send_with_retry(|| self.http.get(&url).bearer_auth(&self.token.0)).await?; </file context>

…rets Fly API returns empty-string values for secrets. Without filtering, the diff engine treats all overlapping keys as Updates. Now: - sync fly: only applies Updates when --force is set - diff fly: filters out phantom Updates entirely

vieiralucas · 2026-03-11T03:50:55Z

@cubic-dev-ai Please do a full re-review of the PR.

cubic-dev-ai · 2026-03-11T03:51:02Z

@cubic-dev-ai Please do a full re-review of the PR.

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 17 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

vieiralucas · 2026-03-11T17:25:30Z

+    // Note: Fly secrets are write-only, so values from fetch are empty strings.
+    // Diff can reliably detect add/remove but not value changes.
+    // Filter out Update ops since they're phantom diffs (empty vs actual value).
+    // Users should use `zopp sync fly --force` to push all values.


i think we should just overwrite by default instead

Fly secrets are write-only so we can't compare values. Instead of filtering out updates and requiring --force, always treat overlapping keys as updates to ensure Fly has current values.

vieiralucas · 2026-03-11T17:32:23Z

Addressed review findings in 87bb800:

diff_fly.rs / sync_fly.rs: Now always includes updates for Fly's write-only secrets — no more filtering. Overlapping keys are always treated as updates to ensure Fly has current values.
client.rs /secrets endpoint (Cubic P1): Not addressing — verified against Fly docs and community posts that the Machines API uses /v1/apps/{app}/secrets, not /secretkeys. The code is correct.

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/zopp-cli/src/commands/sync_fly.rs">

<violation number="1" location="apps/zopp-cli/src/commands/sync_fly.rs:121">
P1: `sync fly` no longer respects `--force`: all overlapping secrets are treated as updates and applied unconditionally.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-03-11T17:37:05Z

+    // Note: Fly secrets are write-only (values are empty strings from fetch).
+    // All overlapping keys show as updates since we can't compare values.
+    // This is correct behavior — we always overwrite to ensure Fly has current values.
+    let operations = zopp_sync::diff(&zopp_map, &fetch_result.secrets);


P1: sync fly no longer respects --force: all overlapping secrets are treated as updates and applied unconditionally.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/zopp-cli/src/commands/sync_fly.rs, line 121: <comment>`sync fly` no longer respects `--force`: all overlapping secrets are treated as updates and applied unconditionally.</comment> <file context> @@ -116,18 +116,9 @@ pub async fn cmd_sync_fly( - }; + // All overlapping keys show as updates since we can't compare values. + // This is correct behavior — we always overwrite to ensure Fly has current values. + let operations = zopp_sync::diff(&zopp_map, &fetch_result.secrets); // 6. Dry-run or no changes: show diff output </file context>

Suggested change

let operations = zopp_sync::diff(&zopp_map, &fetch_result.secrets);

let all_operations = zopp_sync::diff(&zopp_map, &fetch_result.secrets);

let operations: Vec<DiffOperation> = if common.force {

all_operations

} else {

all_operations

.into_iter()

.filter(|op| !matches!(op, DiffOperation::Update { .. }))

.collect()

};

vieiralucas · 2026-03-11T19:11:23Z

Cubic incremental review flagged that --force is no longer respected in sync fly. This is intentional — Fly secrets are write-only (API returns labels only, never values), so we can never detect real value changes. Always overwriting is the correct behavior for a write-only target.

vieiralucas added 7 commits March 10, 2026 23:57

chore: mark story 3.1 in-progress, add story spec

f18bb03

chore: update story 3.1 with completion status

de3325d

chore: update story 3.1 tracking to review phase

092edf0

chore: update Cargo.lock for fly feature dependencies

b7421b4

cubic-dev-ai bot reviewed Mar 11, 2026

View reviewed changes

Comment thread apps/zopp-cli/src/commands/sync_fly.rs

Comment thread apps/zopp-cli/src/commands/diff_fly.rs

fix: use rustls-tls for reqwest to fix cross-compilation on aarch64-l…

fc23bc5

…inux

cubic-dev-ai bot reviewed Mar 11, 2026

View reviewed changes

Comment thread apps/zopp-cli/src/commands/sync_fly.rs

Comment thread apps/zopp-cli/src/commands/diff_fly.rs

cubic-dev-ai bot reviewed Mar 11, 2026

View reviewed changes

vieiralucas added 3 commits March 11, 2026 00:54

chore: mark story 3.1 as completed

8539614

fix: update quinn-proto to 0.11.14 (RUSTSEC-2026-0037)

443e0ed

fix: increase web-e2e server timeout to 5 minutes for CI

ada4629

vieiralucas commented Mar 11, 2026

View reviewed changes

fix: always overwrite Fly secrets instead of filtering updates

87bb800

Fly secrets are write-only so we can't compare values. Instead of filtering out updates and requiring --force, always treat overlapping keys as updates to ensure Fly has current values.

cubic-dev-ai bot reviewed Mar 11, 2026

View reviewed changes

vieiralucas merged commit e3aea52 into main Mar 11, 2026
19 checks passed

vieiralucas deleted the feat/3.1-fly-sync-target branch March 11, 2026 19:47

-    let operations = zopp_sync::diff(&zopp_map, &fetch_result.secrets);
+    let all_operations = zopp_sync::diff(&zopp_map, &fetch_result.secrets);
+    let operations: Vec<DiffOperation> = if common.force {
+        all_operations
+    } else {
+        all_operations
+            .into_iter()
+            .filter(|op| !matches!(op, DiffOperation::Update { .. }))
+            .collect()
+    };

Conversation

vieiralucas commented Mar 11, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Design Decisions

Test Plan

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vieiralucas commented Mar 11, 2026

Uh oh!

cubic-dev-ai bot commented Mar 11, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vieiralucas commented Mar 11, 2026

Uh oh!

cubic-dev-ai bot commented Mar 11, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

vieiralucas Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

vieiralucas commented Mar 11, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vieiralucas commented Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vieiralucas commented Mar 11, 2026 •

edited by cubic-dev-ai bot

Loading

cubic-dev-ai bot Mar 11, 2026 •

edited

Loading

cubic-dev-ai bot Mar 11, 2026 •

edited

Loading