Skip to content

feat(git): change a connected repository's root directory - #341

Open
swkeever wants to merge 4 commits into
mainfrom
skeever/git-root-directory
Open

swkeever wants to merge 4 commits into
mainfrom
skeever/git-root-directory

Conversation

@swkeever

@swkeever swkeever commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

What

volcano git root-directory <path> changes the subdirectory the connected repository builds from. It leaves the repository and production branch alone. --reset builds from the repository root again.

It calls Kong/volcano-hosting#2145's PUT /projects/{id}/git-connection/root-directory, which accepts a project access token. That lets CI or an agent move a project within its repository, while connecting a different repository stays with a person.

volcano git root-directory apps/web
volcano git root-directory --reset

How

  • The command (internal/cmd/git/root_directory.go):

    • It sends the path as given; the server stores it cleaned, so ./apps/web/ becomes apps/web.
    • It takes exactly one of a path or --reset. An empty or whitespace argument is refused, because that's what an unset variable in a script looks like.
    • The path goes through connect's validateRootDirectory, now shared, which refuses an absolute path or one with ...
    • Nothing is read first: the write names only the root directory, so it can't replace a repository connected meanwhile, and repeating it changes nothing.
    • The output says the directory was set and shows the binding. It says nothing was redeployed and the next deploy from the repository builds from it. Then, like connect, it says what a push to the production branch deploys, which is nothing while auto-deploy is off.
  • Errors, shown as the server gives them:

    • 400 (a path the server refuses, such as apps\..\..\outside on Linux, where the CLI's check doesn't treat \ as a separator) and 409 (source export or Git hand-over) read failed to set the root directory: HTTP 4xx: <server message>.
    • 404 goes through the existing explainNotFound, which tells a project with nothing connected ("Connect one with volcano git connect") from one that doesn't exist.
    • With an account token, a project with nothing connected first checks whether a repository can be connected at all (gitconnect.Service.CanConnect, one GET /user/git/connections). A 503 means the API has no GitHub App, as in local mode, and that becomes the reason given instead of advice to connect. A project access token can't read that route, so it skips the check.
  • volcano git connect --root-directory on the repository already bound, when only the root directory differs (same repository id, installation, name and production branch), now calls the new route instead of rebinding.

    • It keeps connect's check that the binding didn't change while the command ran.
    • A rename, a moved default branch or a reinstalled App still rebinds through the full PUT, which refreshes those.
  • A project access token on connect still fails before any write, because resolving the repository reads the account's GitHub connections. So no CLI path reaches the connect PUT's new 403. When --root-directory is set on a connected project, the refusal now names the command that works with the token:

    … run 'volcano login', or set VOLCANO_TOKEN to an account token
    
    To change only the root directory of the connected repository with this token, run:
      volcano git root-directory apps/web
    

Spec

openapi/openapi.yaml carries #2145's spec as merged (b22c133cf): the new path, SetProjectGitRootDirectoryRequest, and the connect PUT's security and 403 text. The merged diff is identical to what was vendored, and every block it touches matches hosting main verbatim. The client is regenerated.

Tests

  • Unit (internal/cmd/git):
    • root-directory: sets only the root directory, makes no GitHub calls and doesn't rebind; says what a push deploys, including nothing while auto-deploy is off, and still succeeds when the deploy settings can't be read; --reset; a project access token; every argument refusal; the server's 400, 403 and 409 messages; nothing connected; nothing can be connected (503); a project token on a project with nothing connected skips that check; a missing project.
    • connect: a root-only change goes through the new route; an emptied --root-directory too; a moved default branch still rebinds; a project token gets the root-directory hint (with shell quoting and --reset); a root directory changed under the command is not overwritten.
  • Local mode: TestLocalModeE2ESmoke gains requireLocalModeLeavesGitToTheCloud.
    • It points volcano git root-directory apps/web at the local stack (its API URL, user token and project) and requires the "git provider integration is not configured … If this is local mode, that is expected" reason, with no "Connect one".
    • It runs in the existing localmode-e2e job against kong/volcano:local-nightly, and passed on this PR.
    • I didn't run it locally: the suite stops and cleans the shared local stack, and other sessions are using Docker on this machine.
    • It doesn't depend on the route itself. An image without #2145 answers 404 there too, which leads to the same 503 check.
  • Cloud: TestCLIE2ESmokeGitRootDirectory is in Kong/volcano-hosting#2161, based on main, on a branch named like this one so hosting's CLI lane builds this branch. It covers:
    • the CLI's ../outside refusal;
    • the server's 400 for apps\..\..\outside;
    • a project access token reaching the route's 404;
    • connect with a project token refused for want of an account token.
  • API round trip: not added. tests/e2e/api can't connect a repository: binding needs a GitHub App installation, which that suite doesn't have. Changing the root directory of a connected repository is covered by the unit tests here and by #2145's handler and service tests.

make lint, go test ./... and make openapi-generated-check pass.

Docs

docs/git.md:

  • a "Changing the root directory" section, and a row in the operations table;
  • the project-access-token split in "How it relates";
  • the local-mode note;
  • the connect paragraph no longer says --root-directory is the only way to change it.

Follow-ups

  • volcano git production-branch: PUT …/production-branch has the same gap (token-changeable, no CLI command).
  • volcano git reads the cloud login even where top-level resource commands run against the local stack. That's existing behavior, unchanged here.

🤖 Generated with Claude Code

Add `volcano git root-directory <path>` and `--reset`, which call
PUT /projects/{id}/git-connection/root-directory. It changes only the
directory the project builds from and accepts a project access token.

`volcano git connect --root-directory` on the repository already bound
now uses the same route instead of rebinding. With a project access
token, connect names the root-directory command.
@swkeever

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T23:35:51.917647Z ded0c85 Manual request
🔒 Security Review ✅ Completed 2026-10-23T23:36:59.217439Z ded0c85 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: cf6c1b3648

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cf6c1b3648

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/output/git.go Outdated
@swkeever

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0177c6c7f0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/cmd/git/connect.go Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 0177c6c7f0

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@swkeever

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 97a40b550a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 97a40b550a

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@swkeever
swkeever marked this pull request as ready for review October 23, 2026 23:21
@swkeever
swkeever requested a review from a team as a code owner October 23, 2026 23:21
Copilot AI balanced review requested due to automatic review settings October 23, 2026 23:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

AI review: no P0-P2 findings

Reviewed ded0c85. Run log

Adds volcano git root-directory <path>|--reset, which calls the new PUT /projects/{id}/git-connection/root-directory route. A project access token can use that route. connect --root-directory on the repository already bound now uses the same route instead of a full rebind. The PR also adds a hint for project tokens, the vendored spec and generated client, docs, unit tests and a local-mode e2e step. The earlier P2 finding (add -- before a suggested directory that starts with a hyphen) is fixed at this head, with tests. Risk is low: the change only adds a command, and the authorization decision belongs to the server.

Not checked: The AI review read the diff, the code, and the PR's text; it ran no tests or builds and read nothing outside this repository.

  • Hosting #2145's route must be live wherever this CLI release points, or connect --root-directory on a connected repository gets a 404 and says the project is not connected.
  • Hosting must confirm that the new route rejects read-only and other-project tokens, and leaves the repository, installation and production branch unchanged.
  • The new local-mode e2e step depends on the local-nightly image answering 503 on /user/git/connections for the local user token. The description says it passed in CI.
  • Cloud E2E coverage for the new command lives in Kong/volcano-hosting#2161, not in this repository.
  • Confirm hosting #2145 is deployed before release; older APIs return 404 for root-directory updates even on connected projects.

Human review: needed

  • Is removing project-token authentication from PUT /projects/{id}/git-connection in openapi/openapi.yaml acceptable for existing API consumers?

The AI review found no P0-P2 problems at ded0c85, so the code needs no second review for bugs. Decide the points above: approve to accept them, or request changes.

Suggested reviewer: shawnwu-kong (has worked on internal/cmd/git/connect.go and connect_test.go, where connect's write routing changed). Any collaborator with write access other than the author can approve.

@github-actions
github-actions Bot requested a review from shawnwu-kong October 23, 2026 23:25
Comment thread internal/cmd/git/connect.go Outdated
@swkeever

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: ded0c85ca1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: ded0c85ca1

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants