Skip to content

app graph design#11712

Open
nithyatsu wants to merge 19 commits intoradius-project:github-demofrom
nithyatsu:design-with-code
Open

app graph design#11712
nithyatsu wants to merge 19 commits intoradius-project:github-demofrom
nithyatsu:design-with-code

Conversation

@nithyatsu
Copy link
Copy Markdown
Contributor

@nithyatsu nithyatsu commented Apr 21, 2026

Description

design doc focusing on rad graph enhancements based on the larger demo efforts.

Type of change

  • This pull request fixes a bug in Radius and has an approved issue (issue link required).
  • This pull request adds or changes features of Radius and has an approved issue (issue link required).
  • This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional).

Fixes: #issue_number

Contributor checklist

Please verify that the PR meets the following requirements, where applicable:

  • An overview of proposed schema changes is included in a linked GitHub issue.
    • Yes
    • Not applicable
  • A design document PR is created in the design-notes repository, if new APIs are being introduced.
    • Yes
    • Not applicable
  • The design document has been reviewed and approved by Radius maintainers/approvers.
    • Yes
    • Not applicable
  • A PR for the samples repository is created, if existing samples are affected by the changes in this PR.
    • Yes
    • Not applicable
  • A PR for the documentation repository is created, if the changes in this PR affect the documentation or any user facing updates are made.
    • Yes
    • Not applicable
  • A PR for the recipes repository is created, if existing recipes are affected by the changes in this PR.
    • Yes
    • Not applicable

Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
```

**Sample Output:** #Q: Should we have the output match below?
```
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willtsai to review the UX

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this output message looks fine to me

The command:
1. Invokes `bicep build` to compile `app.bicep` to ARM JSON.
2. Parses resources, connections, `dependsOn`, and `codeReference` from the JSON.
3. Detects source line mappings by scanning the Bicep file for `resource` declarations.
Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for " Parses resources, connections, dependsOn, and codeReference from the JSON." , should we distinguish between teh two links visually?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, dependsOn should be solid line, connections should be dotted line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Out of the core radius functionality.

| `--source-branch` | Source branch name used as the directory prefix on the orphan branch (required with `--orphan-branch`) | (none — required) |

When `--orphan-branch` is omitted, the artifact is written locally to `--output`. When `--orphan-branch` is provided, `--source-branch` is required and the artifact is committed to `{source-branch}/app.json` on the orphan branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make the source branch optional and set it to current branch if an orphan branch is provided?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, agree with this.

```

### Browser Extension: Repository root

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extensions related code will not be committed to main since they are only for demo purposes.

#### Scenario 3: Interactive navigation from graph to code

A developer clicks on a node in the graph (e.g., "cache") and sees a popup with links to: (1) the source code file referenced by the `codeReference` property, and (2) the `app.bicep` line where the resource is declared.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app.bicep navigation corresponds to what we have today in the github-demo branch. Should we log a user story to support deep link into application code? If so, we have to capture it in a user story. Also, we have to make sure the implementation is robust for complex app definitions.

```

The diffHash enables the browser extension to classify resources as modified vs unchanged without comparing all properties.
========================================
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will retain the diffHash to be consumed by the UI which would replace browser extension.

The PR graph displays all head resources plus removed resources from the base, applying diff status colors to each node.

**Edge case:** If `headArtifact` is null (PR CI hasn't completed yet), the extension shows a "waiting for CI" message instead of rendering a misleading all-red graph.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willtsai to review the UX message

2. **Personal Access Token** — User pastes a PAT directly in settings. Simpler but less secure.

Authentication is optional for public repos but required for private repos and to avoid API rate limits.

Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we capture the efforts needed for Authentication in a doc? @brooke-hamilton @sk593

| `appDefinitionLine` | Auto-detected from Bicep | Not set (null) |
| `codeReference` | From Bicep `properties.codeReference` | Not set (null) |
| Wrapped in | `StaticGraphArtifact` (with version, timestamp) | `ApplicationGraphResponse` (direct API response) |

Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we OK with keeping these extras at just the client model? It wouldn't make sense for these to be in server side since rest calls wouldnt be able to populate them.
| diffHash | Set by ComputeDiffHash() | Not set (null) |
| appDefinitionLine | Auto-detected from Bicep | Not set (null) |
| codeReference | From Bicep properties.codeReference | Not set (null) |

**Race condition risk:** If two PRs push at exactly the same time, one `git push` could fail with a non-fast-forward error. The current code does **not** retry — the push error is returned to the caller and the CI job fails. In practice this is rare because the concurrency group serializes builds per-branch, and different branches write to different directories. A future improvement could add a fetch-rebase-push retry loop.

**Cleanup:** Stale directories for merged/closed PRs are not automatically cleaned up from the orphan branch. Over time, the orphan branch accumulates directories for old branches. This is a known gap — a periodic cleanup job or a post-merge hook could be added to prune stale directories.

Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log items for these two points (race condition risk and cleanup)?

**Run-time graph persistence via `rad shutdown`:**

The `rad shutdown` command (from the `filesystem-state` branch) backs up PostgreSQL state and tears down the k3d cluster. A natural extension is to call `getGraph` for each deployed application during shutdown and write the graph JSON to the `radius-state` orphan branch alongside the SQL dumps. This would make run-time graphs available for visualization even after the cluster is destroyed — enabling the browser extension to show deployed infrastructure topology from the last known state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we have this today.

Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
@nithyatsu nithyatsu marked this pull request as ready for review April 21, 2026 19:39
@nithyatsu nithyatsu requested review from a team as code owners April 21, 2026 19:39
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Comment thread eng/design-notes/resources/2026-04-app-graph.md
|---|---|---|
| Static graph | `{branch}/app.json` on `radius-graph` orphan branch | CI runs `rad graph build` on push/PR |
| Run-time graph | `graphs/{app}.json` on `radius-state` orphan branch | `rad shutdown` serializes after deploy |

Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log stories for rad shutdown or is it tracked as part of repo-radius? Should we add the task related to persisting graph to that story?

Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Comment thread eng/design-notes/resources/2026-04-app-graph.md Outdated

When `--orphan-branch` is omitted, the artifact is written locally to `--output`. When `--orphan-branch` is provided, `--source-branch` is required and the artifact is committed to `{source-branch}/app.json` on the orphan branch. This means each branch gets its own directory — for example, CI for a PR from `feature-add-redis` writes to `feature-add-redis/app.json`, while a merge to `main` writes to `main/app.json`. The browser extension/ UI layers above use these directory names to fetch the correct base and head artifacts for diff comparison.

### CLI: `rad app graph` (existing)
Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we consider validating the command is running in github context, and would we introduce a rad workspace --kind github for this? While it might be an overhead to add this for a single command, if we bring in more specialized behaviors for gh integration, it might be useful to have a github-workspace kind or another marker for the scenario.


* Define a graph schema that is flexible and extensible enough to represent static, run-time, and simulated deployment graphs.
* Review the server-side API (`getGraph` custom action on `Applications.Core/applications|Radius.Core/applications`) that returns the run-time application graph for a deployed application, based on schema decisions.
* Identify a persistence mechanism since the graph should be available irrespective of the ephemeral nature of Radius control plane. The graph construction is still an in-memory operation.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to specify the scope of the persistence you're proposing in this design, i.e. proposal for persistence of the serialized data into an orphan branch as a GitHub specific implementation is very different from a proposal for the persistence of the app graph data into an external graph database (or equivalent).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, I will clarify that.

Comment thread eng/design-notes/resources/2026-04-app-graph.md Outdated

**Race condition risk:** If two PRs push at exactly the same time, one `git push` could fail with a non-fast-forward error. The current code does **not** retry — the push error is returned to the caller and the CI job fails. In practice this is rare because the concurrency group serializes builds per-branch, and different branches write to different directories. A future improvement could add a fetch-rebase-push retry loop.

**Cleanup:** Stale directories for merged/closed PRs are not automatically cleaned up from the orphan branch. Over time, the orphan branch accumulates directories for old branches. This is a known gap — a periodic cleanup job or a post-merge hook could be added to prune stale directories.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these easy to see for the user? or are orphan branches hidden

Comment thread eng/design-notes/resources/2026-04-app-graph.md

**Approach A: Include all properties (current behavior) [Preferred, current implementation]**

Dump every property from the resource's stored state into the graph node. All properties are read from the Radius control plane datastore.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any concern of secrets/ sensitive values being output in plain text on the branch?

Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont have "secrets" stored in any resource that is accessible through rest API. Therefore we should be safe. We should mark outputs of recipes as secret, if appropriate so they are not displayed too, but that is a general guideline.

Comment on lines +106 to +111
```bash
rad graph build \
--bicep app.bicep \
--orphan-branch radius-graph \
--source-branch main
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need additional options of where the serialized graph data will be "outputted":

  1. No output flags --> printed into the console
  2. --output-path <path> --> .json file saved to provided path

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be good to meet up to finalize on the commands and params. I will schedule one for the week.

Copy link
Copy Markdown
Contributor

@zachcasper zachcasper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two high-level comments:

  1. rad app graph and rad graph build need to be the same command. The output should be the same graph, just more data. The "modeled application graph" has the abstract application resources only. The "deployed application graph" has the abstract application resources plus deployed cloud resources.
  2. rad graph build should not have git operations built into it. If we are using it with orphaned branches with GitHub, those operations should be put into the GitHub workflow.

2. Parses resources, connections, `dependsOn`, and `codeReference` from the JSON.
3. Detects source line mappings by scanning the Bicep file for `resource` declarations.
4. Computes a `diffHash` for each resource based on relevant properties.
5. Commits the resulting `StaticGraphArtifact` JSON to `{source-branch}/app.json` on the orphan branch.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is going to be used outside the context of GitHub Radius, rad should only output the JSON and not commit to an orphaned branch. Those should be two separate operations: generate JSON by rad and the commit by git.

| `--bicep` | Path to Bicep application definition file | `app.bicep` |
| `--output` | Path for the output graph artifact (local file mode) | `.radius/static/app.json` |
| `--orphan-branch` | Commit the artifact to this git orphan branch instead of writing a local file | (none — local file mode) |
| `--source-branch` | Source branch name used as the directory prefix on the orphan branch (required with `--orphan-branch`) | (none — required) |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my comment above, I do not believe rad should have git operations built into it. git should be used to set the file system with the proper branch.


## User Experience

### CLI: `rad graph build`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we introducing a new command when we already have rad application graph --output json?


| Flag | Description | Default |
|------|-------------|--------|
| `--bicep` | Path to Bicep application definition file | `app.bicep` |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `--bicep` | Path to Bicep application definition file | `app.bicep` |
| `--app-definition` | Path to Bicep application definition file | `app.bicep` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--bicep is ambiguous. The input here is an application definition which happens to be in the Bicep format.

Comment thread eng/design-notes/resources/2026-04-app-graph.md Outdated
Comment thread eng/design-notes/resources/2026-04-app-graph.md Outdated
cancel-in-progress: true
```

This means: if a new push arrives on the same PR branch while a previous graph build is still running, the in-progress build is cancelled and replaced. Builds for *different* PR branches run in parallel since their `github.ref` values differ.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this cause inconsistencies in the diff views?


## User Experience

### CLI: `rad graph build`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the need for orphan-branch or source-branch here? rad graph build should be just the visualization of the app.bicep right?

Can we make the commands a bit more intuitive? rad graph build and rad app graph don't convey what they are meant to do.

May be rad app graph --static and rad app graph --deployed

* Visualizing application architecture from source code checked into a repository.
* Highlighting infrastructure changes introduced by a Pull Request.

**Limitation:** Because the concrete infrastructure resources depend on the recipe bound to each resource type — which in turn depends on the target Radius environment — the static graph cannot include infrastructure-level details.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we construct a static graph for recipes as well?

Copy link
Copy Markdown
Contributor Author

@nithyatsu nithyatsu May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea. Technically,

  1. we need the env ( can be passed in as a parameter)
  2. We resolve the recipe-packs to get the recipe url, fetch and run a rad app graph build on the bicep recipe or TF plan on the terraform recipe.

We could to place these resources into the outer radius resource box. We might want to populate outputresources and use a combination of outputresources + provisionedState to understand this is "planned".

@willtsai @brooke-hamilton @zachcasper Should we consider this research/ support in scope for the Radius graph feature we will merge into main?


#### 2. Run-time application graph (deployment graph)

The graph of a **live, deployed** application, as described above. This is the only graph type supported today.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only graph type supported today.

Are we changing anything on this from what's supported today?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since the attributes on static graph make sense only on CLI side, we are not changing the server side model and API.

Comment thread eng/design-notes/resources/2026-04-app-graph.md

### Non-goals

* Simulated deployment graph (dry-run) — identified as a future capability but out of scope for this iteration. This also requires enhancing Radius to avail tf plan/ what-if to understand recipe's output resources without executing them.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also requires enhancing Radius to avail tf plan/ what-if to understand recipe's output resources without executing them.

If we are building static graph for app.bicep without what-if, curious why can't we do the same for bicep recipe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused about which of the commands would display that, shuld we revisit the supported commands? I am not sure if rad graph build feels too limited in what it displays.

  1. rad app graph : actual deployed graph
  2. rad app graph -e env-id --dry-run : this should make a detailed graph by running what-ifs on app.bicep as well as on the resolved recipes
  3. rad graph build --bicep app.bicep : this builds static graph, but if we bring in the what -if here, there is not much diff between 2nd and 3rd commands.


#### Scenario 1: PR diff visualization with change highlighting

A developer modifies `app.bicep` to add a new Redis cache and connect it to an existing container. When they open a pull request, a color-coded diff graph appears below the PR description: added resources in **green**, removed resources in **red**, modified resources in **yellow**, and unchanged resources in **gray**. The reviewer can click any node to navigate to the source code or the `app.bicep` definition line.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the direction we’re heading, I wonder if this would be relevant. Would a developer manually edit app.bicep, or would they prefer to edit the graph directly and have the tooling update app.bicep?


A developer modifies `app.bicep` to add a new Redis cache and connect it to an existing container. When they open a pull request, a color-coded diff graph appears below the PR description: added resources in **green**, removed resources in **red**, modified resources in **yellow**, and unchanged resources in **gray**. The reviewer can click any node to navigate to the source code or the `app.bicep` definition line.

#### Scenario 2: Repository root architecture diagram
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be very useful! This will be enabled by default on every repo or do we envision this as an opt in?

Comment on lines +107 to +110
rad graph build \
--bicep app.bicep \
--orphan-branch radius-graph \
--source-branch main
Copy link
Copy Markdown
Member

@kachawla kachawla Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we detect which repository this is run on?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we assume the repo in which the workflow is running to be the repo of interest.


**Sample Input:**
```bash
rad graph build \
Copy link
Copy Markdown
Contributor

@willtsai willtsai Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to discuss the conventions for all our rad app graph commands - they should follow the same patterns across the modeled, planned, and deployed graphs. I propose:

rad app graph app.bicep                        // modeled (static) graph
rad app graph app.bicep -e my-env   // planned ("simulated") graph
rad app graph my-app -e my-env      // deployed ("deployment") graph

"id": "/planes/radius/local/resourcegroups/default/providers/Applications.Core/containers/frontend",
"name": "frontend",
"type": "Applications.Core/containers",
"provisioningState": "Succeeded",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean if this is only a static graph?

"version": "1.0.0",
"generatedAt": "2026-04-16T00:57:29Z",
"sourceFile": "app.bicep",
"application": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No application metadata or name?

"codeReference": "src/cache/redis.ts#L10"
},
{
"id": "/planes/radius/local/resourcegroups/default/providers/Applications.Core/environments/default",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not expect an Environment to be in an application graph.

Copy link
Copy Markdown
Member

@brooke-hamilton brooke-hamilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments:

  • We should persist the file with a name that indicates that it is the graph, e.g. *.graph.json to differentiate from compiled ARM json (if we ever persist that.)
  • This design assumes one app.bicep per repo. We should not assume that.
  • Let's remove the content related to creating a browser extension. I think we would want to commit the CLI command as a single feature.


**Sample Input:**
```bash
rad app graph my-app --output json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to continue to support this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, since this is the deployed graph.

└──────────────┘
```

We will merge Workflows, Browser extension and Cytoscape.js into another repository.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I follow this.. could you share more details?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because of a decision that we would have only the core radius functionality merged. But we have to track other code (specific to GH) too, for which we would use another private repo.


### API design and server-side support

No REST API changes or server-side changes are required for this iteration. The existing `getGraph` custom action on `Applications.Core/applications` continues to serve the run-time graph unchanged. Future work may add `diffHash`, `appDefinitionLine`, and `codeReference` to the server-side `ApplicationGraphResponse` TypeSpec definition.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the scope seems mixed right now between core Radius and GitHub specific experience. Earlier we say that "A static application graph built from Bicep definitions (no deployment required). This is the core Radius enhancement." which is conflicting with what is stated here. Would be good to make the distinction explicit.


**Race condition risk:** If two PRs push at exactly the same time, one `git push` could fail with a non-fast-forward error. The current code does **not** retry — the push error is returned to the caller and the CI job fails. In practice this is rare because the concurrency group serializes builds per-branch, and different branches write to different directories. A future improvement could add a fetch-rebase-push retry loop.

**Cleanup:** Stale directories for merged/closed PRs are not automatically cleaned up from the orphan branch. Over time, the orphan branch accumulates directories for old branches. This is a known gap — a periodic cleanup job or a post-merge hook could be added to prune stale directories.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of keeping them around?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I see the benefit of keeping the PR branches, since the branch would be merged into main in which case the app-graph.json in radius-graph branch would be updated. And they are versioned due to being commited to git, in case we wanted to compare and generate a timeline as a future feature. Please let me know if I am missing some use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants