Skip to content

fix: detect public apps without --public flag and filter shared slugs by app-set - #1128

Merged
waynesun09 merged 2 commits into
mainfrom
fix-public-app-detection
May 19, 2026
Merged

fix: detect public apps without --public flag and filter shared slugs by app-set#1128
waynesun09 merged 2 commits into
mainfrom
fix-public-app-detection

Conversation

@waynesun09

Copy link
Copy Markdown
Member

Summary

  • Always check GetAppClientID before the manifest flow regardless of --public flag. When a public app exists and --public was not passed, prompt the user to confirm installation instead of silently falling through to create a duplicate via the manifest flow.
  • Apply filterSlugsByAppSet to shared slugs from copySharedAppPEMs so cross-app-set installs don't shadow each other (e.g. nonflux-triage no longer blocks fullsend-ai-triage).
  • After recoverCreatedApp + ensureInstalled, resolve AppID from the installation so ROLE_APP_IDS is updated correctly.

Test plan

  • Unit tests for public app detection with --public flag (auto-install, no prompt)
  • Unit tests for public app detection without --public flag (user confirms → installs)
  • Unit tests for public app detection without --public flag (user declines → error with guidance)
  • Unit test for transient API error propagation
  • Unit tests for filterSlugsByAppSet on shared slugs
  • go vet clean
  • E2E: fullsend admin install-repo with --app-set fullsend-ai without --public against an org with existing public apps

…nd detect public apps without --public

- Apply filterSlugsByAppSet to shared slugs so cross-app-set installs don't
  shadow each other (e.g. nonflux-triage no longer blocks fullsend-ai-triage).
- After recoverCreatedApp + ensureInstalled, resolve AppID from the
  installation so ROLE_APP_IDS is updated correctly.
- Always check GetAppClientID before the manifest flow regardless of --public
  flag. When a public app exists and --public was not passed, prompt the user
  to confirm installation instead of silently falling through to create a
  duplicate via the manifest flow.

Signed-off-by: Wayne Sun <gsun@redhat.com>
@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown

Site preview

Preview: https://d846cb82-site.fullsend-ai.workers.dev

Commit: a9a419eb2566970f63c7871364e99b02f3ea94d3

@fullsend-ai-review

fullsend-ai-review Bot commented May 19, 2026

Copy link
Copy Markdown

Review

Findings

No findings.

Previous run

Review

Findings

No findings.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label May 19, 2026
Address review findings:
- Propagate findExistingInstallation errors in recover path instead of
  silently swallowing them, matching the public-app path's behavior.
- Replace misleading "public app" terminology with "existing app" in
  prompts shown when --public is not set.
- Use StepInfo instead of StepDone before user confirmation to avoid
  showing a success checkmark before the user has decided.
- Add tests: recoverCreatedApp AppID resolution through Run(), prompter
  error propagation, and --public flag skips prompt assertion.

Signed-off-by: Wayne Sun <gsun@redhat.com>
@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels May 19, 2026
@waynesun09
waynesun09 added this pull request to the merge queue May 19, 2026
Merged via the queue into main with commit 385f096 May 19, 2026
7 of 8 checks passed
@waynesun09
waynesun09 deleted the fix-public-app-detection branch May 19, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant