fix(deploy): make GitHub/GitLab app deploy work (Source Config, private repos, token re-validate)

[cweibel]

Problem

Several issues in the Deploy Application wizard's git (GitHub/GitLab) path made it unusable end-to-end:

1. Source Config step showed no options and the Next button stayed disabled, so a git deploy could not proceed.
2. When it was forced through, the resulting git clone failed with exit 128 (wrong branch).
3. Private repositories failed ("Project does not exist") when a github.com/gitlab.com endpoint was also registered.
4. A token added after the project name never re-validated.

Root-caused and fixed each below. All changes have unit tests; eslint is clean.

Changes

1. Build the commit list on step activation, not @ViewChild init

The Source Config step (step2_1, the commit picker) is rendered via [hidden], so Angular instantiates it up-front. The step2_1Ref @ViewChild setter called onEnter() at that instant — before step 2's submit had saved a git source — so the commit-list wrapper resolved an empty applicationSource, never fetched commits, and the step stayed invalid (no options, Next disabled). Moved the activation work to step2_1Handle.onEnter, which the stepper fires on real navigation into the step, after step 2 submits. This is the same @ViewChild-fires-once trap already fixed for step2_2/step3 (#5600).

This also resolves the exit 128 clone: the wizard had been carrying a wrong branch (e.g. main on a master-default repo → Remote branch main not found) because the branch was never resolved from the repo's real default.

2. Use the typed token, not a registered endpoint, for private repos

A separately-registered github.com/gitlab.com endpoint tags the source type with its guid. In Private/Enterprise access mode the user supplies a Personal Access Token directly in the form, but project-exists validation + repo/branch/commit lookups were still built against the registered endpoint guid — so requests proxied through the endpoint's stored creds (/api/v1/proxy/<guid>/...) and ignored the typed token. A private repo the token could see returned 404 ("Project does not exist"). Now only Public mode uses the registered endpoint; Private/Enterprise talk to the SCM API directly with the typed token.

3. Re-validate when the access token changes

An async validator only re-runs when its own control (the project name) changes. Adding a PAT after the name was entered didn't re-check (and the cache was keyed on name only), leaving a stale unauthenticated "does not exist". The directive now implements OnChanges + registerOnValidatorChange: an auth-context (scm/guid/token) change clears the cached result and re-triggers validation. Token parsing is also comma-safe.

4. Clearer repository-not-found message

GET /repos/<owner>/<repo> 404s both for "does not exist" and for a private repo the token isn't authorized for (GitHub 404s rather than 403s to avoid leaking existence). Reworded the flat "Project does not exist" to point at exact (case-sensitive) owner/name and, for private repos, token authorization.

Verification

• Unit tests added/updated across deploy-application.component, deploy-application-step2.component, and github-project-exists.directive; all pass under vitest --project=cloud-foundry. eslint clean.
• Validated on a live dev deployment:
  • Public GitHub repo deploy: works.
  • Private GitHub repo with a classic PAT (repo scope, SSO-authorized): validates and deploys end-to-end.
  • Confirmed the Source Config regression reproduces on current develop and is resolved by this branch.

Note: fine-grained PATs must have the target repo in their selected-repositories grant (and org approval) or GitHub returns 404 — this is expected GitHub behavior, not a Stratos issue; the improved message (change 4) now hints at it.

---

Implemented with AI assistance (OpenCode); reviewed by a human contributor. Commits are DCO signed-off.

[norman-abramovitz]

Went through all four commits and verified the mechanisms rather than taking the commit messages at their word. The diagnoses hold up well, and each fix is aimed at the right layer:

• GitSCMService.getSCM returns a fresh instance per call (scm.service.ts:27) and BaseSCM.getEndpoint short-circuits to of(undefined) on an empty guid, falling back to getPublicApi(). So passing '' in Private/Enterprise really does bypass the proxy and use the typed token, and there's no shared-instance leakage between modes.
• Relaxing the endpoint-guid requirement in onNext is safe on the backend: deploy.go:442 only resolves a CNSI record if len(info.EndpointGUID) != 0, and the clone picks up AccessToken independently at deploy.go:667. The frontend was gating a path jetstream already supported.
• The step2_1 change matches the existing step2_2Ref precedent in the same file, and steppers.component.ts routes every setActive through pOnEnter → signalHandle.onEnter, not just the first step. Right hook.
• registerOnValidatorChange is genuinely wired up for async validators (@angular/forms, setUpValidators), so the re-validate trigger works as described.

The one thing I'd like changed

ngOnChanges calls deployData.projectDoesntExist('') unconditionally, including on the very first change. On the redeploy path that discards state the wizard cannot rebuild.

1. gitscm-tab.component.ts:332 seeds checkProjectExists(scm, projectName) just before navigating to /applications/deploy. CfDeployAppDataService is providedIn: 'root', so that result carries into the wizard.
2. The directive's first ngOnChanges always fires with lastAuthContext === '' against something like 'github,<guid>,' — never equal — so it resets projectExists to { name: '', exists: false }.
3. On redeploy the project input is [disabled]="isRedeploy", and a disabled control skips its async validator (updateValueAndValidity only runs validators when enabled). Nothing re-checks, so the reset is permanent.

Worth noting this fires even when step 2 is not the active step. I assumed at first that step content was instantiated lazily — the comment above setActive in steppers.component.ts says as much — but that isn't what happens. <ng-content> inside <ng-template> doesn't defer construction in Ivy. I checked with a throwaway spec putting a directive with an @Input inside a nested @if in a non-active step, and its ngOnChanges ran on first render. So the reset lands during the initial wizard render regardless of where the stepper starts. (Independent of this PR, that steppers.component.ts comment is misleading and its accompanying spec builds StepComponent objects by hand, so it never actually exercises projection. The deferred onEnter delivery is still correct, just not for the stated reason.)

The visible effect is confined: projectInfo$ maps to null and the repository info card — avatar, full name, description — disappears for the rest of the redeploy, on the step whose own heading is "Review source details". The deploy itself still completes; repositoryBranches$ filters on exists upstream of its switchMap, so an in-flight getBranches isn't cancelled and repositoryBranch survives. Whether the card is lost at all depends on which resolves first, the seeded repo lookup or canDeployType$, so it will look intermittent.

An isFirstChange() guard covers it without weakening the fix — the case you're targeting, a token entered after the project name, is by definition never the first change:

ngOnChanges(changes: SimpleChanges): void {
  const change = changes.appGithubProjectExists;
  if (!change) { return; }
  const auth = this.appGithubProjectExists ?? '';
  if (change.isFirstChange()) { this.lastAuthContext = auth; return; }
  if (auth === this.lastAuthContext) { return; }
  this.lastAuthContext = auth;
  this.deployData.projectDoesntExist('');
  this.onChange?.();
}

The spec currently pins the opposite behaviour (expect(reRuns).toBe(1) for the first-change block), so that expectation would become 0. Worth deciding whether that assertion was describing intended behaviour or just recording what the code happened to do.

Smaller points

The new not-found copy also shows while the project name is still being typed. validate() returns githubProjectDoesNotExist: true for anything failing isValidProjectName, and the template renders the error with no dirty/touched gate, so "Repository not found. Check the owner/name is exact (it is case-sensitive), and for a private repository make sure your access token is authorized for it." appears from the first keystroke and stays until the name is complete — owner/re still trips it, since the check wants more than two characters after the slash. The conflation predates this PR, but the longer text makes it far more prominent. A distinct error key for "not a complete owner/repo yet" would be the proper fix; short of that, wording that doesn't imply a lookup already happened would read better.

res.slice(2).join(',') is reasonable hardening, though for what it's worth GitHub (ghp_, github_pat_) and GitLab tokens are base62 plus -/_, so a comma shouldn't arise in practice.

endpointGuid: endpointGuid ?? '' in the saveAppDetails call is now a no-op — the public branch is already guarded non-empty just above, and the other branch is a literal ''.

The two setGitMode specs assert with toHaveBeenCalledWith, which matches any call, while the comment says "Last getSCM call should target...". Fine as it stands, but the comment promises something stronger than the assertion does.

[cweibel]

Thanks for the thorough review — and especially for verifying the mechanisms directly rather than trusting the commit messages.

Fixed the ngOnChanges issue in 90636c2:

• lastAuthContext now starts as undefined (not ''), so the first change only records the baseline and does not clear projectExists or re-run validation. Only a subsequent genuine auth-context change (e.g. a PAT typed after the project name) clears the cached result and re-validates.
• This preserves the redeploy seed: gitscm-tab.component.ts pre-seeds checkProjectExists() before navigating, CfDeployAppDataService is providedIn:'root', and the disabled project input on redeploy never re-runs its async validator — so the earlier unconditional reset was permanently wiping the repo info card. It no longer does.
• Added a regression test (does not clear seeded projectExists state on the first change) and corrected the earlier test that wrongly asserted a re-run on the first change.

You're also right about the steppers.component.ts comment — projected <ng-content> inside <ng-template> is not lazily instantiated in Ivy; directives/@Inputs (and their ngOnChanges) run on first render regardless of the active step. I corrected that comment to describe the real reason for the afterNextRender deferral (ordering relative to the active-step transition, so onEnter side effects don't see stale state), rather than lazy construction. I left the misleading pre-existing spec alone since reworking it is out of scope for this PR, but happy to file a follow-up to make it actually exercise projection.

vitest --project=cloud-foundry on the directive + step2 specs is green; eslint clean.

[norman-abramovitz]

The first-change guard does what's needed, and the undefined sentinel is a better shape than the isFirstChange() I suggested — it also separates "first render" from "token later cleared to empty", which my version would have conflated.

I checked the destroy/recreate path as well, since a fresh directive also starts with lastAuthContext === undefined: switching the source type away from git and back constructs a new directive that skips the clear, but this.scm is still GitHub and this.repository is unchanged, so the retained projectExists stays consistent with both. No gap there.

The new test earns its place — spying on projectDoesntExist and asserting it isn't called on the first change is the actual defect rather than a proxy for it. Good that the earlier expectation was corrected instead of left quietly passing.

One correction on the steppers.component.ts comment, since I'm the one who sent you down that path. The replacement says a synchronous pOnEnter "would run before the entering step is marked active", but s.active = i === index and currentIndex.set(index) both execute immediately above it, so the step is already active by then. The accurate half is the rest of the sentence: the state is set, but change detection hasn't re-rendered the outlet yet. Worth trimming to just that.

Three small things are still open from the earlier review, all in files this PR already touches — no objection to folding them in here rather than tracking them separately:

• endpointGuid: endpointGuid ?? '' in the saveAppDetails call is a no-op; the public branch is already guarded non-empty above it and the other branch is a literal ''.
• The two setGitMode specs assert with toHaveBeenCalledWith, which matches any call, while the comment says "Last getSCM call should target...".
• The comment wording above.

The remaining point I'd rather see as its own change than bolted onto this PR: validate() returns githubProjectDoesNotExist for any name failing isValidProjectName, and the template renders that error with no dirty/touched gate, so the new message appears from the first keystroke and owner/re still trips it. It predates this PR, and the proper fix — a distinct error key for "not a complete owner/repo yet" — carries its own template and test changes. Better as a separate change.

And yes, please do file the follow-up for the stepper spec. It builds StepComponent objects by hand, so it never exercises projection at all — which is why the incorrect comment survived as long as it did.

To be clear on status: the blocking issue is resolved, so the outstanding "changes requested" is only about the three one-liners above. Push those and I'll approve — no need to open anything separate for them, we'll keep it on this PR.

[norman-abramovitz]

LGTM